Merge branch 'main' into za/3145-use-transaction-atomic

This commit is contained in:
zandercymatics 2025-01-30 10:01:57 -07:00
commit de3119ce7f
No known key found for this signature in database
GPG key ID: FF4636ABEC9682B7
5 changed files with 80 additions and 50 deletions

View file

@ -46,7 +46,7 @@
{# 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>

View file

@ -69,7 +69,6 @@
</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 }}"
@ -77,6 +76,7 @@
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 @@
{% 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 %}

View file

@ -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

View file

@ -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

View file

@ -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