Refactor admin views to use the SuccessMessageMixin (#11463)

* Refactor admin views to use the SuccessMessageMixin

This updates most of our views to include success_message on them,
and use the SuccessMessageMixin on the base ProjectAdminMixin.

This should get us proper dashboard validation for this code now.

* Add DeleteViewWithMessage to support delete messages

* Add test

* Refactor mixin

* Add a couple more tests

* Fix tests

* Fix feedback from review

readthedocs/core/mixins.py

@@ -1,6 +1,8 @@
 """Common mixin classes for views."""
+
+from django.contrib import messages
 from django.contrib.auth.mixins import LoginRequiredMixin
-from vanilla import ListView
+from vanilla import DeleteView, ListView
 
 from readthedocs.proxito.cache import cache_response, private_response
 
@@ -63,3 +65,21 @@ def dispatch(self, request, *args, **kwargs):
 
     def can_be_cached(self, request):
         return self.cache_response
+
+
+class DeleteViewWithMessage(DeleteView):
+
+    """
+    Delete view that shows a message after deleting an object.
+
+    Refs https://code.djangoproject.com/ticket/21926
+    """
+
+    success_message = None
+
+    def post(self, request, *args, **kwargs):
+        resp = super().post(request, *args, **kwargs)
+        # Check if resp is a redirect, which means the object was deleted
+        if resp.status_code == 302 and self.success_message:
+            messages.success(self.request, self.success_message)
+        return resp

readthedocs/organizations/views/private.py

@@ -9,13 +9,13 @@
 from django.utils import timezone
 from django.utils.http import urlencode
 from django.utils.translation import gettext_lazy as _
-from vanilla import CreateView, DeleteView, FormView, ListView, UpdateView
+from vanilla import CreateView, FormView, ListView, UpdateView
 
 from readthedocs.audit.filters import OrganizationSecurityLogFilter
 from readthedocs.audit.models import AuditLog
 from readthedocs.core.filters import FilterContextMixin
 from readthedocs.core.history import UpdateChangeReasonPostView
-from readthedocs.core.mixins import PrivateViewMixin
+from readthedocs.core.mixins import DeleteViewWithMessage, PrivateViewMixin
 from readthedocs.invitations.models import Invitation
 from readthedocs.organizations.filters import OrganizationListFilterSet
 from readthedocs.organizations.forms import (
@@ -123,7 +123,7 @@ class DeleteOrganization(
     PrivateViewMixin,
     UpdateChangeReasonPostView,
     OrganizationView,
-    DeleteView,
+    DeleteViewWithMessage,
 ):
     template_name = "organizations/admin/organization_delete.html"
 
@@ -151,14 +151,15 @@ def form_valid(self, form):
         return super().form_valid(form)
 
 
-class DeleteOrganizationOwner(PrivateViewMixin, OrganizationOwnerView, DeleteView):
+class DeleteOrganizationOwner(
+    PrivateViewMixin, OrganizationOwnerView, DeleteViewWithMessage
+):
     success_message = _("Owner removed")
     http_method_names = ["post"]
 
     def post(self, request, *args, **kwargs):
         if self._is_last_user():
             return HttpResponseBadRequest(_("User is the last owner, can't be removed"))
-        messages.success(self.request, self.success_message)
         return super().post(request, *args, **kwargs)
 
 
@@ -172,17 +173,11 @@ class DeleteOrganizationTeam(
     PrivateViewMixin,
     UpdateChangeReasonPostView,
     OrganizationTeamView,
-    DeleteView,
+    DeleteViewWithMessage,
 ):
     template_name = "organizations/team_delete.html"
     success_message = _("Team deleted")
 
-    def post(self, request, *args, **kwargs):
-        """Hack to show messages on delete."""
-        resp = super().post(request, *args, **kwargs)
-        messages.success(self.request, self.success_message)
-        return resp
-
     def get_success_url(self):
         return reverse_lazy(
             "organization_team_list",
@@ -203,6 +198,7 @@ class UpdateOrganizationTeamProject(PrivateViewMixin, OrganizationTeamView, Upda
 
 class AddOrganizationTeamMember(PrivateViewMixin, OrganizationTeamMemberView, FormView):
     template_name = "organizations/team_member_create.html"
+    # No success message here, since it's set in the form.
 
     def form_valid(self, form):
         # Manually calling to save, since this isn't a ModelFormView.
@@ -215,17 +211,11 @@ def form_valid(self, form):
 
 
 class DeleteOrganizationTeamMember(
-    PrivateViewMixin, OrganizationTeamMemberView, DeleteView
+    PrivateViewMixin, OrganizationTeamMemberView, DeleteViewWithMessage
 ):
     success_message = _("Member removed from team")
     http_method_names = ["post"]
 
-    def post(self, request, *args, **kwargs):
-        """Hack to show messages on delete."""
-        resp = super().post(request, *args, **kwargs)
-        messages.success(self.request, self.success_message)
-        return resp
-
 
 class OrganizationSecurityLog(PrivateViewMixin, OrganizationMixin, ListView):
 

readthedocs/projects/tests/test_domain_views.py

@@ -1,4 +1,5 @@
 from django.contrib.auth.models import User
+from django.contrib.messages import get_messages
 from django.test import TestCase, override_settings
 from django.urls import reverse
 from django_dynamic_fixture import get
@@ -34,6 +35,11 @@ def test_domain_creation(self):
         domain = self.project.domains.first()
         self.assertEqual(domain.domain, "test.example.com")
 
+        # Ensure a message is shown
+        messages = list(get_messages(resp.wsgi_request))
+        self.assertEqual(len(messages), 1)
+        self.assertEqual(str(messages[0]), "Domain created")
+
     def test_domain_deletion(self):
         domain = get(Domain, project=self.project, domain="test.example.com")
         self.assertEqual(self.project.domains.count(), 1)
@@ -44,6 +50,11 @@ def test_domain_deletion(self):
         self.assertEqual(resp.status_code, 302)
         self.assertEqual(self.project.domains.count(), 0)
 
+        # Ensure a message is shown
+        messages = list(get_messages(resp.wsgi_request))
+        self.assertEqual(len(messages), 1)
+        self.assertEqual(str(messages[0]), "Domain deleted")
+
     def test_domain_edit(self):
         domain = get(
             Domain, project=self.project, domain="test.example.com", canonical=False

readthedocs/projects/tests/test_views.py

@@ -1,5 +1,6 @@
 from allauth.account.models import EmailAddress
 from django.contrib.auth.models import User
+from django.contrib.messages import get_messages
 from django.test import TestCase, override_settings
 from django.urls import reverse
 from django_dynamic_fixture import get
@@ -255,6 +256,11 @@ def test_delete_maintainer(self):
         self.assertEqual(resp.status_code, 302)
         self.assertNotIn(self.user, self.project.users.all())
 
+        # Ensure a message is shown
+        messages = list(get_messages(resp.wsgi_request))
+        self.assertEqual(len(messages), 1)
+        self.assertEqual(str(messages[0]), "User deleted")
+
     def test_delete_last_maintainer(self):
         url = reverse("projects_users_delete", args=[self.project.slug])
         self.client.force_login(self.user)

readthedocs/projects/views/base.py

@@ -3,6 +3,7 @@
 
 import structlog
 from django.conf import settings
+from django.contrib.messages.views import SuccessMessageMixin
 from django.shortcuts import get_object_or_404, render
 
 from readthedocs.projects.models import Project
@@ -40,7 +41,7 @@ def get_context_data(self, **kwargs):
 
 
 # Mixins
-class ProjectAdminMixin:
+class ProjectAdminMixin(SuccessMessageMixin):
 
     """
     Mixin class that provides project sublevel objects.
@@ -49,6 +50,9 @@ class ProjectAdminMixin:
 
     project_url_field
         The URL kwarg name for the project slug
+
+    success_message
+        Message when the form is successfully saved, comes from SuccessMessageMixin
     """
 
     project_url_field = "project_slug"