fix(ISSUE-8): try using admin form (#16)

* Remove casting to list in _get_form_data

* feat(ISSUE-8): Use Django's form and hide with css

* feat(ISSUE-8): Updated dev release on testpypi

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

Makefile

@@ -18,11 +18,16 @@ package:
 	python3 setup.py sdist bdist_wheel
 
 upload-testpypi:
-	python3 -m twine upload --repository testpypi dist/django_admin_confirm-$(VERSION)
+	python3 -m twine upload --repository testpypi dist/django_admin_confirm-$(VERSION)*
 
 i-have-tested-with-testpypi-and-am-ready-to-release:
-	python3 -m twine upload --repository pypi dist/django_admin_confirm-$(VERSION)
+	python3 -m twine upload --repository pypi dist/django_admin_confirm-$(VERSION)*
 
 install-testpypi:
 	pip uninstall django_admin_confirm
 	python -m pip install --index-url https://test.pypi.org/simple/ django_admin_confirm==${VERSION}
+
+testpypi:
+	python3 -m twine upload --repository testpypi dist/django_admin_confirm-$(VERSION)*
+	pip uninstall django_admin_confirm
+	python -m pip install --index-url https://test.pypi.org/simple/ django_admin_confirm==${VERSION}

admin_confirm/admin.py

@@ -137,26 +137,9 @@ class AdminConfirmMixin:
 
         return changed_data
 
-    def _get_form_data(self, request):
-        """
-        Parses the request post params into a format that can be used for the hidden form on the
-        change confirmation page.
-        """
-        form_data = request.POST.copy()
-
-        for key in SAVE_ACTIONS + [
-            "_confirm_change",
-            "_confirm_add",
-            "csrfmiddlewaretoken",
-        ]:
-            if form_data.get(key):
-                form_data.pop(key)
-
-        form_data = [(k, list(v)) for k, v in form_data.lists()]
-        return form_data
-
     def _change_confirmation_view(self, request, object_id, form_url, extra_context):
         # This code is taken from super()._changeform_view
+        # https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1575-L1592
         to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR))
         if to_field and not self.to_field_allowed(request, to_field):
             raise DisallowedModelAdminToField(
@@ -186,8 +169,12 @@ class AdminConfirmMixin:
         )
 
         form = ModelForm(request.POST, request.FILES, obj)
+        # Note to self: For inline instances see:
+        # https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1582
+
         # End code from super()._changeform_view
 
+        # Get changed data to show on confirmation
         changed_data = self._get_changed_data(form, model, obj, add)
 
         changed_confirmation_fields = set(
@@ -197,8 +184,6 @@ class AdminConfirmMixin:
             # No confirmation required for changed fields, continue to save
             return super()._changeform_view(request, object_id, form_url, extra_context)
 
-        # Parse raw form data from POST
-        form_data = self._get_form_data(request)
         # Parse the original save action from request
         save_action = None
         for key in request.POST.keys():
@@ -218,10 +203,10 @@ class AdminConfirmMixin:
             "app_label": opts.app_label,
             "model_name": opts.model_name,
             "opts": opts,
-            "form_data": form_data,
             "changed_data": changed_data,
             "add": add,
             "submit_name": save_action,
+            "form": form,
             **(extra_context or {}),
         }
         return self.render_change_confirmation(request, context)

admin_confirm/static/admin/css/confirmation.css

@@ -22,3 +22,7 @@
   width: 30%;
   white-space: nowrap;
 }
+
+.hidden {
+  display: none;
+}

admin_confirm/templates/admin/change_confirmation.html

@@ -42,12 +42,9 @@
     {% include "admin/change_data.html" %}
     <form method="post" action="{% url opts|admin_urlname:'change' object_id|admin_urlquote %}">{% csrf_token %}
   {% endif %}
-
+    <div class=hidden>
-    {% for key, values in form_data %}
+        {{ form }}
-        {% for v in values %}
+    </div>
-        <input type="hidden" name="{{ key }}" value="{{v}}">
-        {% endfor %}
-    {% endfor %}
     {% if is_popup %}<input type="hidden" name="{{ is_popup_var }}" value="1">{% endif %}
     {% if to_field %}<input type="hidden" name="{{ to_field_var }}" value="{{ to_field }}">{% endif %}
     <div class="submit-row">

admin_confirm/tests/helpers.py

