diff --git a/.gitignore b/.gitignore index dde0baf..a695b50 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,9 @@ sdist/ *.egg-info/ .DS_Store +# Editor settings +.vscode/ + # Unit test / coverage reports htmlcov/ .tox/ diff --git a/README.md b/README.md index 53cc8fa..39d898c 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,6 @@ Running tests: ``` make test -tox ``` Testing new changes on test project: @@ -171,12 +170,11 @@ make run Honestly this part is just for my reference. But who knows :) maybe we'll have another maintainer in the future. -Run tests, check coverage, check readme, run tox +Run tests, check coverage, check readme ``` make test make check-readme -tox ``` Update version in `setup.py` diff --git a/admin_confirm/admin.py b/admin_confirm/admin.py index 6198712..1249a45 100644 --- a/admin_confirm/admin.py +++ b/admin_confirm/admin.py @@ -6,7 +6,8 @@ from django.template.response import TemplateResponse from django.contrib.admin.options import TO_FIELD_VAR from django.utils.translation import gettext as _ from django.contrib.admin import helpers -from django.db.models import Model +from django.db.models import Model, ManyToManyField + from django.forms import ModelForm from admin_confirm.utils import snake_to_title_case @@ -119,18 +120,43 @@ class AdminConfirmMixin: default_value = model._meta.get_field(name).get_default() 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] + changed_data[name] = [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, 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() + # Note: getattr does not work on ManyToManyFields + field_object = model._meta.get_field(name) initial_value = getattr(obj, name) + if isinstance(field_object, ManyToManyField): + initial_value = field_object.value_to_string(obj) + if initial_value != new_value: changed_data[name] = [initial_value, new_value] + + print(changed_data) 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 to_field = request.POST.get(TO_FIELD_VAR, request.GET.get(TO_FIELD_VAR)) @@ -174,18 +200,14 @@ class AdminConfirmMixin: return super()._changeform_view(request, object_id, form_url, extra_context) # Parse raw form data from POST - form_data = {} + form_data = self._get_form_data(request) # Parse the original save action from request save_action = None - for key, value in request.POST.items(): + for key in request.POST.keys(): if key in SAVE_ACTIONS: save_action = key - continue + break - if key.startswith("_") or key == "csrfmiddlewaretoken": - continue - - form_data[key] = value title_action = _("adding") if add else _("changing") diff --git a/admin_confirm/templates/admin/change_confirmation.html b/admin_confirm/templates/admin/change_confirmation.html index 99b3e25..dc2adee 100644 --- a/admin_confirm/templates/admin/change_confirmation.html +++ b/admin_confirm/templates/admin/change_confirmation.html @@ -43,8 +43,10 @@
{% csrf_token %} {% endif %} - {% for key, value in form_data.items %} - + {% for key, values in form_data %} + {% for v in values %} + + {% endfor %} {% endfor %} {% if is_popup %}{% endif %} {% if to_field %}{% endif %} diff --git a/admin_confirm/templates/admin/change_data.html b/admin_confirm/templates/admin/change_data.html index efd4b73..da5d220 100644 --- a/admin_confirm/templates/admin/change_data.html +++ b/admin_confirm/templates/admin/change_data.html @@ -1,4 +1,4 @@ -{% if changed_data %} +{% load formatting %} {% if changed_data %}

Confirm Values:

