diff --git a/.coveragerc b/.coveragerc index 4edd7b1..3334c71 100644 --- a/.coveragerc +++ b/.coveragerc @@ -1,2 +1,4 @@ [run] relative_files = True +omit = admin_confirm/tests/* +branch = True diff --git a/.gitignore b/.gitignore index a695b50..0228584 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,5 @@ docs/_build/ # Database db.sqlite3 + +tmp/ diff --git a/Makefile b/Makefile index fb6b564..b358d1a 100644 --- a/Makefile +++ b/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 diff --git a/README.md b/README.md index a9231c8..d378c5f 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,12 @@ Typical Usage: confirmation_fields = ['field1', 'field2'] ``` +## Disclaimer + +Be aware that not all possible combinations of ModelAdmin have been tested, even if test coverage is high. + +See [testing readme](admin_confirm/tests/README.md) for more details + ## Installation Install django-admin-confirm by running: diff --git a/admin_confirm/admin.py b/admin_confirm/admin.py index 6c8ee56..9286bec 100644 --- a/admin_confirm/admin.py +++ b/admin_confirm/admin.py @@ -6,11 +6,23 @@ 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, ManyToManyField +from django.db.models import Model, ManyToManyField, FileField, ImageField from django.forms import ModelForm -from admin_confirm.utils import snake_to_title_case - -SAVE_ACTIONS = ["_save", "_saveasnew", "_addanother", "_continue"] +from admin_confirm.utils import get_admin_change_url, snake_to_title_case +from django.core.cache import cache +from django.views.decorators.cache import cache_control +from django.forms.formsets import all_valid +from admin_confirm.constants import ( + CACHE_TIMEOUT, + CONFIRMATION_RECEIVED, + CONFIRM_ADD, + CONFIRM_CHANGE, + SAVE, + SAVE_ACTIONS, + CACHE_KEYS, + SAVE_AND_CONTINUE, + SAVE_AS_NEW, +) class AdminConfirmMixin: @@ -34,7 +46,9 @@ class AdminConfirmMixin: if self.confirmation_fields is not None: return self.confirmation_fields - return flatten_fieldsets(self.get_fieldsets(request, obj)) + model_fields = set([field.name for field in self.model._meta.fields]) + admin_fields = set(flatten_fieldsets(self.get_fieldsets(request, obj))) + return list(model_fields & admin_fields) def render_change_confirmation(self, request, context): opts = self.model._meta @@ -81,14 +95,22 @@ class AdminConfirmMixin: context, ) + @cache_control(private=True) def changeform_view(self, request, object_id=None, form_url="", extra_context=None): if request.method == "POST": - if (not object_id and "_confirm_add" in request.POST) or ( - object_id and "_confirm_change" in request.POST + if (not object_id and CONFIRM_ADD in request.POST) or ( + object_id and CONFIRM_CHANGE in request.POST ): + cache.delete_many(CACHE_KEYS.values()) return self._change_confirmation_view( request, object_id, form_url, extra_context ) + elif CONFIRMATION_RECEIVED in request.POST: + return self._confirmation_received_view( + request, object_id, form_url, extra_context + ) + else: + cache.delete_many(CACHE_KEYS.values()) extra_context = { **(extra_context or {}), @@ -111,32 +133,160 @@ class AdminConfirmMixin: Returns a dictionary of the fields and their changed values if any """ - changed_data = {} - if form.is_valid(): - if add: - 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: - # Show what the default value is - 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_from_object(obj) - if initial_value != new_value: - changed_data[name] = [initial_value, new_value] + def _display_for_changed_data(field, initial_value, new_value): + if not (isinstance(field, FileField) or isinstance(field, ImageField)): + return [initial_value, new_value] + + if initial_value: + if new_value == False: + # Clear has been selected + return [initial_value.name, None] + elif new_value: + return [initial_value.name, new_value.name] + else: + # No cover: Technically doesn't get called in current code because + # This function is only called if there was a difference in the data + return [initial_value.name, initial_value.name] # pragma: no cover + + if new_value: + return [None, new_value.name] + + return [None, None] + + changed_data = {} + if add: + for name, new_value in form.cleaned_data.items(): + # Don't consider default values as changed for adding + field_object = model._meta.get_field(name) + default_value = field_object.get_default() + if new_value is not None and new_value != default_value: + # Show what the default value is + changed_data[name] = _display_for_changed_data( + field_object, 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() + + field_object = model._meta.get_field(name) + initial_value = getattr(obj, name) + + # Note: getattr does not work on ManyToManyFields + if isinstance(field_object, ManyToManyField): + initial_value = field_object.value_from_object(obj) + + if initial_value != new_value: + changed_data[name] = _display_for_changed_data( + field_object, initial_value, new_value + ) return changed_data + def _confirmation_received_view(self, request, object_id, form_url, extra_context): + """ + When the form is a multipart form, the object and POST are cached + This is required because file(s) cannot be programmically uploaded + ie. There is no way to set a file on the html form + + If the form isn't multipart, this function would not be called. + If there are no file changes, do nothing to the request and send to Django. + + If there are files uploaded, save the files from cached object to either: + - the object instance if already exists + - or save the new object and modify the request from `add` to `change` + and pass the request to Django + """ + + def _reconstruct_request_files(): + """ + Reconstruct the file(s) from the cached object (if any). + Returns a dictionary of field name to cached file + """ + reconstructed_files = {} + + cached_object = cache.get(CACHE_KEYS["object"]) + query_dict = cache.get(CACHE_KEYS["post"]) + # Reconstruct the files from cached object + if not cached_object: + return + + if not query_dict: + # Use the current POST, since it should mirror cached POST + query_dict = request.POST + + if type(cached_object) != self.model: + # Do not use cache if the model doesn't match this model + return + + for field in self.model._meta.get_fields(): + if not (isinstance(field, FileField) or isinstance(field, ImageField)): + continue + + cached_file = getattr(cached_object, field.name) + # If a file was uploaded, the field is omitted from the POST since it's in request.FILES + if not query_dict.get(field.name) and cached_file: + reconstructed_files[field.name] = cached_file + + return reconstructed_files + + reconstructed_files = _reconstruct_request_files() + if reconstructed_files: + obj = None + + # remove the _confirm_add and _confirm_change from post + modified_post = request.POST.copy() + cached_post = cache.get(CACHE_KEYS["post"]) + # No cover: __reconstruct_request_files currently checks for cached post so cached_post won't be None + if cached_post: # pragma: no cover + modified_post = cached_post.copy() + if CONFIRM_ADD in modified_post: + del modified_post[CONFIRM_ADD] + if CONFIRM_CHANGE in modified_post: + del modified_post[CONFIRM_CHANGE] + + if object_id and not SAVE_AS_NEW in request.POST: + # Update the obj with the new uploaded files + # then pass rest of changes to Django + obj = self.model.objects.filter(id=object_id).first() + else: + # Create the obj and pass the rest as changes to Django + # (Since we are not handling the formsets/inlines) + # Note that this results in the "Yes, I'm Sure" submission + # act as a `change` not an `add` + obj = cache.get(CACHE_KEYS["object"]) + + # No cover: __reconstruct_request_files currently checks for cached obj so obj won't be None + if obj: # pragma: no cover + for field, file in reconstructed_files.items(): + setattr(obj, field, file) + obj.save() + object_id = str(obj.id) + # Update the request path, used in the message to user and redirect + # Used in `self.response_change` + request.path = get_admin_change_url(obj) + + if SAVE_AS_NEW in request.POST: + # We have already saved the new object + # So change action to _continue + del modified_post[SAVE_AS_NEW] + if self.save_as_continue: + modified_post[SAVE_AND_CONTINUE] = True + else: + modified_post[SAVE] = True + if "id" in modified_post: + del modified_post["id"] + modified_post["id"] = object_id + + request.POST = modified_post + + cache.delete_many(CACHE_KEYS.values()) + return super()._changeform_view(request, object_id, form_url, extra_context) + 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 @@ -169,13 +319,19 @@ 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 - + form_validated = form.is_valid() + if form_validated: + new_object = self.save_form(request, form, change=not add) + else: + new_object = form.instance + formsets, inline_instances = self._create_formsets( + request, new_object, change=not add + ) # End code from super()._changeform_view + add_or_new = add or SAVE_AS_NEW in request.POST # Get changed data to show on confirmation - changed_data = self._get_changed_data(form, model, obj, add) + changed_data = self._get_changed_data(form, model, obj, add_or_new) changed_confirmation_fields = set( self.get_confirmation_fields(request, obj) @@ -186,13 +342,17 @@ class AdminConfirmMixin: # Parse the original save action from request save_action = None - for key in request.POST.keys(): + # No cover: There would not be a case of not request.POST.keys() and form is valid + for key in request.POST.keys(): # pragma: no cover if key in SAVE_ACTIONS: save_action = key break - title_action = _("adding") if add else _("changing") + if form.is_multipart(): + cache.set(CACHE_KEYS["post"], request.POST, timeout=CACHE_TIMEOUT) + cache.set(CACHE_KEYS["object"], new_object, timeout=CACHE_TIMEOUT) + title_action = _("adding") if add_or_new else _("changing") context = { **self.admin_site.each_context(request), "preserved_filters": self.get_preserved_filters(request), @@ -205,8 +365,10 @@ class AdminConfirmMixin: "opts": opts, "changed_data": changed_data, "add": add, + "save_as_new": SAVE_AS_NEW in request.POST, "submit_name": save_action, "form": form, + "formsets": formsets, **(extra_context or {}), } return self.render_change_confirmation(request, context) diff --git a/admin_confirm/constants.py b/admin_confirm/constants.py new file mode 100644 index 0000000..b062cd4 --- /dev/null +++ b/admin_confirm/constants.py @@ -0,0 +1,17 @@ +from django.conf import settings + +SAVE = "_save" +SAVE_AS_NEW = "_saveasnew" +ADD_ANOTHER = "_addanother" +SAVE_AND_CONTINUE = "_continue" +SAVE_ACTIONS = [SAVE, SAVE_AS_NEW, ADD_ANOTHER, SAVE_AND_CONTINUE] + +CONFIRM_ADD = "_confirm_add" +CONFIRM_CHANGE = "_confirm_change" +CONFIRMATION_RECEIVED = "_confirmation_received" + +CACHE_TIMEOUT = getattr(settings, "ADMIN_CONFIRM_CACHE_TIMEOUT", 10) +CACHE_KEYS = { + "object": "admin_confirm__confirmation_object", + "post": "admin_confirm__confirmation_request_post", +} diff --git a/admin_confirm/templates/admin/change_confirmation.html b/admin_confirm/templates/admin/change_confirmation.html index 6f123f3..da06731 100644 --- a/admin_confirm/templates/admin/change_confirmation.html +++ b/admin_confirm/templates/admin/change_confirmation.html @@ -31,27 +31,29 @@ {% block content %} - {% if add %} -

{% blocktrans with escaped_object=object %}Are you sure you want to add the {{ model_name }}?{% endblocktrans %}

- {% include "admin/change_data.html" %} -
{% csrf_token %} + {% if add or save_as_new %} +

{% blocktrans with escaped_object=object %}Are you sure you want to add the {{ model_name }}?{% endblocktrans %}

+ {% else %} +

{% blocktrans with escaped_object=object %}Are you sure you want to change the {{ model_name }} "{{ object_name }}"?{% endblocktrans %}

+ {% endif %} - {% else %} - -

{% blocktrans with escaped_object=object %}Are you sure you want to change the {{ model_name }} "{{ object_name }}"?{% endblocktrans %}

{% include "admin/change_data.html" %} - {% csrf_token %} - {% endif %} - - {% if is_popup %}{% endif %} - {% if to_field %}{% endif %} -
- -

- {% trans "No, continue to edit" %} -

-
-
+ +
{% csrf_token %} + + {% if is_popup %}{% endif %} + {% if to_field %}{% endif %} + {% if form.is_multipart %}{% endif %} +
+ +

+ {% trans "No, continue to edit" %} +

+
+
{% endblock %} diff --git a/admin_confirm/tests/README.md b/admin_confirm/tests/README.md new file mode 100644 index 0000000..bb7626e --- /dev/null +++ b/admin_confirm/tests/README.md @@ -0,0 +1,100 @@ +# Testing Documentation/Notes + +[![Coverage Status](https://coveralls.io/repos/github/TrangPham/django-admin-confirm/badge.svg)](https://coveralls.io/github/TrangPham/django-admin-confirm) + +Hello, friend! You have found the list of test cases that this package can benefit from. + +You seem concerned about the stability and reliability of this package. You're probably wondering if you should include it in your production codebase. Well, although I have tried very hard to get 100% code coverage, there are so many permutations of ModelAdmins in the wild. And I'm only one person. + +So if you want to include this package in your production codebase, be aware that AdminConfirmMixin works best with simple unmodified ModelAdmins. + +## Save Options + +- [x] Save +- [x] Conitnue +- [x] Save As New +- [x] Add another + +### Field types + +- [x] CharField +- [x] PositiveIntegerField +- [x] DecimalField +- [x] TextField +- [x] ImageField +- [x] FileField +- [x] ManyToManyField +- [x] OneToOneField +- [x] ForeignKey + +- [x] Custom Readonly fields + +### Options + +- [x] .exclude +- [x] .fields +- [x] .readonly_fields +- [x] Actions + +### Options to test + +- [x] ModelAdmin.fieldsets +- [ ] ModelAdmin.form +- [ ] ModelAdmin.raw_id_fields +- [ ] ModelAdmin.radio_fields +- [ ] ModelAdmin.autocomplete_fields +- [ ] ModelAdmin.prepopulated_fields + +## ModelAdmin form template overrides? + +https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#custom-template-options +(Maybe??? IDK this is esoteric) + +## Function overrides to test + +- [ ] .save_model() +- [ ] .get_readonly_fields() +- [ ] .get_fields() +- [ ] .get_excludes() +- [ ] .get_form() +- [ ] .get_autocomplete_fields() +- [ ] .get_prepopulated_fields() +- [x] .get_fieldsets() +- [ ] ModelAdmin.formfield_for_manytomany() +- [ ] ModelAdmin.formfield_for_foreignkey() +- [ ] ModelAdmin.formfield_for_choice_field() +- [ ] ModelAdmin.get_changeform_initial_data() + +## Inline instance support?? + +Confirmation on inline changes is not a current feature of this project. + +Confirmation on add/change of ModelAdmin that includes inlines needs to be tested. Use AdminConfirmMixin with ModelAdmin containing inlines at your own risk. + +- [ ] .inlines +- [ ] .get_inline_instances() +- [ ] .get_inlines() (New in Django 3.0) +- [ ] .get_formsets_with_inlines() + +#### Options for inlines + +- [ ] classes of inlines: Tabular, Stacked, etc +- [ ] extra +- [ ] action on the inline: add or change +- [ ] clicking add another on the inline + +## IDK if we want to support these + +- [ ] .get_changelist_form() +- [ ] ModelAdmin.list_editable +- [ ] ModelAdmin.changelist_view() + +- [ ] ModelAdmin.add_view(request, form_url='', extra_context=None) +- [ ] ModelAdmin.change_view(request, object_id, form_url='', extra_context=None) + +## More tests for these? + +Note: Currently the code always calls super().\_changeform_view(), which would ensure permissions correct as well + +- [x] ModelAdmin.has_add_permission +- [x] ModelAdmin.has_change_permission diff --git a/admin_confirm/tests/helpers.py b/admin_confirm/tests/helpers.py index 7dc2f0d..7ec89a9 100644 --- a/admin_confirm/tests/helpers.py +++ b/admin_confirm/tests/helpers.py @@ -1,8 +1,13 @@ +from django.core.cache import cache from django.test import TestCase, RequestFactory from django.contrib.auth.models import User -class ConfirmAdminTestCase(TestCase): +class AdminConfirmTestCase(TestCase): + """ + Helper TestCase class and common associated assertions + """ + @classmethod def setUpTestData(cls): cls.superuser = User.objects.create_superuser( @@ -10,6 +15,7 @@ class ConfirmAdminTestCase(TestCase): ) def setUp(self): + cache.clear() self.client.force_login(self.superuser) self.factory = RequestFactory() @@ -24,7 +30,9 @@ class ConfirmAdminTestCase(TestCase): # ManyToManyField should be embedded self.assertIn("related-widget-wrapper", rendered_content) - def _assertSubmitHtml(self, rendered_content, save_action="_save"): + def _assertSubmitHtml( + self, rendered_content, save_action="_save", multipart_form=False + ): # Submit should conserve the save action self.assertIn( f'', @@ -34,6 +42,16 @@ class ConfirmAdminTestCase(TestCase): self.assertNotIn("_confirm_add", rendered_content) self.assertNotIn("_confirm_change", rendered_content) + confirmation_received_html = ( + '' + ) + + if multipart_form: + # Should have _confirmation_received as a hidden field + self.assertIn(confirmation_received_html, rendered_content) + else: + self.assertNotIn(confirmation_received_html, rendered_content) + def _assertSimpleFieldFormHtml(self, rendered_content, fields): for k, v in fields.items(): self.assertIn(f'name="{k}"', rendered_content) diff --git a/admin_confirm/tests/unit/test_admin_options.py b/admin_confirm/tests/unit/test_admin_options.py new file mode 100644 index 0000000..ff9b808 --- /dev/null +++ b/admin_confirm/tests/unit/test_admin_options.py @@ -0,0 +1,307 @@ +from unittest import mock +from django.core.cache import cache + +from admin_confirm.tests.helpers import AdminConfirmTestCase +from tests.market.admin import ShoppingMallAdmin +from tests.market.models import GeneralManager, ShoppingMall, Town +from tests.factories import ShopFactory + +from admin_confirm.constants import CACHE_KEYS, CONFIRMATION_RECEIVED + + +@mock.patch.object(ShoppingMallAdmin, "inlines", []) +class TestAdminOptions(AdminConfirmTestCase): + @mock.patch.object(ShoppingMallAdmin, "confirmation_fields", ["name"]) + @mock.patch.object(ShoppingMallAdmin, "fields", ["name", "town"]) + def test_change_model_with_m2m_field_without_input_for_m2m_field_should_work(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 not have cached the unsaved obj + cached_item = cache.get(CACHE_KEYS["object"]) + self.assertIsNone(cached_item) + + # 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"] + + 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_when_m2m_field_in_exclude_changes_to_field_should_not_be_saved(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 not have cached the unsaved obj + cached_item = cache.get(CACHE_KEYS["object"]) + self.assertIsNone(cached_item) + + # 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"] + + 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"]) + @mock.patch.object(ShoppingMallAdmin, "inlines", []) + def test_if_confirmation_fields_in_exclude_should_not_trigger_confirmation(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) + + @mock.patch.object(ShoppingMallAdmin, "confirmation_fields", ["name"]) + @mock.patch.object(ShoppingMallAdmin, "readonly_fields", ["shops", "name"]) + @mock.patch.object(ShoppingMallAdmin, "inlines", []) + def test_if_confirmation_fields_in_readonly_should_not_trigger_confirmation(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) + + @mock.patch.object(ShoppingMallAdmin, "confirmation_fields", ["name"]) + @mock.patch.object(ShoppingMallAdmin, "readonly_fields", ["shops"]) + @mock.patch.object(ShoppingMallAdmin, "inlines", []) + def test_readonly_fields_should_not_change(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 not have cached the unsaved obj + cached_item = cache.get(CACHE_KEYS["object"]) + self.assertIsNone(cached_item) + + # 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"] + + 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)) diff --git a/admin_confirm/tests/test_confirm_actions.py b/admin_confirm/tests/unit/test_confirm_actions.py similarity index 100% rename from admin_confirm/tests/test_confirm_actions.py rename to admin_confirm/tests/unit/test_confirm_actions.py diff --git a/admin_confirm/tests/test_confirm_change_and_add.py b/admin_confirm/tests/unit/test_confirm_change_and_add.py similarity index 84% rename from admin_confirm/tests/test_confirm_change_and_add.py rename to admin_confirm/tests/unit/test_confirm_change_and_add.py index 46498fe..53e153b 100644 --- a/admin_confirm/tests/test_confirm_change_and_add.py +++ b/admin_confirm/tests/unit/test_confirm_change_and_add.py @@ -1,17 +1,20 @@ +from unittest import mock 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 admin_confirm.tests.helpers import AdminConfirmTestCase +from tests.market.admin import ItemAdmin, InventoryAdmin, ShoppingMallAdmin +from tests.market.models import Item, Inventory, ShoppingMall from tests.factories import ItemFactory, ShopFactory, InventoryFactory -class TestConfirmChangeAndAdd(ConfirmAdminTestCase): +@mock.patch.object(ShoppingMallAdmin, "inlines", []) +class TestConfirmChangeAndAdd(AdminConfirmTestCase): def test_get_add_without_confirm_add(self): + ItemAdmin.confirm_add = False response = self.client.get(reverse("admin:market_item_add")) self.assertFalse(response.context_data.get("confirm_add")) self.assertNotIn("_confirm_add", response.rendered_content) @@ -70,6 +73,36 @@ class TestConfirmChangeAndAdd(ConfirmAdminTestCase): # Should not have been added yet self.assertEqual(Inventory.objects.count(), 0) + def test_post_change_with_confirm_change_shoppingmall_name(self): + # When testing found that even though name was in confirmation_fields + # When only name changed, `form.is_valid` = False, and thus didn't trigger + # confirmation page previously, even though it should have + + mall = ShoppingMall.objects.create(name="name") + data = { + "id": mall.id, + "name": "new name", + "_confirm_change": True, + "csrfmiddlewaretoken": "fake token", + "_save": True, + } + response = self.client.post( + f"/admin/market/shoppingmall/{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) + self._assertSubmitHtml(rendered_content=response.rendered_content) + + # Hasn't changed item yet + mall.refresh_from_db() + self.assertEqual(mall.name, "name") + def test_post_change_with_confirm_change(self): item = ItemFactory(name="item") data = { @@ -100,7 +133,9 @@ class TestConfirmChangeAndAdd(ConfirmAdminTestCase): self._assertSimpleFieldFormHtml( rendered_content=response.rendered_content, fields=form_data ) - self._assertSubmitHtml(rendered_content=response.rendered_content) + self._assertSubmitHtml( + rendered_content=response.rendered_content, multipart_form=True + ) # Hasn't changed item yet item.refresh_from_db() @@ -120,9 +155,22 @@ class TestConfirmChangeAndAdd(ConfirmAdminTestCase): def test_get_confirmation_fields_should_default_if_not_set(self): expected_fields = [f.name for f in Item._meta.fields if f.name != "id"] ItemAdmin.confirmation_fields = None + ItemAdmin.fields = expected_fields admin = ItemAdmin(Item, AdminSite()) actual_fields = admin.get_confirmation_fields(self.factory.request()) - self.assertEqual(expected_fields, actual_fields) + for field in expected_fields: + self.assertIn(field, actual_fields) + + def test_get_confirmation_fields_default_should_only_include_fields_shown_on_admin( + self, + ): + admin_fields = ["name", "price"] + ItemAdmin.confirmation_fields = None + ItemAdmin.fields = admin_fields + admin = ItemAdmin(Item, AdminSite()) + actual_fields = admin.get_confirmation_fields(self.factory.request()) + for field in admin_fields: + self.assertIn(field, actual_fields) def test_get_confirmation_fields_if_set(self): expected_fields = ["name", "currency"] diff --git a/admin_confirm/tests/test_confirm_change_and_add_m2m_field.py b/admin_confirm/tests/unit/test_confirm_change_and_add_m2m_field.py similarity index 95% rename from admin_confirm/tests/test_confirm_change_and_add_m2m_field.py rename to admin_confirm/tests/unit/test_confirm_change_and_add_m2m_field.py index 6387162..e564c94 100644 --- a/admin_confirm/tests/test_confirm_change_and_add_m2m_field.py +++ b/admin_confirm/tests/unit/test_confirm_change_and_add_m2m_field.py @@ -1,12 +1,15 @@ +from unittest import mock +from admin_confirm.admin import AdminConfirmMixin from django.urls import reverse -from admin_confirm.tests.helpers import ConfirmAdminTestCase +from admin_confirm.tests.helpers import AdminConfirmTestCase from tests.market.admin import ShoppingMallAdmin from tests.market.models import ShoppingMall from tests.factories import ShopFactory -class TestConfirmChangeAndAddM2MField(ConfirmAdminTestCase): +@mock.patch.object(ShoppingMallAdmin, "inlines", []) +class TestConfirmChangeAndAddM2MField(AdminConfirmTestCase): def test_post_add_without_confirm_add_m2m(self): shops = [ShopFactory() for i in range(3)] @@ -84,6 +87,9 @@ class TestConfirmChangeAndAddM2MField(ConfirmAdminTestCase): ] self.assertEqual(response.template_name, expected_templates) + # Should show two lists for the m2m current and modified values + self.assertEqual(response.rendered_content.count("