@@ -0,0 +1,40 @@
+from django.test import TestCase, RequestFactory
+from django.contrib.auth.models import User
+
+
+class ConfirmAdminTestCase(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.superuser = User.objects.create_superuser(
+            username="super", email="super@email.org", password="pass"
+        )
+
+    def setUp(self):
+        self.client.force_login(self.superuser)
+        self.factory = RequestFactory()
+
+    def _assertManyToManyFormHtml(self, rendered_content, options, selected_ids):
+        # Form data should be embedded and hidden on confirmation page
+        # Should have the correct ManyToMany options selected
+        for option in options:
+            self.assertIn(
+                f'<option value="{option.id}"{" selected" if option.id in selected_ids else ""}>{str(option)}</option>',
+                rendered_content,
+            )
+        # ManyToManyField should be embedded
+        self.assertIn("related-widget-wrapper", rendered_content)
+
+    def _assertSubmitHtml(self, rendered_content, save_action="_save"):
+        # Submit should conserve the save action
+        self.assertIn(
+            f'<input type="submit" value="Yes, I’m sure" name="{save_action}">',
+            rendered_content,
+        )
+        # There should not be _confirm_add or _confirm_change sent in the form on confirmaiton page
+        self.assertNotIn("_confirm_add", rendered_content)
+        self.assertNotIn("_confirm_change", rendered_content)
+
+    def _assertSimpleFieldFormHtml(self, rendered_content, fields):
+        for k, v in fields.items():
+            self.assertIn(f'name="{k}"', rendered_content)
+            self.assertIn(f'value="{v}"', rendered_content)

admin_confirm/tests/test_confirm_change_and_add.py

@@ -1,27 +1,16 @@
-from django.test import TestCase, RequestFactory
 from django.contrib.auth.models import User
 from django.contrib.admin.sites import AdminSite
 from django.contrib.admin.options import TO_FIELD_VAR
 from django.http import HttpResponseForbidden, HttpResponseBadRequest
 from django.urls import reverse
 
-
+from admin_confirm.tests.helpers import ConfirmAdminTestCase
 from tests.market.admin import ItemAdmin, InventoryAdmin
 from tests.market.models import Item, Inventory
 from tests.factories import ItemFactory, ShopFactory, InventoryFactory
 
 
-class TestConfirmChangeAndAdd(TestCase):
+class TestConfirmChangeAndAdd(ConfirmAdminTestCase):
-    @classmethod
-    def setUpTestData(cls):
-        cls.superuser = User.objects.create_superuser(
-            username="super", email="super@email.org", password="pass"
-        )
-
-    def setUp(self):
-        self.client.force_login(self.superuser)
-        self.factory = RequestFactory()
-
     def test_get_add_without_confirm_add(self):
         response = self.client.get(reverse("admin:market_item_add"))
         self.assertFalse(response.context_data.get("confirm_add"))
@@ -53,7 +42,13 @@ class TestConfirmChangeAndAdd(TestCase):
     def test_post_add_with_confirm_add(self):
         item = ItemFactory()
         shop = ShopFactory()
-        data = {"shop": shop.id, "item": item.id, "quantity": 5, "_confirm_add": True}
+        data = {
+            "shop": shop.id,
+            "item": item.id,
+            "quantity": 5,
+            "_confirm_add": True,
+            "_continue": True,
+        }
         response = self.client.post(reverse("admin:market_inventory_add"), data)
         # Ensure not redirected (confirmation page does not redirect)
         self.assertEqual(response.status_code, 200)
@@ -63,11 +58,13 @@ class TestConfirmChangeAndAdd(TestCase):
             "admin/change_confirmation.html",
         ]
         self.assertEqual(response.template_name, expected_templates)
+
         form_data = {"shop": str(shop.id), "item": str(item.id), "quantity": str(5)}
