diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 25e8b0fb6..3f1e85dfa 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -721,39 +721,20 @@ class TestDomainManagers(TestDomainOverview): self.assertNotIn("Last", email_content) self.assertNotIn("First Last", email_content) - @boto3_mocking.patching @less_console_noise_decorator - def test_domain_invitation_email_displays_error_non_existent(self): - """Inviting a non existent user sends them an email, with email as the name.""" - # make sure there is no user with this email - email_address = "mayor@igorville.gov" - User.objects.filter(email=email_address).delete() - - # Give the user who is sending the email an invalid email address - self.user.email = "" - self.user.save() - + def test_domain_invitation_email_validation_blocks_bad_email(self): + """Inviting a bad email blocks at validation.""" + email_address = "mayor" self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) - mock_client = MagicMock() - mock_error_message = MagicMock() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with patch("django.contrib.messages.error") as mock_error_message: - add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - add_page.form["email"] = email_address - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - add_page.form.submit().follow() + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + response = add_page.form.submit() - expected_message_content = "Can't send invitation email. No email is associated with your account." + self.assertContains(response, "Enter an email address in the required format, like name@example.com.") - # Grab the message content - returned_error_message = mock_error_message.call_args[0][1] - - # Check that the message content is what we expect - self.assertEqual(expected_message_content, returned_error_message) - - @boto3_mocking.patching @less_console_noise_decorator def test_domain_invitation_email_displays_error(self): """When the requesting user has no email, an error is displayed""" @@ -764,28 +745,25 @@ class TestDomainManagers(TestDomainOverview): # Give the user who is sending the email an invalid email address self.user.email = "" + self.user.is_staff = False self.user.save() self.domain_information, _ = DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain) - mock_client = MagicMock() + with patch("django.contrib.messages.error") as mock_error: + add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + add_page.form["email"] = email_address + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + add_page.form.submit() - mock_error_message = MagicMock() - with boto3_mocking.clients.handler_for("sesv2", mock_client): - with patch("django.contrib.messages.error") as mock_error_message: - add_page = self.app.get(reverse("domain-users-add", kwargs={"pk": self.domain.id})) - session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] - add_page.form["email"] = email_address - self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - add_page.form.submit().follow() + expected_message_content = f"Can't send invitation email. No email is associated with the account for 'test_user'." - expected_message_content = "Can't send invitation email. No email is associated with your account." - - # Grab the message content - returned_error_message = mock_error_message.call_args[0][1] - - # Check that the message content is what we expect - self.assertEqual(expected_message_content, returned_error_message) + # Assert that the error message was called with the correct argument + mock_error.assert_called_once_with( + ANY, + expected_message_content, + ) @less_console_noise_decorator def test_domain_invitation_cancel(self): diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 01383ae77..f92bb59ab 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -2533,6 +2533,8 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): cls.new_member_email = "new_user@example.com" + AllowedEmail.objects.get_or_create(email=cls.new_member_email) + # Assign permissions to the user making requests UserPortfolioPermission.objects.create( user=cls.user, @@ -2550,8 +2552,10 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): UserPortfolioPermission.objects.all().delete() Portfolio.objects.all().delete() User.objects.all().delete() + AllowedEmail.objects.all().delete() super().tearDownClass() + @boto3_mocking.patching @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @@ -2563,25 +2567,28 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): session_id = self.client.session.session_key self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) - # Simulate submission of member invite for new user - final_response = self.client.post( - reverse("new-member"), - { - "member_access_level": "basic", - "basic_org_domain_request_permissions": "view_only", - "email": self.new_member_email, - }, - ) + mock_client = MagicMock() - # Ensure the final submission is successful - self.assertEqual(final_response.status_code, 302) # redirects after success + with boto3_mocking.clients.handler_for("sesv2", mock_client): + # Simulate submission of member invite for new user + final_response = self.client.post( + reverse("new-member"), + { + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, + "email": self.new_member_email, + }, + ) - # Validate Database Changes - portfolio_invite = PortfolioInvitation.objects.filter( - email=self.new_member_email, portfolio=self.portfolio - ).first() - self.assertIsNotNone(portfolio_invite) - self.assertEqual(portfolio_invite.email, self.new_member_email) + # Ensure the final submission is successful + self.assertEqual(final_response.status_code, 302) # Redirects + + # Validate Database Changes + portfolio_invite = PortfolioInvitation.objects.filter( + email=self.new_member_email, portfolio=self.portfolio + ).first() + self.assertIsNotNone(portfolio_invite) + self.assertEqual(portfolio_invite.email, self.new_member_email) @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -2600,14 +2607,15 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): response = self.client.post( reverse("new-member"), { - "member_access_level": "basic", - "basic_org_domain_request_permissions": "view_only", + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "email": self.invited_member_email, }, ) - self.assertEqual(response.status_code, 302) # Redirects + self.assertEqual(response.status_code, 200) - # TODO: verify messages + # verify messages + self.assertContains(response, "This user is already assigned to a portfolio invitation. Based on current waffle flag settings, users cannot be assigned to multiple portfolios.") # Validate Database has not changed invite_count_after = PortfolioInvitation.objects.count() @@ -2630,14 +2638,15 @@ class TestPortfolioInviteNewMemberView(TestWithUser, WebTest): response = self.client.post( reverse("new-member"), { - "member_access_level": "basic", - "basic_org_domain_request_permissions": "view_only", + "role": UserPortfolioRoleChoices.ORGANIZATION_MEMBER.value, + "domain_request_permission_member": UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS.value, "email": self.user.email, }, ) - self.assertEqual(response.status_code, 302) # Redirects + self.assertEqual(response.status_code, 200) - # TODO: verify messages + # Verify messages + self.assertContains(response, "This user is already assigned to a portfolio. Based on current waffle flag settings, users cannot be assigned to multiple portfolios.") # Validate Database has not changed invite_count_after = PortfolioInvitation.objects.count() @@ -2783,7 +2792,11 @@ class TestEditPortfolioMemberView(WebTest): @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) def test_admin_removing_own_admin_role(self): - """Tests an admin removing their own admin role redirects to home.""" + """Tests an admin removing their own admin role redirects to home. + + Removing the admin role will remove both view and edit members permissions. + Note: The user can remove the edit members permissions but as long as they stay in admin role, they will at least still have view members permissions.""" + self.client.force_login(self.user) # Get the user's admin permission diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 60fb9b7b1..f17acb820 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -1254,6 +1254,8 @@ class DomainAddUserView(DomainFormBaseView): messages.success(self.request, f"{requested_email} has been invited.") except Exception as e: self._handle_portfolio_exceptions(e, requested_email, requestor_org) + # If that first invite does not succeed take an early exit + return redirect(self.get_success_url()) try: if requested_user is None: @@ -1288,7 +1290,6 @@ class DomainAddUserView(DomainFormBaseView): send_domain_invitation_email( email=email, requestor=requestor, - requested_user=requested_user, domain=self.object, is_member_of_different_org=member_of_different_org, ) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index bce668665..f5c16e2ac 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -163,13 +163,14 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): def post(self, request, pk): portfolio_permission = get_object_or_404(UserPortfolioPermission, pk=pk) + user_initially_is_admin = UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_permission.roles user = portfolio_permission.user form = self.form_class(request.POST, instance=portfolio_permission) if form.is_valid(): # Check if user is removing their own admin or edit role removing_admin_role_on_self = ( request.user == user - and UserPortfolioRoleChoices.ORGANIZATION_ADMIN in portfolio_permission.roles + and user_initially_is_admin and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in form.cleaned_data.get("role", []) ) form.save()