diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..32c8f3a --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,4 @@ +{ + "python.formatting.provider": "black", + "editor.formatOnSave": true +} \ No newline at end of file diff --git a/Makefile b/Makefile index e770a5c..c541d1b 100644 --- a/Makefile +++ b/Makefile @@ -7,12 +7,18 @@ test: coverage-badge -f -o coverage.svg python -m readme_renderer README.md -o /tmp/README.html +check-readme: + python -m readme_renderer README.md -o /tmp/README.html + migrate: ./tests/manage.py makemigrations ./tests/manage.py migrate +shell: + ./tests/manage.py shell + package: python3 setup.py sdist bdist_wheel upload-testpypi: - python3 -m twine upload --repository testpypi dist/* + python3 -m twine upload --repository testpypi dist/django_admin_confirm-$(VERSION)* diff --git a/README.md b/README.md index fb9e4e8..23123c0 100644 --- a/README.md +++ b/README.md @@ -2,11 +2,15 @@ ![coverage](https://raw.githubusercontent.com/TrangPham/django-admin-confirm/main/coverage.svg) -AdminConfirmMixin is a mixin for ModelAdmin to add confirmations to changes and additions. +AdminConfirmMixin is a mixin for ModelAdmin to add confirmations to change, add and actions. -![Screenshot of Confirmation Page](https://raw.githubusercontent.com/TrangPham/django-admin-confirm/main/screenshot.png) +![Screenshot of Change Confirmation Page](https://raw.githubusercontent.com/TrangPham/django-admin-confirm/main/screenshot_confirm_change.png) -It can be configured to add a confirmation page upon saving changes and/or additions on ModelAdmin. +It can be configured to add a confirmation page on ModelAdmin upon: + +- saving changes +- adding new instances +- performing actions Typical Usage: @@ -39,12 +43,74 @@ To override a template, your app should be listed before `admin_confirm` in INST ## Configuration Options +**Attributes:** + - `confirm_change` _Optional[bool]_ - decides if changes should trigger confirmation - `confirm_add` _Optional[bool]_ - decides if additions should trigger confirmation -- `confirmation_fields` _Optional[Array[string]]_ - sets which fields changes should trigger confirmation -- `change_confirmation_template` _Optional[string]_ - path to custom html template to use +- `confirmation_fields` _Optional[Array[string]]_ - sets which fields should trigger confirmation for add/change. For adding new instances, the field would only trigger a confirmation if it's set to a value that's not its default. +- `change_confirmation_template` _Optional[string]_ - path to custom html template to use for change/add +- `action_confirmation_template` _Optional[string]_ - path to custom html template to use for actions -Note that setting `confirmation_fields` without setting `confirm_change` or `confirm_add` would not trigger confirmation. +Note that setting `confirmation_fields` without setting `confirm_change` or `confirm_add` would not trigger confirmation for change/add. Confirmations for actions does not use the `confirmation_fields` option. + +**Method Overrides:** +If you want even more control over the confirmation, these methods can be overridden: + +- `get_confirmation_fields(self, request: HttpRequest, obj: Optional[Object]) -> List[str]` +- `render_change_confirmation(self, request: HttpRequest, context: dict) -> TemplateResponse` +- `render_action_confirmation(self, request: HttpRequest, context: dict) -> TemplateResponse` + +## Usage + +**Confirm Change:** + +```py + from admin_confirm import AdminConfirmMixin + + class MyModelAdmin(AdminConfirmMixin, ModelAdmin): + confirm_change = True + confirmation_fields = ['field1', 'field2'] +``` + +This would confirm changes on changes that include modifications on`field1` and/or `field2`. + +**Confirm Add:** + +```py + from admin_confirm import AdminConfirmMixin + + class MyModelAdmin(AdminConfirmMixin, ModelAdmin): + confirm_add = True + confirmation_fields = ['field1', 'field2'] +``` + +This would confirm add on adds that set `field1` and/or `field2` to a non default value. + +Note: `confirmation_fields` apply to both add/change confirmations. + +**Confirm Action:** + +```py + from admin_confirm import AdminConfirmMixin + + class MyModelAdmin(AdminConfirmMixin, ModelAdmin): + actions = ["action1", "action2"] + + def action1(modeladmin, request, queryset): + # Do something with the queryset + + @confirm_action + def action2(modeladmin, request, queryset): + # Do something with the queryset + + action2.allowed_permissions = ('change',) +``` + +This would confirm `action2` but not `action1`. + +![Screenshot of Action Confirmation Page](https://raw.githubusercontent.com/TrangPham/django-admin-confirm/main/screenshot_confirm_action.png) + +Action confirmation will respect `allowed_permissions` and the `has_xxx_permission` methods. ## Contribution & Appreciation @@ -64,7 +130,7 @@ Your appreciation is also very welcome :) Feel free to: 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. -- [ ] confirmations on changelist actions +- [x] confirmations on changelist actions - [ ] global actions on changelist page - [ ] instance actions on change/view page - [ ] action logs (adding actions to history of instances) diff --git a/admin_confirm/admin.py b/admin_confirm/admin.py index c70ada6..83f0bef 100644 --- a/admin_confirm/admin.py +++ b/admin_confirm/admin.py @@ -4,6 +4,8 @@ from django.core.exceptions import PermissionDenied 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 admin_confirm.utils import snake_to_title_case class AdminConfirmMixin: @@ -17,7 +19,8 @@ class AdminConfirmMixin: confirmation_fields = None # Custom templates (designed to be over-ridden in subclasses) - confirmation_template = None + change_confirmation_template = None + action_confirmation_template = None def get_confirmation_fields(self, request, obj=None): """ @@ -39,7 +42,7 @@ class AdminConfirmMixin: return TemplateResponse( request, - self.confirmation_template + self.change_confirmation_template or [ "admin/{}/{}/change_confirmation.html".format( app_label, opts.model_name @@ -50,6 +53,29 @@ class AdminConfirmMixin: context, ) + def render_action_confirmation(self, request, context): + opts = self.model._meta + app_label = opts.app_label + + request.current_app = self.admin_site.name + context.update( + media=self.media, + opts=opts, + ) + + return TemplateResponse( + request, + self.action_confirmation_template + or [ + "admin/{}/{}/action_confirmation.html".format( + app_label, opts.model_name + ), + "admin/{}/action_confirmation.html".format(app_label), + "admin/action_confirmation.html", + ], + context, + ) + 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 ( @@ -168,3 +194,40 @@ class AdminConfirmMixin: **(extra_context or {}), } return self.render_change_confirmation(request, context) + + +def confirm_action(func): + """ + @confirm_action function wrapper for Django ModelAdmin actions + Will redirect to a confirmation page to ask for confirmation + + Next, it would call the action if confirmed. Otherwise, it would + return to the changelist without performing action. + """ + + def func_wrapper(modeladmin, request, queryset): + # First called by `Go` which would not have confirm_action in params + if request.POST.get("_confirm_action"): + return func(modeladmin, request, queryset) + + # get_actions will only return the actions that are allowed + has_perm = modeladmin.get_actions(request).get(func.__name__) is not None + + action_display_name = snake_to_title_case(func.__name__) + title = f"Confirm Action: {action_display_name}" + + context = { + **modeladmin.admin_site.each_context(request), + "title": title, + "queryset": queryset, + "has_perm": has_perm, + "action": func.__name__, + "action_display_name": action_display_name, + "action_checkbox_name": helpers.ACTION_CHECKBOX_NAME, + "submit_name": "confirm_action", + } + + # Display confirmation page + return modeladmin.render_action_confirmation(request, context) + + return func_wrapper diff --git a/admin_confirm/templates/admin/action_confirmation.html b/admin_confirm/templates/admin/action_confirmation.html new file mode 100644 index 0000000..b3a24b7 --- /dev/null +++ b/admin_confirm/templates/admin/action_confirmation.html @@ -0,0 +1,56 @@ +{% extends "admin/base_site.html" %} +{% load i18n l10n admin_urls static %} + +{% block extrahead %} + {{ block.super }} + {{ media }} + +{% endblock %} + +{% block extrastyle %} + {{ block.super }} + + +{% endblock %} + +{% block bodyclass %}{{ block.super }} app-{{ opts.app_label }} model-{{ opts.model_name }} change-confirmation{% endblock %} + +{% block breadcrumbs %} +
+{% trans 'Home' %} +› {{ opts.app_config.verbose_name }} +› {{ opts.verbose_name_plural|capfirst }} +› {% trans 'Confirm Action' %} +
+{% endblock %} + +{% block content %} +{% if has_perm %} +

{% trans 'Are you sure you want to perform action' %} {{ action_display_name }} {% trans 'on the following' %} {{ opts.verbose_name_plural|capfirst }}?

+ +
{% csrf_token %} + {% for obj in queryset %} + + {% endfor %} + +
+ +

+ {% trans "No, go back" %} +

+
+
+{% else %} +

{% trans "You don't have permissions to perform action" %} {{ action_display_name }} {% trans 'on' %} {{ opts.verbose_name_plural|capfirst }}

+
+
+

+ {% trans "Go back" %} +

+
+{% endif %} +{% endblock %} diff --git a/admin_confirm/templates/admin/change_confirmation.html b/admin_confirm/templates/admin/change_confirmation.html index 9e6a183..99b3e25 100644 --- a/admin_confirm/templates/admin/change_confirmation.html +++ b/admin_confirm/templates/admin/change_confirmation.html @@ -30,35 +30,19 @@ {% endblock %} {% block content %} -{% if add %} -

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

- {% if changed_data %} -
-

Confirm Values:

- - {% for field, values in changed_data.items %} - - {% endfor %} -
{{ field }}:{{ values.1 }}
-
-
{% csrf_token %} + + {% 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 %} + + {% 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 %} -{% else %} -

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

- {% if changed_data %} -
-

Confirm Values:

- - - {% for field, values in changed_data.items %} - - {% endfor %} -
FieldCurrent ValueNew Value
{{ field }}{{ values.0 }}{{ values.1 }}
-
- {% endif %} - {% csrf_token %} -{% endif %} -
+ {% for key, value in form_data.items %} {% endfor %} @@ -70,6 +54,5 @@ {% trans "No, continue to edit" %}

-
{% endblock %} diff --git a/admin_confirm/templates/admin/change_data.html b/admin_confirm/templates/admin/change_data.html new file mode 100644 index 0000000..713f835 --- /dev/null +++ b/admin_confirm/templates/admin/change_data.html @@ -0,0 +1,11 @@ +{% if changed_data %} +
+

Confirm Values:

+ + + {% for field, values in changed_data.items %} + + {% endfor %} +
FieldCurrent ValueNew Value
{{ field }}{{ values.0 }}{{ values.1 }}
+
+{% endif %} diff --git a/admin_confirm/tests/test_confirm_actions.py b/admin_confirm/tests/test_confirm_actions.py new file mode 100644 index 0000000..e5c96ab --- /dev/null +++ b/admin_confirm/tests/test_confirm_actions.py @@ -0,0 +1,281 @@ +from django.test import TestCase, RequestFactory +from django.contrib.admin.sites import AdminSite +from django.contrib.auth.models import Permission, User +from django.contrib.admin.options import TO_FIELD_VAR +from django.urls import reverse + + +from tests.market.admin import ShopAdmin +from tests.market.models import Shop + + +class TestConfirmActions(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_get_changelist_should_not_be_affected(self): + response = self.client.get(reverse("admin:market_shop_changelist")) + self.assertIsNotNone(response) + self.assertNotIn("Confirm Action", response.rendered_content) + + def test_action_without_confirmation(self): + post_params = { + "action": ["show_message_no_confirmation"], + "select_across": ["0"], + "index": ["0"], + "_selected_action": ["3", "2", "1"], + } + response = self.client.post( + reverse("admin:market_shop_changelist"), + data=post_params, + follow=True, # Follow the redirect to get content + ) + self.assertIsNotNone(response) + self.assertEqual(response.status_code, 200) + # Should not use confirmaiton page + self.assertNotIn("action_confirmation", response.template_name) + + # The action was to show user a message + self.assertIn("You selected without confirmation", response.rendered_content) + + def test_action_with_confirmation_should_show_confirmation_page(self): + post_params = { + "action": ["show_message"], + "select_across": ["0"], + "index": ["0"], + "_selected_action": ["3", "2", "1"], + } + response = self.client.post( + reverse("admin:market_shop_changelist"), + data=post_params, + follow=True, # Follow the redirect to get content + ) + self.assertIsNotNone(response) + self.assertEqual(response.status_code, 200) + # Should use confirmaiton page + self.assertEqual( + response.template_name, + [ + "admin/market/shop/action_confirmation.html", + "admin/market/action_confirmation.html", + "admin/action_confirmation.html", + ], + ) + + # The action was to show user a message, and should not happen yet + self.assertNotIn("You selected", response.rendered_content) + + def test_no_permissions_in_database_for_action_with_confirmation(self): + """ + Django would not show the action in changelist action selector + If the user doesn't have permissions, but this doesn't prevent + user from calling post with the params to perform the action. + + If the permissions are denied because of Permission in the database, + Django would redirect to the changelist. + """ + # Create a user without permissions for action + user = User.objects.create_user( + username="user", + email="user@email.org", + password="pass", + is_active=True, + is_staff=True, + is_superuser=False, + ) + # Give user permissions to ShopAdmin change, add, view but not delete + for permission in Permission.objects.filter( + codename__in=["change_shop", "view_shop", "add_shop"] + ): + user.user_permissions.add(permission) + + self.client.force_login(user) + + post_params = { + "action": ["show_message"], + "select_across": ["0"], + "index": ["0"], + "_selected_action": ["3", "2", "1"], + } + response = self.client.post( + reverse("admin:market_shop_changelist"), + data=post_params, + follow=True, # Follow the redirect to get content + ) + self.assertIsNotNone(response) + self.assertEqual(response.status_code, 200) + # Should not use confirmaiton page + self.assertEqual( + response.template_name, + [ + "admin/market/shop/change_list.html", + "admin/market/change_list.html", + "admin/change_list.html", + ], + ) + + # The action was to show user a message, and should not happen + self.assertNotIn("You selected", response.rendered_content) + + # Django won't show the action as an option to you + self.assertIn("No action selected", response.rendered_content) + + def test_no_permissions_in_code_non_superuser_for_action_with_confirmation(self): + """ + Django would not show the action in changelist action selector + If the user doesn't have permissions, but this doesn't prevent + user from calling post with the params to perform the action. + + If the permissions are denied because of Permission in the database, + Django would redirect to the changelist. + + It should also respect the has_xxx_permission methods + """ + # Create a user without permissions for action + user = User.objects.create_user( + username="user", + email="user@email.org", + password="pass", + is_active=True, + is_staff=True, + is_superuser=False, + ) + # Give user permissions to ShopAdmin change, add, view and delete + for permission in Permission.objects.filter( + codename__in=["change_shop", "view_shop", "add_shop", "delete_shop"] + ): + user.user_permissions.add(permission) + + self.client.force_login(user) + + # ShopAdmin has defined: + # def has_delete_permission(self, request, obj=None): + # return request.user.is_superuser + + post_params = { + "action": ["show_message"], + "select_across": ["0"], + "index": ["0"], + "_selected_action": ["3", "2", "1"], + } + response = self.client.post( + reverse("admin:market_shop_changelist"), + data=post_params, + follow=True, # Follow the redirect to get content + ) + self.assertIsNotNone(response) + self.assertEqual(response.status_code, 200) + # Should not use confirmaiton page + self.assertEqual( + response.template_name, + [ + "admin/market/shop/change_list.html", + "admin/market/change_list.html", + "admin/change_list.html", + ], + ) + + # The action was to show user a message, and should not happen yet + self.assertNotIn("You selected", response.rendered_content) + + # Django won't show the action as an option to you + self.assertIn("No action selected", response.rendered_content) + + def test_no_permissions_in_code_superuser_for_action_with_confirmation(self): + """ + Django would not show the action in changelist action selector + If the user doesn't have permissions, but this doesn't prevent + user from calling post with the params to perform the action. + + When permissions are denied from a change in code + (ie has_xxx_permission in ModelAdmin), Django should still + redirect to changelist. This should be true even if the user is + a superuser. + """ + # ShopAdmin has defined: + # def has_delete_permission(self, request, obj=None): + # return request.user.is_superuser + + ShopAdmin.has_delete_permission = lambda self, request, obj=None: False + post_params = { + "action": ["show_message"], + "select_across": ["0"], + "index": ["0"], + "_selected_action": ["3", "2", "1"], + } + response = self.client.post( + reverse("admin:market_shop_changelist"), + data=post_params, + follow=True, # Follow the redirect to get content + ) + self.assertIsNotNone(response) + self.assertEqual(response.status_code, 200) + # Should not use confirmaiton page + self.assertEqual( + response.template_name, + [ + "admin/market/shop/change_list.html", + "admin/market/change_list.html", + "admin/change_list.html", + ], + ) + + # The action was to show user a message, and should not happen yet + self.assertNotIn("You selected", response.rendered_content) + + # Django won't show the action as an option to you + self.assertIn("No action selected", response.rendered_content) + + # Remove our modification for ShopAdmin + ShopAdmin.has_delete_permission = ( + lambda self, request, obj=None: request.user.is_superuser + ) + + def test_confirm_action_submit_button_should_perform_action(self): + """ + The submit button should have param "_confirm_action" + + Simulate calling the post request that the button would + """ + post_params = { + "_confirm_action": ["Yes, I'm sure"], + "action": ["show_message"], + "_selected_action": ["3", "2", "1"], + } + response = self.client.post( + reverse("admin:market_shop_changelist"), + data=post_params, + follow=True, # Follow the redirect to get content + ) + self.assertIsNotNone(response) + self.assertEqual(response.status_code, 200) + # Should not use confirmaiton page, since we clicked Yes, I'm sure + self.assertEqual( + response.template_name, + [ + "admin/market/shop/change_list.html", + "admin/market/change_list.html", + "admin/change_list.html", + ], + ) + + # The action was to show user a message, and should happen + self.assertIn("You selected", response.rendered_content) + + def test_should_use_action_confirmation_template_if_set(self): + expected_template = "market/admin/my_custom_template.html" + ShopAdmin.action_confirmation_template = expected_template + admin = ShopAdmin(Shop, AdminSite()) + actual_template = admin.render_action_confirmation( + self.factory.request(), context={} + ).template_name + self.assertEqual(expected_template, actual_template) + # Clear our setting to not affect other tests + ShopAdmin.action_confirmation_template = None diff --git a/admin_confirm/tests/test_admin.py b/admin_confirm/tests/test_confirm_change_and_add.py similarity index 97% rename from admin_confirm/tests/test_admin.py rename to admin_confirm/tests/test_confirm_change_and_add.py index 5ece1b4..fb55c7a 100644 --- a/admin_confirm/tests/test_admin.py +++ b/admin_confirm/tests/test_confirm_change_and_add.py @@ -11,7 +11,7 @@ from tests.market.models import Item, Inventory from tests.factories import ItemFactory, ShopFactory, InventoryFactory -class TestAdminConfirmMixin(TestCase): +class TestConfirmChangeAndAdd(TestCase): @classmethod def setUpTestData(cls): cls.superuser = User.objects.create_superuser( @@ -138,13 +138,14 @@ class TestAdminConfirmMixin(TestCase): def test_custom_template(self): expected_template = "market/admin/my_custom_template.html" - ItemAdmin.confirmation_template = expected_template + ItemAdmin.change_confirmation_template = expected_template admin = ItemAdmin(Item, AdminSite()) actual_template = admin.render_change_confirmation( self.factory.request(), context={} ).template_name self.assertEqual(expected_template, actual_template) - ItemAdmin.confirmation_template = None + # Clear our setting to not affect other tests + ItemAdmin.change_confirmation_template = None def test_form_invalid(self): self.assertEqual(InventoryAdmin.confirmation_fields, ["quantity"]) @@ -162,7 +163,7 @@ class TestAdminConfirmMixin(TestCase): f"/admin/market/inventory/{inventory.id}/change/", data ) - # Form invalid should show erros on form + # Form invalid should show errors on form self.assertEqual(response.status_code, 200) print(response.rendered_content) self.assertIsNotNone(response.context_data.get("errors")) diff --git a/admin_confirm/utils.py b/admin_confirm/utils.py new file mode 100644 index 0000000..9eed8c8 --- /dev/null +++ b/admin_confirm/utils.py @@ -0,0 +1,2 @@ +def snake_to_title_case(string: str) -> str: + return " ".join(string.split("_")).title() diff --git a/screenshot_confirm_action.png b/screenshot_confirm_action.png new file mode 100644 index 0000000..dad130d Binary files /dev/null and b/screenshot_confirm_action.png differ diff --git a/screenshot_confirm_action_no_perm.png b/screenshot_confirm_action_no_perm.png new file mode 100644 index 0000000..a81234d Binary files /dev/null and b/screenshot_confirm_action_no_perm.png differ diff --git a/screenshot.png b/screenshot_confirm_change.png similarity index 100% rename from screenshot.png rename to screenshot_confirm_change.png diff --git a/setup.py b/setup.py index 890240f..4747551 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ README = open(os.path.join(here, "README.md")).read() setup( name="django-admin-confirm", - version="0.1", + version="0.2.dev1", packages=["admin_confirm"], description="Adds confirmation to Django Admin changes and additions", long_description_content_type="text/markdown", @@ -18,5 +18,5 @@ setup( install_requires=[ "Django>=1.7", ], - python_requires='>=3', + python_requires=">=3", ) diff --git a/tests/market/admin.py b/tests/market/admin.py index 100c65f..a65dbd8 100644 --- a/tests/market/admin.py +++ b/tests/market/admin.py @@ -1,6 +1,6 @@ from django.contrib import admin -from admin_confirm.admin import AdminConfirmMixin +from admin_confirm.admin import AdminConfirmMixin, confirm_action from .models import Item, Inventory, Shop @@ -20,6 +20,21 @@ class InventoryAdmin(AdminConfirmMixin, admin.ModelAdmin): class ShopAdmin(AdminConfirmMixin, admin.ModelAdmin): confirmation_fields = ["name"] + actions = ["show_message", "show_message_no_confirmation"] + + @confirm_action + def show_message(modeladmin, request, queryset): + shops = ", ".join(shop.name for shop in queryset) + modeladmin.message_user(request, f"You selected with confirmation: {shops}") + + show_message.allowed_permissions = ('delete',) + + def show_message_no_confirmation(modeladmin, request, queryset): + shops = ", ".join(shop.name for shop in queryset) + modeladmin.message_user(request, f"You selected without confirmation: {shops}") + + def has_delete_permission(self, request, obj=None): + return request.user.is_superuser admin.site.register(Item, ItemAdmin) admin.site.register(Inventory, InventoryAdmin) diff --git a/tests/test_project/settings.py b/tests/test_project/settings.py index 629218e..7ce07ab 100644 --- a/tests/test_project/settings.py +++ b/tests/test_project/settings.py @@ -24,7 +24,7 @@ SECRET_KEY = "=yddl-40388w3e2hl$e8)revce=n67_idi8pfejtn3!+2%!_qt" # SECURITY WARNING: don't run with debug turned on in production! DEBUG = True -ALLOWED_HOSTS = ["127.0.0.1"] +ALLOWED_HOSTS = ["127.0.0.1", "localhost"] # Application definition