diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7dbe7abb0..31c75e05e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1329,6 +1329,14 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): get_roles.short_description = "Roles" # type: ignore + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" @@ -1661,6 +1669,14 @@ class PortfolioInvitationAdmin(BaseInvitationAdmin): # Call the parent save method to save the object super().save_model(request, obj, form, change) + def delete_queryset(self, request, queryset): + """We override the delete method in the model. + When deleting in DJA, if you select multiple items in a table using checkboxes and apply a delete action, + the model delete does not get called. This method gets called instead. + This override makes sure our code in the model gets executed in these situations.""" + for obj in queryset: + obj.delete() # Calls the overridden delete method on each instance + class DomainInformationResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 11c564c36..8feeb0794 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -8,6 +8,7 @@ from registrar.models import DomainInvitation, UserPortfolioPermission from .utility.portfolio_helper import ( UserPortfolioPermissionChoices, UserPortfolioRoleChoices, + cleanup_after_portfolio_member_deletion, validate_portfolio_invitation, ) # type: ignore from .utility.time_stamped_model import TimeStampedModel @@ -115,3 +116,27 @@ class PortfolioInvitation(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_portfolio_invitation(self) + + def delete(self, *args, **kwargs): + + User = get_user_model() + + email = self.email # Capture the email before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + if self.status == self.PortfolioInvitationStatus.INVITED: + + # Query the user by email + users = User.objects.filter(email=email) + + if users.count() > 1: + # This should never happen, log an error if more than one object is returned + logger.error(f"Multiple users found with the same email: {email}") + + # Retrieve the first user, or None if no users are found + user = users.first() + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=email, user=user) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index c4be90a9b..11d9c56e3 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -5,6 +5,7 @@ from registrar.models.utility.portfolio_helper import ( UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay, + cleanup_after_portfolio_member_deletion, validate_user_portfolio_permission, ) from .utility.time_stamped_model import TimeStampedModel @@ -188,3 +189,13 @@ class UserPortfolioPermission(TimeStampedModel): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() validate_user_portfolio_permission(self) + + def delete(self, *args, **kwargs): + + user = self.user # Capture the user before the instance is deleted + portfolio = self.portfolio # Capture the portfolio before the instance is deleted + + # Call the superclass delete method to actually delete the instance + super().delete(*args, **kwargs) + + cleanup_after_portfolio_member_deletion(portfolio=portfolio, email=user.email, user=user) diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index b3bb07c3d..8c42b80c7 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -210,3 +210,32 @@ def validate_portfolio_invitation(portfolio_invitation): "This user is already assigned to a portfolio invitation. " "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) + + +def cleanup_after_portfolio_member_deletion(portfolio, email, user=None): + """ + Cleans up after removing a portfolio member or a portfolio invitation. + + Args: + portfolio: portfolio + user: passed when removing a portfolio member. + email: passed when removing a portfolio invitation, or passed as user.email + when removing a portfolio member. + """ + + DomainInvitation = apps.get_model("registrar.DomainInvitation") + UserDomainRole = apps.get_model("registrar.UserDomainRole") + + # Fetch domain invitations matching the criteria + invitations = DomainInvitation.objects.filter( + email=email, domain__domain_info__portfolio=portfolio, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + + # Call `cancel_invitation` on each invitation + for invitation in invitations: + invitation.cancel_invitation() + invitation.save() + + if user: + # Remove user's domain roles for the current portfolio + UserDomainRole.objects.filter(user=user, domain__domain_info__portfolio=portfolio).delete() diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index ef811e083..157420be7 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -164,6 +164,7 @@ class TestPortfolioInvitations(TestCase): DomainInformation.objects.all().delete() Domain.objects.all().delete() UserPortfolioPermission.objects.all().delete() + UserDomainRole.objects.all().delete() Portfolio.objects.all().delete() PortfolioInvitation.objects.all().delete() User.objects.all().delete() @@ -442,6 +443,294 @@ class TestPortfolioInvitations(TestCase): pass + @less_console_noise_decorator + def test_delete_portfolio_invitation_deletes_portfolio_domain_invitations(self): + """Deleting a portfolio invitation causes domain invitations for the same email on the same + portfolio to be canceled.""" + + email_with_no_user = "email-with-no-user@email.gov" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_in_portfolio_2) + + domain_not_in_portfolio, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio) + invite_3, _ = DomainInvitation.objects.get_or_create(email=email_with_no_user, domain=domain_not_in_portfolio) + + invitation_of_email_with_no_user, _ = PortfolioInvitation.objects.get_or_create( + email=email_with_no_user, + portfolio=self.portfolio, + roles=[self.portfolio_role_base, self.portfolio_role_admin], + additional_permissions=[self.portfolio_permission_1, self.portfolio_permission_2], + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # Delete member (invite) + invitation_of_email_with_no_user.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + @less_console_noise_decorator + def test_deleting_a_retrieved_invitation_has_no_side_effects(self): + """Deleting a retrieved portfolio invitation causes no side effects.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # retrieve the invitation + self.invitation.retrieve() + self.invitation.save() + + # Delete member (invite) + self.invitation.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # Test that no side effects have been triggered + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + @less_console_noise_decorator + def test_delete_portfolio_invitation_deletes_user_domain_roles(self): + """Deleting a portfolio invitation causes domain invitations for the same email on the same + portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the + user if the user exists.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # Delete member (invite) + self.invitation.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles have been deleted for the domains in portfolio + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + + # The user domain role on the domain not in portfolio still exists + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + class TestUserPortfolioPermission(TestCase): @less_console_noise_decorator @@ -457,6 +746,7 @@ class TestUserPortfolioPermission(TestCase): Domain.objects.all().delete() DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() + DomainInvitation.objects.all().delete() UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() User.objects.all().delete() @@ -750,6 +1040,129 @@ class TestUserPortfolioPermission(TestCase): # Should return the forbidden permissions for member role self.assertEqual(member_only_permissions, set(member_forbidden)) + @less_console_noise_decorator + def test_delete_portfolio_permission_deletes_user_domain_roles(self): + """Deleting a user portfolio permission causes domain invitations for the same email on the same + portfolio to be canceled, also deletes any exiting user domain roles on the portfolio for the + user if the user exists.""" + + domain_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_1.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_1, portfolio=self.portfolio + ) + invite_1, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_1) + + domain_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_2, portfolio=self.portfolio + ) + invite_2, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_in_portfolio_2) + + domain_in_portfolio_3, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_3.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_3, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_3, role=UserDomainRole.Roles.MANAGER + ) + + domain_in_portfolio_4, _ = Domain.objects.get_or_create( + name="domain_in_portfolio_and_invited_4.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create( + creator=self.user, domain=domain_in_portfolio_4, portfolio=self.portfolio + ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_in_portfolio_4, role=UserDomainRole.Roles.MANAGER + ) + + domain_not_in_portfolio_1, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_1) + invite_3, _ = DomainInvitation.objects.get_or_create(email=self.user.email, domain=domain_not_in_portfolio_1) + + domain_not_in_portfolio_2, _ = Domain.objects.get_or_create( + name="domain_not_in_portfolio_2.gov", state=Domain.State.READY + ) + DomainInformation.objects.get_or_create(creator=self.user, domain=domain_not_in_portfolio_2) + UserDomainRole.objects.get_or_create( + user=self.user, domain=domain_not_in_portfolio_2, role=UserDomainRole.Roles.MANAGER + ) + + # Create portfolio permission + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + portfolio=self.portfolio, user=self.user, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + # The domain invitations start off as INVITED + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.INVITED) + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles exist + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + + # Delete member (user portfolio permission) + portfolio_permission.delete() + + # Reload the objects from the database + invite_1 = DomainInvitation.objects.get(pk=invite_1.pk) + invite_2 = DomainInvitation.objects.get(pk=invite_2.pk) + invite_3 = DomainInvitation.objects.get(pk=invite_3.pk) + + # The domain invitations to the portfolio domains have been canceled + self.assertEqual(invite_1.status, DomainInvitation.DomainInvitationStatus.CANCELED) + self.assertEqual(invite_2.status, DomainInvitation.DomainInvitationStatus.CANCELED) + + # Invite 3 is unaffected + self.assertEqual(invite_3.status, DomainInvitation.DomainInvitationStatus.INVITED) + + # The user domain roles have been deleted for the domains in portfolio + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_3, + ).exists() + ) + self.assertFalse( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_in_portfolio_4, + ).exists() + ) + + # The user domain role on the domain not in portfolio still exists + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user, + domain=domain_not_in_portfolio_2, + ).exists() + ) + class TestUser(TestCase): """Test actions that occur on user login,