-        for k, v in form_data.items():
+        self._assertSimpleFieldFormHtml(
-            self.assertIn(
+            rendered_content=response.rendered_content, fields=form_data
-                f'<input type="hidden" name="{ k }" value="{ v }">',
+        )
-                response.rendered_content,
+        self._assertSubmitHtml(
+            rendered_content=response.rendered_content, save_action="_continue"
         )
 
         # Should not have been added yet
@@ -96,14 +93,14 @@ class TestConfirmChangeAndAdd(TestCase):
         form_data = {
             "name": "name",
             "price": str(2.0),
-            "id": str(item.id),
+            # "id": str(item.id),
             "currency": Item.VALID_CURRENCIES[0][0],
         }
-        for k, v in form_data.items():
+
-            self.assertIn(
+        self._assertSimpleFieldFormHtml(
-                f'<input type="hidden" name="{ k }" value="{ v }">',
+            rendered_content=response.rendered_content, fields=form_data
-                response.rendered_content,
         )
+        self._assertSubmitHtml(rendered_content=response.rendered_content)
 
         # Hasn't changed item yet
         item.refresh_from_db()

admin_confirm/tests/test_confirm_change_and_add_m2m_field.py

@@ -1,24 +1,12 @@
-from django.test import TestCase, RequestFactory
-from django.contrib.auth.models import User
 from django.urls import reverse
 
-
+from admin_confirm.tests.helpers import ConfirmAdminTestCase
 from tests.market.admin import ShoppingMallAdmin
 from tests.market.models import ShoppingMall
 from tests.factories import ShopFactory
 
 
-class TestConfirmChangeAndAddM2MField(TestCase):
+class TestConfirmChangeAndAddM2MField(ConfirmAdminTestCase):
-    @classmethod
-    def setUpTestData(cls):
-        cls.superuser = User.objects.create_superuser(
-            username="super", email="super@email.org", password="pass"
-        )
-
-    def setUp(self):
-        self.client.force_login(self.superuser)
-        self.factory = RequestFactory()
-
     def test_post_add_without_confirm_add_m2m(self):
         shops = [ShopFactory() for i in range(3)]
 
@@ -50,19 +38,13 @@ class TestConfirmChangeAndAddM2MField(TestCase):
             "admin/change_confirmation.html",
         ]
         self.assertEqual(response.template_name, expected_templates)
-        form_data = [("name", "name")] + [("shops", s.id) for s in shops]
+
-        for k, v in form_data:
+        self._assertManyToManyFormHtml(
-            self.assertIn(
+            rendered_content=response.rendered_content,
-                f'<input type="hidden" name="{ k }" value="{ v }">',
+            options=shops,
-                response.rendered_content,
+            selected_ids=data["shops"],
         )
-        # Submit should conserve the save action
+        self._assertSubmitHtml(rendered_content=response.rendered_content)
-        self.assertIn(
-            '<input type="submit" value="Yes, I’m sure" name="_save">',
-            response.rendered_content,
-        )
-        # There should not be _confirm_add sent in the form on confirmaiton page
-        self.assertNotIn("_confirm_add", response.rendered_content)
 
         # Should not have been added yet
         self.assertEqual(ShoppingMall.objects.count(), 0)
@@ -84,7 +66,7 @@ class TestConfirmChangeAndAddM2MField(TestCase):
         # Currently ShoppingMall configured with confirmation_fields = ['name']
         data = {
             "name": "Not My Mall",
-            "shops": ["1", "2"],
+            "shops": [1, 2],
             "id": shopping_mall.id,
             "_confirm_change": True,
             "csrfmiddlewaretoken": "fake token",
@@ -101,25 +83,15 @@ class TestConfirmChangeAndAddM2MField(TestCase):
             "admin/change_confirmation.html",
         ]
         self.assertEqual(response.template_name, expected_templates)
-        form_data = {
-            "name": "Not My Mall",
-            "shops": "1",
-            "shops": "2",
-            "id": str(shopping_mall.id),
-        }
 
-        for k, v in form_data.items():
+        self._assertManyToManyFormHtml(
-            self.assertIn(
+            rendered_content=response.rendered_content,
-                f'<input type="hidden" name="{ k }" value="{ v }">',
+            options=shops,
-                response.rendered_content,
+            selected_ids=data["shops"],
         )
-        # Submit should conserve the save action
+        self._assertSubmitHtml(
-        self.assertIn(
+            rendered_content=response.rendered_content, save_action="_continue"
-            '<input type="submit" value="Yes, I’m sure" name="_continue">',
-            response.rendered_content,
         )
-        # There should not be _confirm_change sent in the form on confirmaiton page
-        self.assertNotIn("_confirm_change", response.rendered_content)
 
         # Hasn't changed item yet
         shopping_mall.refresh_from_db()
@@ -148,7 +120,7 @@ class TestConfirmChangeAndAddM2MField(TestCase):
         # Currently ShoppingMall configured with confirmation_fields = ['name']
         data = {
             "name": "Not My Mall",
-            "shops": ["1", "2", "3"],
+            "shops": [1, 2, 3],
             "id": shopping_mall.id,
             "_confirm_change": True,
             "csrfmiddlewaretoken": "fake token",
@@ -165,19 +137,6 @@ class TestConfirmChangeAndAddM2MField(TestCase):
             "admin/change_confirmation.html",
         ]
         self.assertEqual(response.template_name, expected_templates)
-        form_data = [
-            ("name", "Not My Mall"),
-            ("shops", "1"),
-            ("shops", "2"),
-            ("shops", "3"),
-            ("id", str(shopping_mall.id)),
-        ]
-
-        for k, v in form_data:
-            self.assertIn(
-                f'<input type="hidden" name="{ k }" value="{ v }">',
-                response.rendered_content,
-            )
 
         # Hasn't changed item yet
         shopping_mall.refresh_from_db()
@@ -209,7 +168,7 @@ class TestConfirmChangeAndAddM2MField(TestCase):
         data = {
             "id": shopping_mall.id,
             "name": "name",
-            "shops": ["1", "2", "3"],  # These shops don't exist
+            "shops": [1, 2, 3],  # These shops don't exist
             "_confirm_change": True,
             "csrfmiddlewaretoken": "fake token",
         }

setup.py

@@ -6,7 +6,7 @@ README = open(os.path.join(here, "README.md")).read()
 
 setup(
     name="django-admin-confirm",
-    version="0.2.3.dev4",
+    version="0.2.3.dev5",
     packages=["admin_confirm"],
     description="Adds confirmation to Django Admin changes, additions and actions",
     long_description_content_type="text/markdown",