Feat/confirm actions (#2)

* Working wrapper for actions * checking permissions for action * Refactor/clean change_confirmation template a bit * Update README * Update README * Adding unit tests for confirm_action decorator * Updated tests/readme * Update after testing upload to test pypi * Clean up and format code Co-authored-by: Thu Trang Pham <thu@joinmodernhealth.com>
2020-11-29 13:31:27 -08:00
parent 9edc66f31a
commit 1b617170bf
16 changed files with 535 additions and 47 deletions
@@ -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
@@ -0,0 +1,56 @@
+{% extends "admin/base_site.html" %}
+{% load i18n l10n admin_urls static %}
+
+{% block extrahead %}
+    {{ block.super }}
+    {{ media }}
+    <script src="{% static 'admin/js/cancel.js' %}" async></script>
+{% endblock %}
+
+{% block extrastyle %}
+    {{ block.super }}
+    <link rel="stylesheet" type="text/css" href="{% static "admin/css/forms.css" %}">
+    <link rel="stylesheet" type="text/css" href="{% static "admin/css/confirmation.css" %}">
+{% endblock %}
+
+{% block bodyclass %}{{ block.super }} app-{{ opts.app_label }} model-{{ opts.model_name }} change-confirmation{% endblock %}
+
+{% block breadcrumbs %}
+<div class="breadcrumbs">
+<a href="{% url 'admin:index' %}">{% trans 'Home' %}</a>
+&rsaquo; <a href="{% url 'admin:app_list' app_label=opts.app_label %}">{{ opts.app_config.verbose_name }}</a>
+&rsaquo; <a href="{% url opts|admin_urlname:'changelist' %}">{{ opts.verbose_name_plural|capfirst }}</a>
+&rsaquo; {% trans 'Confirm Action' %}
+</div>
+{% endblock %}
+
+{% block content %}
+{% if has_perm %}
+  <p>{% trans 'Are you sure you want to perform action' %} {{ action_display_name }} {% trans 'on the following' %} {{ opts.verbose_name_plural|capfirst }}?</p>
+  <ul>
+    {% for obj in queryset %}
+      <li>{{ obj }}</li>
+    {% endfor %}
+  </ul>
+  <form method="post">{% csrf_token %}
+  {% for obj in queryset %}
+    <input type="hidden" name="{{ action_checkbox_name }}" value="{{ obj.pk|unlocalize }}">
+  {% endfor %}
+  <input type="hidden" name="action" value="{{ action }}">
+  <div class="submit-row">
+      <input type="submit" value="{% trans 'Yes, I’m sure' %}" name="_confirm_action">
+      <p class="deletelink-box">
+          <a href="{% url opts|admin_urlname:'changelist' %}" class="button cancel-link">{% trans "No, go back" %}</a>
+      </p>
+  </div>
+  </form>
+{% else %}
+  <p>{% trans "You don't have permissions to perform action" %} {{ action_display_name }} {% trans 'on' %} {{ opts.verbose_name_plural|capfirst }}</p>
+  <br/>
+  <div class="submit-row">
+      <p class="deletelink-box">
+          <a href="{% url opts|admin_urlname:'changelist' %}" class="button cancel-link">{% trans "Go back" %}</a>
+      </p>
+  </div>
+{% endif %}
+{% endblock %}
@@ -30,35 +30,19 @@
 {% endblock %}

 {% block content %}
-{% if add %}
-    <p>{% blocktrans with escaped_object=object %}Are you sure you want to add the {{ model_name }}?{% endblocktrans %}</p>
-    {% if changed_data %}
-      <div class="changed-data">
-          <p><b>Confirm Values:</b></p>
-          <table>
-    {% for field, values in changed_data.items %}
-        <tr><th style="text-align: right">{{ field }}:</th><td>{{ values.1 }}</td></tr>
-    {% endfor %}
-    </table>
-      </div>
-    <form method="post" action="{% url opts|admin_urlname:'add'%}">{% csrf_token %}
+
+  {% if add %}
+      <p>{% blocktrans with escaped_object=object %}Are you sure you want to add the {{ model_name }}?{% endblocktrans %}</p>
+      {% include "admin/change_data.html" %}
+      <form method="post" action="{% url opts|admin_urlname:'add'%}">{% csrf_token %}
+
+  {% else %}
+
+    <p>{% blocktrans with escaped_object=object %}Are you sure you want to change the {{ model_name }} "{{ object_name }}"?{% endblocktrans %}</p>
+    {% include "admin/change_data.html" %}
+    <form method="post" action="{% url opts|admin_urlname:'change' object_id|admin_urlquote %}">{% csrf_token %}
  {% endif %}
-{% else %}
-  <p>{% blocktrans with escaped_object=object %}Are you sure you want to change the {{ model_name }} "{{ object_name }}"?{% endblocktrans %}</p>
-  {% if changed_data %}
-      <div class="changed-data">
-    <p><b>Confirm Values:</b></p>
-    <table>
-    <tr><th>Field</th><th>Current Value</th><th>New Value</th></tr>
-    {% for field, values in changed_data.items %}
-        <tr><td>{{ field }}</td><td>{{ values.0 }}</td><td>{{ values.1 }}</td></tr>
-    {% endfor %}
-    </table>
-      </div>
-  {% endif %}
-  <form method="post" action="{% url opts|admin_urlname:'change' object_id|admin_urlquote %}">{% csrf_token %}
-{% endif %}
-  <div>
+
    {% for key, value in form_data.items %}
        <input type="hidden" name="{{ key }}" value="{{ value }}">
    {% endfor %}
@@ -70,6 +54,5 @@
            <a href="#" class="button cancel-link">{% trans "No, continue to edit" %}</a>
        </p>
    </div>
-  </div>
  </form>
 {% endblock %}
@@ -0,0 +1,11 @@
+{% if changed_data %}
+    <div class="changed-data">
+    <p><b>Confirm Values:</b></p>
+    <table>
+        <tr><th>Field</th><th>Current Value</th><th>New Value</th></tr>
+        {% for field, values in changed_data.items %}
+            <tr><td>{{ field }}</td><td>{{ values.0 }}</td><td>{{ values.1 }}</td></tr>
+        {% endfor %}
+    </table>
+    </div>
+{% endif %}
@@ -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
@@ -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"))
@@ -0,0 +1,2 @@
+def snake_to_title_case(string: str) -> str:
+    return " ".join(string.split("_")).title()