Skip to content

Commit

Permalink
Merge pull request #3138 from cisagov/bob/3133-transfer-user-bug
Browse files Browse the repository at this point in the history
#3133: Transfer user duplicate portfolio permission bug - [BOB]
  • Loading branch information
rachidatecs authored Dec 5, 2024
2 parents d3a7f45 + 78f708a commit 7a03ffc
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 0 deletions.
57 changes: 57 additions & 0 deletions src/registrar/tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices
from django.contrib.sessions.backends.db import SessionStore
from django.contrib.auth import get_user_model
from django.contrib import messages

from unittest.mock import ANY, patch, Mock
from django.forms import ValidationError
Expand Down Expand Up @@ -2452,6 +2453,33 @@ def test_transfer_user_transfers_user_portfolio_roles(self):

self.assertEquals(user_portfolio_permission.user, self.user1)

@less_console_noise_decorator
def test_transfer_user_transfers_user_portfolio_roles_no_error_when_duplicates(self):
"""Assert that duplicate portfolio user roles do not throw errorsd"""
portfolio1 = Portfolio.objects.create(organization_name="Hotel California", creator=self.user2)
UserPortfolioPermission.objects.create(
user=self.user1, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
)
UserPortfolioPermission.objects.create(
user=self.user2, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
)

with patch.object(messages, "error"):
user_transfer_page = self.app.get(reverse("transfer_user", args=[self.user1.pk]))

submit_form = user_transfer_page.forms[1]
submit_form["selected_user"] = self.user2.pk
submit_form.submit()

# Verify portfolio permissions remain valid for the original user
self.assertTrue(
UserPortfolioPermission.objects.filter(
user=self.user1, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
).exists()
)

messages.error.assert_not_called()

@less_console_noise_decorator
def test_transfer_user_transfers_domain_request_creator_and_investigator(self):
"""Assert that domain request fields get transferred"""
Expand Down Expand Up @@ -2506,6 +2534,35 @@ def test_transfer_user_transfers_domain_role(self):
self.assertEquals(user_domain_role1.user, self.user1)
self.assertEquals(user_domain_role2.user, self.user1)

@less_console_noise_decorator
def test_transfer_user_transfers_domain_role_no_error_when_duplicate(self):
"""Assert that duplicate user domain roles do not throw errors"""
domain_1, _ = Domain.objects.get_or_create(name="chrome.gov", state=Domain.State.READY)
domain_2, _ = Domain.objects.get_or_create(name="v8.gov", state=Domain.State.READY)
UserDomainRole.objects.get_or_create(user=self.user1, domain=domain_1, role=UserDomainRole.Roles.MANAGER)
UserDomainRole.objects.get_or_create(user=self.user2, domain=domain_1, role=UserDomainRole.Roles.MANAGER)
UserDomainRole.objects.get_or_create(user=self.user2, domain=domain_2, role=UserDomainRole.Roles.MANAGER)

with patch.object(messages, "error"):

user_transfer_page = self.app.get(reverse("transfer_user", args=[self.user1.pk]))
submit_form = user_transfer_page.forms[1]
submit_form["selected_user"] = self.user2.pk
submit_form.submit()

self.assertTrue(
UserDomainRole.objects.filter(
user=self.user1, domain=domain_1, role=UserDomainRole.Roles.MANAGER
).exists()
)
self.assertTrue(
UserDomainRole.objects.filter(
user=self.user1, domain=domain_2, role=UserDomainRole.Roles.MANAGER
).exists()
)

messages.error.assert_not_called()

@less_console_noise_decorator
def test_transfer_user_transfers_verified_by_staff_requestor(self):
"""Assert that verified by staff creator gets transferred"""
Expand Down
4 changes: 4 additions & 0 deletions src/registrar/views/transfer_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ def update_joins_and_log(cls, model_class, field_name, selected_user, current_us
if model_class.objects.filter(user=current_user, domain=obj.domain).exists():
continue # Skip the update to avoid a duplicate

if model_class == UserPortfolioPermission:
if model_class.objects.filter(user=current_user, portfolio=obj.portfolio).exists():
continue # Skip the update to avoid a duplicate

# Update the field on the object and save it
setattr(obj, field_name, current_user)
obj.save()
Expand Down

0 comments on commit 7a03ffc

Please sign in to comment.