Cache should obey exclude and fields

Makefile

@@ -3,6 +3,7 @@ run:
 
 test:
 	coverage run --source admin_confirm --branch -m pytest
+	coverage report -m
 
 check-readme:
 	python -m readme_renderer README.md -o /tmp/README.html

admin_confirm/admin.py

@@ -171,20 +171,36 @@ def _confirmation_received_view(self, request, object_id, form_url, extra_contex
         these stored on it
         """
         add = object_id is None
+        obj = None
         new_object = cache.get(CACHE_KEYS["object"])
         change_message = cache.get(CACHE_KEYS["change_message"])
 
-        new_object.id = object_id
-        new_object.save()
+        exclude = self.exclude
+        fields = self.fields
+        opts = new_object._meta
+
+        # Must respect fields and exclude, the new_object would only have values for
+        # fields that are in the post form. Thus, we must only save those values.
+        if not add:
+            obj = type(new_object).objects.get(id=object_id)
+            if fields:
+                for field in fields:
+                    setattr(obj, field, getattr(new_object, field))
+                obj.save()
+                new_object = obj
+            else:
+                new_object.id = object_id
+                new_object.save()
+        else:
+            new_object.id = object_id
+            new_object.save()
 
         # Can't use QueryDict.get() because it only returns the last value for multiselect
         query_dict = {k: v for k, v in request.POST.lists()}
 
         # Taken from _save_m2m with slight modification
         # https://github.com/django/django/blob/master/django/forms/models.py#L430-L449
-        exclude = self.exclude
-        fields = self.fields
-        opts = new_object._meta
+
         # Note that for historical reasons we want to include also
         # private_fields here. (GenericRelation was previously a fake
         # m2m field).
@@ -255,7 +271,6 @@ def _change_confirmation_view(self, request, object_id, form_url, extra_context)
         # https://github.com/django/django/blob/master/django/contrib/admin/options.py#L1582
 
         # End code from super()._changeform_view
-
         if not (form_validated and all_valid(formsets)):
             # Form not valid, cannot confirm
             return super()._changeform_view(request, object_id, form_url, extra_context)
@@ -281,7 +296,6 @@ def _change_confirmation_view(self, request, object_id, form_url, extra_context)
                 break
 
         title_action = _("adding") if add else _("changing")
-
         context = {
             **self.admin_site.each_context(request),
             "preserved_filters": self.get_preserved_filters(request),

admin_confirm/tests/test_admin_options.py

@@ -0,0 +1,206 @@
+from unittest import mock
+from django.core.files.uploadedfile import SimpleUploadedFile
+from django.core.cache import cache
+from django.urls import reverse
+
+from admin_confirm.tests.helpers import ConfirmAdminTestCase
+from tests.market.admin import ItemAdmin, ShoppingMallAdmin
+from tests.market.models import GeneralManager, Item, ShoppingMall, Town
+from tests.factories import ItemFactory, ShopFactory
+
+from admin_confirm.constants import CACHE_KEYS
+
+
+class TestAdminOptions(ConfirmAdminTestCase):
+    @mock.patch.object(ShoppingMallAdmin, "confirmation_fields", ["name"])
+    @mock.patch.object(ShoppingMallAdmin, "fields", ["name", "town"])
+    def test_m2m_field_not_in_fields(self):
+        gm = GeneralManager.objects.create(name="gm")
+        shops = [ShopFactory() for i in range(3)]
+        town = Town.objects.create(name="town")
+        mall = ShoppingMall.objects.create(name="mall", general_manager=gm, town=town)
+        mall.shops.set(shops)
+
+        # new values
+        gm2 = GeneralManager.objects.create(name="gm2")
+        shops2 = [ShopFactory() for i in range(3)]
+        town2 = Town.objects.create(name="town2")
+
+        data = {
+            "id": mall.id,
+            "name": "name",
+            "town": town2.id,
+            "_confirm_change": True,
+            "_continue": True,
+        }
+        response = self.client.post(
+            f"/admin/market/shoppingmall/{mall.id}/change/", data=data
+        )
+
+        # Should be shown confirmation page
+        self._assertSubmitHtml(
+            rendered_content=response.rendered_content, save_action="_continue"
+        )
+
+        # Should have cached the unsaved obj
+        cached_item = cache.get(CACHE_KEYS["object"])
+        self.assertIsNotNone(cached_item)
+        self.assertIsNone(cached_item.id)
+
+        cached_change_message = cache.get(CACHE_KEYS["change_message"])
+        self.assertIsNotNone(cached_change_message)
+        self.assertIn("changed", cached_change_message[0].keys())
+
+        # Should not have saved changes yet
+        self.assertEqual(ShoppingMall.objects.count(), 1)
+        mall.refresh_from_db()
+        self.assertEqual(mall.name, "mall")
+        self.assertEqual(mall.general_manager, gm)
+        self.assertEqual(mall.town, town)
+        for shop in mall.shops.all():
+            self.assertIn(shop, shops)
+
+        # Click "Yes, I'm Sure"
+        confirmation_received_data = data
+        del confirmation_received_data["_confirm_change"]
+        confirmation_received_data["_confirmation_received"] = True
+
+        response = self.client.post(
+            f"/admin/market/shoppingmall/{mall.id}/change/",
+            data=confirmation_received_data,
+        )
+
+        # Should not have redirected to changelist
+        self.assertEqual(response.url, f"/admin/market/shoppingmall/{mall.id}/change/")
+
+        # Should have saved obj
+        self.assertEqual(ShoppingMall.objects.count(), 1)
+        saved_item = ShoppingMall.objects.all().first()
+        # should have updated fields that were in form
+        self.assertEqual(saved_item.name, data["name"])
+        self.assertEqual(saved_item.town, town2)
+        # should have presevered the fields that are not in form
+        self.assertEqual(saved_item.general_manager, gm)
+        for shop in saved_item.shops.all():
+            self.assertIn(shop, shops)
+
+        # Should have cleared cache
+        for key in CACHE_KEYS.values():
+            self.assertIsNone(cache.get(key))
+
+    @mock.patch.object(ShoppingMallAdmin, "confirmation_fields", ["name"])
+    @mock.patch.object(ShoppingMallAdmin, "exclude", ["shops"])
+    def test_m2m_field_in_exclude(self):
+        gm = GeneralManager.objects.create(name="gm")
+        shops = [ShopFactory() for i in range(3)]
+        town = Town.objects.create(name="town")
+        mall = ShoppingMall.objects.create(name="mall", general_manager=gm, town=town)
+        mall.shops.set(shops)
+
+        # new values
+        gm2 = GeneralManager.objects.create(name="gm2")
+        shops2 = [ShopFactory() for i in range(3)]
+        town2 = Town.objects.create(name="town2")
+
+        data = {
+            "id": mall.id,
+            "name": "name",
+            "general_manager": gm2.id,
+            "shops": [1],
+            "town": town2.id,
+            "_confirm_change": True,
+            "_continue": True,
+        }
+        response = self.client.post(
+            f"/admin/market/shoppingmall/{mall.id}/change/", data=data
+        )
+        # Should be shown confirmation page
+        self._assertSubmitHtml(
+            rendered_content=response.rendered_content, save_action="_continue"
+        )
+
+        # Should have cached the unsaved obj
+        cached_item = cache.get(CACHE_KEYS["object"])
+        self.assertIsNotNone(cached_item)
+        self.assertIsNone(cached_item.id)
+
+        cached_change_message = cache.get(CACHE_KEYS["change_message"])
+        self.assertIsNotNone(cached_change_message)
+        self.assertIn("changed", cached_change_message[0].keys())
+
+        # Should not have saved changes yet
+        self.assertEqual(ShoppingMall.objects.count(), 1)
+        mall.refresh_from_db()
+        self.assertEqual(mall.name, "mall")
+        self.assertEqual(mall.general_manager, gm)
+        self.assertEqual(mall.town, town)
+        for shop in mall.shops.all():
+            self.assertIn(shop, shops)
+
+        # Click "Yes, I'm Sure"
+        confirmation_received_data = data
+        del confirmation_received_data["_confirm_change"]
+        confirmation_received_data["_confirmation_received"] = True
+
+        response = self.client.post(
+            f"/admin/market/shoppingmall/{mall.id}/change/",
+            data=confirmation_received_data,
+        )
+
+        # Should not have redirected to changelist
+        self.assertEqual(response.url, f"/admin/market/shoppingmall/{mall.id}/change/")
+
+        # Should have saved obj
+        self.assertEqual(ShoppingMall.objects.count(), 1)
+        saved_item = ShoppingMall.objects.all().first()
+        # should have updated fields that were in form
+        self.assertEqual(saved_item.name, data["name"])
+        self.assertEqual(saved_item.town, town2)
+        self.assertEqual(saved_item.general_manager, gm2)
+        # should have presevered the fields that are not in form (exclude)
+        for shop in saved_item.shops.all():
+            self.assertIn(shop, shops)
+
+        # Should have cleared cache
+        for key in CACHE_KEYS.values():
+            self.assertIsNone(cache.get(key))
+
+    @mock.patch.object(ShoppingMallAdmin, "confirmation_fields", ["name"])
+    @mock.patch.object(ShoppingMallAdmin, "exclude", ["shops", "name"])
+    def test_confirmation_fields_in_exclude(self):
+        gm = GeneralManager.objects.create(name="gm")
+        shops = [ShopFactory() for i in range(3)]
+        town = Town.objects.create(name="town")
+        mall = ShoppingMall.objects.create(name="mall", general_manager=gm, town=town)
+        mall.shops.set(shops)
+
+        # new values
+        gm2 = GeneralManager.objects.create(name="gm2")
+        shops2 = [ShopFactory() for i in range(3)]
+        town2 = Town.objects.create(name="town2")
+
+        data = {
+            "id": mall.id,
+            "name": "name",
+            "general_manager": gm2.id,
+            "shops": [1],
+            "town": town2.id,
+            "_confirm_change": True,
+            "_continue": True,
+        }
+        response = self.client.post(
+            f"/admin/market/shoppingmall/{mall.id}/change/", data=data
+        )
+        # Should not be shown confirmation page
+        # SInce we used "Save and Continue", should show change page
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, f"/admin/market/shoppingmall/{mall.id}/change/")
+
+        # Should have saved the non excluded fields
+        mall.refresh_from_db()
+        for shop in shops:
+            self.assertIn(shop, mall.shops.all())
+        self.assertEqual(mall.name, "mall")
+        # Should have saved other fields
+        self.assertEqual(mall.town, town2)
+        self.assertEqual(mall.general_manager, gm2)

admin_confirm/tests/test_confirmation_cache.py

@@ -1,3 +1,4 @@
+from unittest import mock
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.core.cache import cache
 from django.urls import reverse