diff --git a/src/registrar/templates/domain_base.html b/src/registrar/templates/domain_base.html index 165441c91..58038d0a4 100644 --- a/src/registrar/templates/domain_base.html +++ b/src/registrar/templates/domain_base.html @@ -46,7 +46,7 @@ {# messages block is under the back breadcrumb link #} {% if messages %} {% for message in messages %} -
+
{{ message }}
diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 8f26a7f98..c6e6baa93 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -69,7 +69,6 @@ {% if not portfolio %}{{ item.permission.role|title }}{% endif %} - {% if can_delete_users %} Remove @@ -112,18 +112,6 @@ {% csrf_token %} {% endif %} - {% else %} - - {% endif %} {% endfor %} diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 45758e502..d4766b474 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -721,6 +721,7 @@ class TestDomainManagers(TestDomainOverview): """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 @@ class TestDomainManagers(TestDomainOverview): 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 @@ class TestDomainManagers(TestDomainOverview): 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 = "mayor@igorville.gov" + new_user = User.objects.create(email=email_address, username="mayor") + email_address_2 = "secondmayor@igorville.gov" + 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 = "mayor@igorville.gov" + 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 diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 5cac3c667..83898934c 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -1074,9 +1074,6 @@ class DomainUsersView(DomainBaseView): """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 @@ class DomainUsersView(DomainBaseView): 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 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): """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 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): # 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 @@ class DomainDeleteUserView(UserDomainRolePermissionDeleteView): # 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 diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 11384ca09..236ef8696 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -343,12 +343,6 @@ class UserDeleteDomainRolePermission(PermissionsLoginMixin): 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