Merge pull request #3388 from cisagov/bob/2711-domain-mgr-remove

#2711: Domain manager view - remove manager when only one manager

src/registrar/templates/domain_base.html

@@ -46,7 +46,7 @@ <h4 class="usa-alert__heading">Attention!</h4>
         {# messages block is under the back breadcrumb link #}
         {% if messages %}
         {% for message in messages %}
-        <div class="usa-alert usa-alert--{{ message.tags }} usa-alert--slim margin-bottom-3">
+        <div class="usa-alert usa-alert--{{ message.tags }} usa-alert--slim margin-bottom-2">
             <div class="usa-alert__body">
             {{ message }}
             </div>

src/registrar/templates/domain_users.html

@@ -69,14 +69,14 @@ <h2> Domain managers </h2>
         </th>
         {% if not portfolio %}<td data-label="Role">{{ item.permission.role|title }}</td>{% endif %}
         <td>
-        {% if can_delete_users %}
           <a
             id="button-toggle-user-alert-{{ forloop.counter }}"
             href="#toggle-user-alert-{{ forloop.counter }}"
             class="usa-button--unstyled text-no-underline"
             aria-controls="toggle-user-alert-{{ forloop.counter }}"
             data-open-modal
             aria-disabled="false"
+            aria-label="Remove {{ item.permission.user.email }}""
           >
             Remove
           </a>
@@ -112,18 +112,6 @@ <h2> Domain managers </h2>
               {% csrf_token %}
             </form>
           {% endif %}
-        {% else %}
-          <input 
-          type="submit" 
-          class="usa-button--unstyled disabled-button usa-tooltip usa-tooltip--registrar" 
-          value="Remove"
-          data-position="bottom"
-          title="Domains must have at least one domain manager"
-          data-tooltip="true"
-          aria-disabled="true"
-          role="button"
-          >
-        {% endif %}
         </td>
       </tr>
       {% endfor %}

src/registrar/tests/test_views_domain.py

@@ -721,6 +721,7 @@ def tearDown(self):
         """Ensure that the user has its original permissions"""
         PortfolioInvitation.objects.all().delete()
         UserPortfolioPermission.objects.all().delete()
+        UserDomainRole.objects.all().delete()
         User.objects.exclude(id=self.user.id).delete()
         super().tearDown()
 
@@ -1258,8 +1259,8 @@ def test_domain_invitation_cancel_retrieved_invitation(self):
         response = self.client.post(reverse("invitation-cancel", kwargs={"pk": invitation.id}), follow=True)
         # Assert that an error message is displayed to the user
         self.assertContains(response, f"Invitation to {email_address} has already been retrieved.")
-        # Assert that the Cancel link is not displayed
-        self.assertNotContains(response, "Cancel")
+        # Assert that the Cancel link (form) is not displayed
+        self.assertNotContains(response, f"/invitation/{invitation.id}/cancel")
         # Assert that the DomainInvitation is not deleted
         self.assertTrue(DomainInvitation.objects.filter(id=invitation.id).exists())
         DomainInvitation.objects.filter(email=email_address).delete()
@@ -1317,6 +1318,57 @@ def test_domain_invitation_flow(self):
         home_page = self.app.get(reverse("home"))
         self.assertContains(home_page, self.domain.name)
 
+    @less_console_noise_decorator
+    def test_domain_user_role_delete(self):
+        """Posting to the delete view deletes a user domain role."""
+        # add two managers to the domain so that one can be successfully deleted
+        email_address = "[email protected]"
+        new_user = User.objects.create(email=email_address, username="mayor")
+        email_address_2 = "[email protected]"
+        new_user_2 = User.objects.create(email=email_address_2, username="secondmayor")
+        user_domain_role = UserDomainRole.objects.create(
+            user=new_user, domain=self.domain, role=UserDomainRole.Roles.MANAGER
+        )
+        UserDomainRole.objects.create(user=new_user_2, domain=self.domain, role=UserDomainRole.Roles.MANAGER)
+        response = self.client.post(
+            reverse("domain-user-delete", kwargs={"pk": self.domain.id, "user_pk": new_user.id}), follow=True
+        )
+        # Assert that a success message is displayed to the user
+        self.assertContains(response, f"Removed {email_address} as a manager for this domain.")
+        # Assert that the second user is displayed
+        self.assertContains(response, f"{email_address_2}")
+        # Assert that the UserDomainRole is deleted
+        self.assertFalse(UserDomainRole.objects.filter(id=user_domain_role.id).exists())
+
+    @less_console_noise_decorator
+    def test_domain_user_role_delete_only_manager(self):
+        """Posting to the delete view attempts to delete a user domain role when there is only one manager."""
+        # self.user is the only domain manager, so attempt to delete it
+        response = self.client.post(
+            reverse("domain-user-delete", kwargs={"pk": self.domain.id, "user_pk": self.user.id}), follow=True
+        )
+        # Assert that an error message is displayed to the user
+        self.assertContains(response, "Domains must have at least one domain manager.")
+        # Assert that the user is still displayed
+        self.assertContains(response, f"{self.user.email}")
+        # Assert that the UserDomainRole still exists
+        self.assertTrue(UserDomainRole.objects.filter(user=self.user, domain=self.domain).exists())
+
+    @less_console_noise_decorator
+    def test_domain_user_role_delete_self_delete(self):
+        """Posting to the delete view attempts to delete a user domain role when there is only one manager."""
+        # add one manager, so there are two and the logged in user, self.user, can be deleted
+        email_address = "[email protected]"
+        new_user = User.objects.create(email=email_address, username="mayor")
+        UserDomainRole.objects.create(user=new_user, domain=self.domain, role=UserDomainRole.Roles.MANAGER)
+        response = self.client.post(
+            reverse("domain-user-delete", kwargs={"pk": self.domain.id, "user_pk": self.user.id}), follow=True
+        )
+        # Assert that a success message is displayed to the user
+        self.assertContains(response, f"You are no longer managing the domain {self.domain}.")
+        # Assert that the UserDomainRole no longer exists
+        self.assertFalse(UserDomainRole.objects.filter(user=self.user, domain=self.domain).exists())
+
 
 class TestDomainNameservers(TestDomainOverview, MockEppLib):
     @less_console_noise_decorator

src/registrar/views/domain.py

@@ -1074,9 +1074,6 @@ def get_context_data(self, **kwargs):
         """The initial value for the form (which is a formset here)."""
         context = super().get_context_data(**kwargs)
 
-        # Add conditionals to the context (such as "can_delete_users")
-        context = self._add_booleans_to_context(context)
-
         # Get portfolio from session (if set)
         portfolio = self.request.session.get("portfolio")
 
@@ -1162,20 +1159,6 @@ def _add_invitations_to_context(self, context, portfolio):
 
         return context
 
-    def _add_booleans_to_context(self, context):
-        # Determine if the current user can delete managers
-        domain_pk = None
-        can_delete_users = False
-
-        if self.kwargs is not None and "pk" in self.kwargs:
-            domain_pk = self.kwargs["pk"]
-            # Prevent the end user from deleting themselves as a manager if they are the
-            # only manager that exists on a domain.
-            can_delete_users = UserDomainRole.objects.filter(domain__id=domain_pk).count() > 1
-
-        context["can_delete_users"] = can_delete_users
-        return context
-
 
 class DomainAddUserView(DomainFormBaseView):
     """Inside of a domain's user management, a form for adding users.
@@ -1310,7 +1293,7 @@ def get_success_url(self):
         """Refreshes the page after a delete is successful"""
         return reverse("domain-users", kwargs={"pk": self.object.domain.id})
 
-    def get_success_message(self, delete_self=False):
+    def get_success_message(self):
         """Returns confirmation content for the deletion event"""
 
         # Grab the text representation of the user we want to delete
@@ -1320,7 +1303,7 @@ def get_success_message(self, delete_self=False):
 
         # If the user is deleting themselves, return a specific message.
         # If not, return something more generic.
-        if delete_self:
+        if self.delete_self:
             message = f"You are no longer managing the domain {self.object.domain}."
         else:
             message = f"Removed {email_or_name} as a manager for this domain."
@@ -1333,22 +1316,35 @@ def form_valid(self, form):
         # Delete the object
         super().form_valid(form)
 
-        # Is the user deleting themselves? If so, display a different message
-        delete_self = self.request.user == self.object.user
-
-        # Email domain managers
-
         # Add a success message
-        messages.success(self.request, self.get_success_message(delete_self))
+        messages.success(self.request, self.get_success_message())
         return redirect(self.get_success_url())
 
     def post(self, request, *args, **kwargs):
-        """Custom post implementation to redirect to home in the event that the user deletes themselves"""
+        """Custom post implementation to ensure last userdomainrole is not removed and to
+        redirect to home in the event that the user deletes themselves"""
+        self.object = self.get_object()  # Retrieve the UserDomainRole to delete
+
+        # Is the user deleting themselves?
+        self.delete_self = self.request.user == self.object.user
+
+        # Check if this is the only UserDomainRole for the domain
+        if not len(UserDomainRole.objects.filter(domain=self.object.domain)) > 1:
+            if self.delete_self:
+                messages.error(
+                    request,
+                    "Domains must have at least one domain manager. "
+                    "To remove yourself, the domain needs another domain manager.",
+                )
+            else:
+                messages.error(request, "Domains must have at least one domain manager.")
+            return redirect(self.get_success_url())
+
+        # normal delete processing in the event that the above condition not reached
         response = super().post(request, *args, **kwargs)
 
         # If the user is deleting themselves, redirect to home
-        delete_self = self.request.user == self.object.user
-        if delete_self:
+        if self.delete_self:
             return redirect(reverse("home"))
 
         return response

src/registrar/views/utility/mixins.py

@@ -343,12 +343,6 @@ def has_permission(self):
         if not (has_delete_permission or user_is_analyst_or_superuser):
             return False
 
-        # Check if more than one manager exists on the domain.
-        # If only one exists, prevent this from happening
-        has_multiple_managers = len(UserDomainRole.objects.filter(domain=domain_pk)) > 1
-        if not has_multiple_managers:
-            return False
-
         return True