feat(ISSUE-8): fixing error caused by ManyToManyField on confirmation (#9)

* feat(ISSUE-8): ISSUE-8: ManyToManyField causes error on confirmations

* feat(ISSUE-8): Update some readme and remove print statements

Co-authored-by: Thu Trang Pham <thu@joinmodernhealth.com>

README.md

@@ -114,6 +114,8 @@ This would confirm `action2` but not `action1`.
 
 Action confirmation will respect `allowed_permissions` and the `has_xxx_permission` methods.
 
+> Note: AdminConfirmMixin does not confirm any changes on inlines
+
 ## Contribution & Appreciation
 
 Contributions are most welcome :) Feel free to:
@@ -206,6 +208,7 @@ Go on github and make a release in UI
 This is a list of features which could potentially be added in the future. Some of which might make more sense in their own package.
 
 - [x] confirmations on changelist actions
+- [ ] confirmations on inlines
 - [ ] global actions on changelist page
 - [ ] instance actions on change/view page
 - [ ] action logs (adding actions to history of instances)

admin_confirm/admin.py

@@ -123,39 +123,25 @@ class AdminConfirmMixin:
         )
 
         form = ModelForm(request.POST, request.FILES, obj)
-        form_validated = form.is_valid()
-        if form_validated:
-            new_object = self.save_form(request, form, change=not add)
-        else:
-            new_object = form.instance
-
         # End code from super()._changeform_view
 
         changed_data = {}
-        if form_validated:
+        if form.is_valid():
             if add:
-                for name in form.cleaned_data:
-                    new_value = getattr(new_object, name)
+                for name, new_value in form.cleaned_data.items():
                     # Don't consider default values as changed for adding
                     default_value = model._meta.get_field(name).get_default()
-                    if (
-                        new_value is not None
-                        and new_value != default_value
-                    ):
+                    if new_value is not None and new_value != default_value:
                         # Show what the default value is
                         changed_data[name] = [str(default_value), new_value]
             else:
                 # Parse the changed data - Note that using form.changed_data would not work because initial is not set
-                for name, field in form.fields.items():
+                for name, new_value in form.cleaned_data.items():
                     # Since the form considers initial as the value first shown in the form
                     # It could be incorrect when user hits save, and then hits "No, go back to edit"
                     obj.refresh_from_db()
                     initial_value = getattr(obj, name)
-                    new_value = getattr(new_object, name)
-                    if (
-                        field.has_changed(initial_value, new_value)
-                        and initial_value != new_value
-                    ):
+                    if initial_value != new_value:
                         changed_data[name] = [initial_value, new_value]
 
         changed_confirmation_fields = set(

admin_confirm/tests/test_confirm_change_and_add.py

@@ -165,7 +165,6 @@ class TestConfirmChangeAndAdd(TestCase):
 
         # Form invalid should show errors on form
         self.assertEqual(response.status_code, 200)
-        print(response.rendered_content)
         self.assertIsNotNone(response.context_data.get("errors"))
         self.assertEqual(
             response.context_data["errors"][0],

tests/market/admin.py

@@ -2,7 +2,7 @@ from django.contrib import admin
 
 from admin_confirm.admin import AdminConfirmMixin, confirm_action
 
-from .models import Item, Inventory, Shop
+from .models import Item, Inventory, Shop, ShoppingMall
 
 
 class ItemAdmin(AdminConfirmMixin, admin.ModelAdmin):
@@ -19,7 +19,6 @@ class InventoryAdmin(AdminConfirmMixin, admin.ModelAdmin):
 
 class ShopAdmin(AdminConfirmMixin, admin.ModelAdmin):
     confirmation_fields = ["name"]
-
     actions = ["show_message", "show_message_no_confirmation"]
 
     @confirm_action
@@ -27,7 +26,7 @@ class ShopAdmin(AdminConfirmMixin, admin.ModelAdmin):
         shops = ", ".join(shop.name for shop in queryset)
         modeladmin.message_user(request, f"You selected with confirmation: {shops}")
 
-    show_message.allowed_permissions = ('delete',)
+    show_message.allowed_permissions = ("delete",)
 
     def show_message_no_confirmation(modeladmin, request, queryset):
         shops = ", ".join(shop.name for shop in queryset)
@@ -36,6 +35,14 @@ class ShopAdmin(AdminConfirmMixin, admin.ModelAdmin):
     def has_delete_permission(self, request, obj=None):
         return request.user.is_superuser
 
+
+class ShoppingMallAdmin(AdminConfirmMixin, admin.ModelAdmin):
+    confirm_add = True
+    confirm_change = True
+    confirmation_fields = ["name"]
+
+
 admin.site.register(Item, ItemAdmin)
 admin.site.register(Inventory, InventoryAdmin)
 admin.site.register(Shop, ShopAdmin)
+admin.site.register(ShoppingMall, ShoppingMallAdmin)

tests/market/migrations/0005_shoppingmall.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.1.6 on 2021-02-18 17:46
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('market', '0004_inventory_notes'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='ShoppingMall',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('name', models.CharField(max_length=120)),
+                ('shops', models.ManyToManyField(to='market.Shop')),
+            ],
+        ),
+    ]

tests/market/models.py

@@ -33,3 +33,11 @@ class Inventory(models.Model):
     item = models.ForeignKey(to=Item, on_delete=models.CASCADE)
     quantity = models.PositiveIntegerField(default=0, null=True, blank=True)
     notes = models.TextField(default="This is the default", null=True, blank=True)
+
+
+class ShoppingMall(models.Model):
+    name = models.CharField(max_length=120)
+    shops = models.ManyToManyField(Shop)
+
+    def __str__(self):
+        return self.name