@@ -10,8 +10,8 @@ {% for field, values in changed_data.items %} - - + + {% endfor %}
{{ field }}{{ values.0 }}{{ values.1 }}{{ values.0|format_change_data_field_value }}{{ values.1|format_change_data_field_value }}
diff --git a/admin_confirm/templates/admin/submit_line.html b/admin_confirm/templates/admin/submit_line.html index e25abb1..75d4893 100644 --- a/admin_confirm/templates/admin/submit_line.html +++ b/admin_confirm/templates/admin/submit_line.html @@ -3,10 +3,10 @@ {% block submit-row %} {% if confirm_change %} - + {% endif %} {% if confirm_add %} - + {% endif %} {{ block.super }} {% endblock %} diff --git a/admin_confirm/templatetags/__init__.py b/admin_confirm/templatetags/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/admin_confirm/templatetags/formatting.py b/admin_confirm/templatetags/formatting.py new file mode 100644 index 0000000..ab71826 --- /dev/null +++ b/admin_confirm/templatetags/formatting.py @@ -0,0 +1,12 @@ +from django import template +from django.db.models.query import QuerySet + +register = template.Library() + + +@register.filter +def format_change_data_field_value(field_value): + if isinstance(field_value, QuerySet): + return list(field_value) + + return field_value diff --git a/admin_confirm/tests/test_confirm_change_and_add.py b/admin_confirm/tests/test_confirm_change_and_add.py index 1d4dc96..1255651 100644 --- a/admin_confirm/tests/test_confirm_change_and_add.py +++ b/admin_confirm/tests/test_confirm_change_and_add.py @@ -64,7 +64,6 @@ class TestConfirmChangeAndAdd(TestCase): ] self.assertEqual(response.template_name, expected_templates) form_data = {"shop": str(shop.id), "item": str(item.id), "quantity": str(5)} - self.assertEqual(response.context_data["form_data"], form_data) for k, v in form_data.items(): self.assertIn( f'', @@ -100,7 +99,6 @@ class TestConfirmChangeAndAdd(TestCase): "id": str(item.id), "currency": Item.VALID_CURRENCIES[0][0], } - self.assertEqual(response.context_data["form_data"], form_data) for k, v in form_data.items(): self.assertIn( f'', diff --git a/admin_confirm/tests/test_confirm_change_and_add_m2m_field.py b/admin_confirm/tests/test_confirm_change_and_add_m2m_field.py new file mode 100644 index 0000000..c2a0106 --- /dev/null +++ b/admin_confirm/tests/test_confirm_change_and_add_m2m_field.py @@ -0,0 +1,183 @@ +from django.test import TestCase, RequestFactory +from django.contrib.auth.models import User +from django.urls import reverse + + +from tests.market.admin import InventoryAdmin, ShoppingMallAdmin +from tests.market.models import Item, Inventory, ShoppingMall +from tests.factories import ItemFactory, ShopFactory, InventoryFactory + + +class TestConfirmChangeAndAddM2MField(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 test_post_add_without_confirm_add_m2m(self): + shops = [ShopFactory() for i in range(3)] + + data = {"name": "name", "shops": [s.id for s in shops]} + response = self.client.post(reverse("admin:market_shoppingmall_add"), data) + # Redirects to changelist and is added + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/admin/market/shoppingmall/") + self.assertEqual(ShoppingMall.objects.count(), 1) + + def test_post_add_with_confirm_add_m2m(self): + ShoppingMallAdmin.confirmation_fields = ["shops"] + shops = [ShopFactory() for i in range(3)] + + data = {"name": "name", "shops": [s.id for s in shops], "_confirm_add": True} + response = self.client.post(reverse("admin:market_shoppingmall_add"), data) + + # Ensure not redirected (confirmation page does not redirect) + self.assertEqual(response.status_code, 200) + expected_templates = [ + "admin/market/shoppingmall/change_confirmation.html", + "admin/market/change_confirmation.html", + "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.assertIn( + f'', + response.rendered_content, + ) + + # Should not have been added yet + self.assertEqual(ShoppingMall.objects.count(), 0) + + def test_m2m_field_post_change_with_confirm_change(self): + shops = [ShopFactory() for i in range(10)] + shopping_mall = ShoppingMall.objects.create(name="My Mall") + shopping_mall.shops.set(shops) + # Currently ShoppingMall configured with confirmation_fields = ['name'] + data = { + "name": "Not My Mall", + "shops": "1", + "id": shopping_mall.id, + "_confirm_change": True, + "csrfmiddlewaretoken": "fake token", + "_save": True, + } + response = self.client.post( + f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data + ) + # Ensure not redirected (confirmation page does not redirect) + self.assertEqual(response.status_code, 200) + expected_templates = [ + "admin/market/shoppingmall/change_confirmation.html", + "admin/market/change_confirmation.html", + "admin/change_confirmation.html", + ] + self.assertEqual(response.template_name, expected_templates) + form_data = { + "name": "Not My Mall", + "shops": "1", + "id": str(shopping_mall.id), + } + + for k, v in form_data.items(): + self.assertIn( + f'', + response.rendered_content, + ) + + # Hasn't changed item yet + shopping_mall.refresh_from_db() + self.assertEqual(shopping_mall.name, "My Mall") + + def test_m2m_field_post_change_with_confirm_change_multiple_selected(self): + shops = [ShopFactory() for i in range(10)] + shopping_mall = ShoppingMall.objects.create(name="My Mall") + shopping_mall.shops.set(shops) + # Currently ShoppingMall configured with confirmation_fields = ['name'] + data = { + "name": "Not My Mall", + "shops": ["1", "2", "3"], + "id": shopping_mall.id, + "_confirm_change": True, + "csrfmiddlewaretoken": "fake token", + "_save": True, + } + response = self.client.post( + f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data + ) + # Ensure not redirected (confirmation page does not redirect) + self.assertEqual(response.status_code, 200) + expected_templates = [ + "admin/market/shoppingmall/change_confirmation.html", + "admin/market/change_confirmation.html", + "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'', + response.rendered_content, + ) + + # Hasn't changed item yet + shopping_mall.refresh_from_db() + self.assertEqual(shopping_mall.name, "My Mall") + + def test_post_change_without_confirm_change_m2m_value(self): + # make the m2m the confirmation_field + ShoppingMallAdmin.confirmation_fields = ["shops"] + shops = [ShopFactory() for i in range(3)] + shopping_mall = ShoppingMall.objects.create(name="name") + shopping_mall.shops.set(shops) + assert shopping_mall.shops.count() == 3 + + data = {"name": "name", "id": str(shopping_mall.id), "shops": ["1"]} + response = self.client.post( + f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data + ) + # Redirects to changelist + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, "/admin/market/shoppingmall/") + # Shop has changed + shopping_mall.refresh_from_db() + self.assertEqual(shopping_mall.shops.count(), 1) + + def test_form_invalid_m2m_value(self): + ShoppingMallAdmin.confirmation_fields = ["shops"] + shopping_mall = ShoppingMall.objects.create(name="name") + + data = { + "id": shopping_mall.id, + "name": "name", + "shops": ["1", "2", "3"], # These shops don't exist + "_confirm_change": True, + "csrfmiddlewaretoken": "fake token", + } + response = self.client.post( + f"/admin/market/shoppingmall/{shopping_mall.id}/change/", data + ) + + # Form invalid should show errors on form + self.assertEqual(response.status_code, 200) + self.assertIsNotNone(response.context_data.get("errors")) + self.assertTrue( + str(response.context_data["errors"][0][0]).startswith( + "Select a valid choice." + ) + ) + # Should not have updated inventory + shopping_mall.refresh_from_db() + self.assertEqual(shopping_mall.shops.count(), 0)