diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index ecdf54bbc..979f20826 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -1,8 +1,8 @@ # This workflow can be run from the CLI # gh workflow run reset-db.yaml -f environment=ENVIRONMENT -name: Reset database -run-name: Reset database for ${{ github.event.inputs.environment }} +name: Delete and Recreate database +run-name: Delete and Recreate for ${{ github.event.inputs.environment }} on: workflow_dispatch: @@ -53,7 +53,7 @@ jobs: sudo apt-get update sudo apt-get install cf8-cli cf api api.fr.cloud.gov - cf auth "$CF_USERNAME" "$CF_PASSWORD" + cf auth "$cf_username" "$cf_password" cf target -o cisa-dotgov -s $DESTINATION_ENVIRONMENT diff --git a/src/registrar/assets/src/js/getgov/portfolio-member-page.js b/src/registrar/assets/src/js/getgov/portfolio-member-page.js index c96677ebc..95723fc7e 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -128,7 +128,7 @@ export function initAddNewMemberPageListeners() { }); } else { // for admin users, the permissions are always the same - appendPermissionInContainer('Domains', 'Viewer, all', permissionDetailsContainer); + appendPermissionInContainer('Domains', 'Viewer', permissionDetailsContainer); appendPermissionInContainer('Domain requests', 'Creator', permissionDetailsContainer); appendPermissionInContainer('Members', 'Manager', permissionDetailsContainer); } diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 78439188e..fa4c2d8dc 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -107,6 +107,7 @@ DEBUG = env_debug # Controls production specific feature toggles IS_PRODUCTION = env_is_production SECRET_ENCRYPT_METADATA = secret_encrypt_metadata +BASE_URL = env_base_url # Applications are modular pieces of code. # They are provided by Django, by third-parties, or by yourself. diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 2725224f1..9824ed68a 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -127,7 +127,7 @@ class BasePortfolioMemberForm(forms.ModelForm): domain_permissions = forms.ChoiceField( choices=[ (UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS.value, "Viewer, limited"), - (UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS.value, "Viewer, all"), + (UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS.value, "Viewer"), ], widget=forms.RadioSelect, required=False, @@ -338,6 +338,24 @@ class BasePortfolioMemberForm(forms.ModelForm): and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in new_roles ) + def is_change(self) -> bool: + """ + Determines if the form has changed by comparing the initial data + with the submitted cleaned data. + + Returns: + bool: True if the form has changed, False otherwise. + """ + # Compare role values + previous_roles = set(self.initial.get("roles", [])) + new_roles = set(self.cleaned_data.get("roles", [])) + + # Compare additional permissions values + previous_permissions = set(self.initial.get("additional_permissions") or []) + new_permissions = set(self.cleaned_data.get("additional_permissions") or []) + + return previous_roles != new_roles or previous_permissions != new_permissions + class PortfolioMemberForm(BasePortfolioMemberForm): """ diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 8feeb0794..99febc92e 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -9,6 +9,10 @@ from .utility.portfolio_helper import ( UserPortfolioPermissionChoices, UserPortfolioRoleChoices, cleanup_after_portfolio_member_deletion, + get_domain_requests_display, + get_domains_display, + get_members_display, + get_role_display, validate_portfolio_invitation, ) # type: ignore from .utility.time_stamped_model import TimeStampedModel @@ -85,6 +89,60 @@ class PortfolioInvitation(TimeStampedModel): """ return UserPortfolioPermission.get_portfolio_permissions(self.roles, self.additional_permissions) + @property + def role_display(self): + """ + Returns a human-readable display name for the user's role. + + Uses the `get_role_display` function to determine if the user is an "Admin", + "Basic" member, or has no role assigned. + + Returns: + str: The display name of the user's role. + """ + return get_role_display(self.roles) + + @property + def domains_display(self): + """ + Returns a string representation of the user's domain access level. + + Uses the `get_domains_display` function to determine whether the user has + "Viewer" access (can view all domains) or "Viewer, limited" access. + + Returns: + str: The display name of the user's domain permissions. + """ + return get_domains_display(self.roles, self.additional_permissions) + + @property + def domain_requests_display(self): + """ + Returns a string representation of the user's access to domain requests. + + Uses the `get_domain_requests_display` function to determine if the user + is a "Creator" (can create and edit requests), a "Viewer" (can only view requests), + or has "No access" to domain requests. + + Returns: + str: The display name of the user's domain request permissions. + """ + return get_domain_requests_display(self.roles, self.additional_permissions) + + @property + def members_display(self): + """ + Returns a string representation of the user's access to managing members. + + Uses the `get_members_display` function to determine if the user is a + "Manager" (can edit members), a "Viewer" (can view members), or has "No access" + to member management. + + Returns: + str: The display name of the user's member management permissions. + """ + return get_members_display(self.roles, self.additional_permissions) + @transition(field="status", source=PortfolioInvitationStatus.INVITED, target=PortfolioInvitationStatus.RETRIEVED) def retrieve(self): """When an invitation is retrieved, create the corresponding permission. diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 82a0465c5..d5476ab9a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -269,7 +269,7 @@ class User(AbstractUser): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) def is_portfolio_admin(self, portfolio): - return "Admin" in self.portfolio_role_summary(portfolio) + return self.has_edit_portfolio_permission(portfolio) def get_first_portfolio(self): permission = self.portfolio_permissions.first() @@ -277,49 +277,6 @@ class User(AbstractUser): return permission.portfolio return None - def portfolio_role_summary(self, portfolio): - """Returns a list of roles based on the user's permissions.""" - roles = [] - - # Define the conditions and their corresponding roles - conditions_roles = [ - (self.has_edit_portfolio_permission(portfolio), ["Admin"]), - ( - self.has_view_all_domains_portfolio_permission(portfolio) - and self.has_any_requests_portfolio_permission(portfolio) - and self.has_edit_request_portfolio_permission(portfolio), - ["View-only admin", "Domain requestor"], - ), - ( - self.has_view_all_domains_portfolio_permission(portfolio) - and self.has_any_requests_portfolio_permission(portfolio), - ["View-only admin"], - ), - ( - self.has_view_portfolio_permission(portfolio) - and self.has_edit_request_portfolio_permission(portfolio) - and self.has_any_domains_portfolio_permission(portfolio), - ["Domain requestor", "Domain manager"], - ), - ( - self.has_view_portfolio_permission(portfolio) and self.has_edit_request_portfolio_permission(portfolio), - ["Domain requestor"], - ), - ( - self.has_view_portfolio_permission(portfolio) and self.has_any_domains_portfolio_permission(portfolio), - ["Domain manager"], - ), - (self.has_view_portfolio_permission(portfolio), ["Member"]), - ] - - # Evaluate conditions and add roles - for condition, role_list in conditions_roles: - if condition: - roles.extend(role_list) - break - - return roles - def get_portfolios(self): return self.portfolio_permissions.all() diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 5378dc185..e077daa57 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -6,6 +6,10 @@ from registrar.models.utility.portfolio_helper import ( DomainRequestPermissionDisplay, MemberPermissionDisplay, cleanup_after_portfolio_member_deletion, + get_domain_requests_display, + get_domains_display, + get_members_display, + get_role_display, validate_user_portfolio_permission, ) from .utility.time_stamped_model import TimeStampedModel @@ -181,6 +185,60 @@ class UserPortfolioPermission(TimeStampedModel): # This is the same as portfolio_permissions & common_forbidden_perms. return portfolio_permissions.intersection(common_forbidden_perms) + @property + def role_display(self): + """ + Returns a human-readable display name for the user's role. + + Uses the `get_role_display` function to determine if the user is an "Admin", + "Basic" member, or has no role assigned. + + Returns: + str: The display name of the user's role. + """ + return get_role_display(self.roles) + + @property + def domains_display(self): + """ + Returns a string representation of the user's domain access level. + + Uses the `get_domains_display` function to determine whether the user has + "Viewer" access (can view all domains) or "Viewer, limited" access. + + Returns: + str: The display name of the user's domain permissions. + """ + return get_domains_display(self.roles, self.additional_permissions) + + @property + def domain_requests_display(self): + """ + Returns a string representation of the user's access to domain requests. + + Uses the `get_domain_requests_display` function to determine if the user + is a "Creator" (can create and edit requests), a "Viewer" (can only view requests), + or has "No access" to domain requests. + + Returns: + str: The display name of the user's domain request permissions. + """ + return get_domain_requests_display(self.roles, self.additional_permissions) + + @property + def members_display(self): + """ + Returns a string representation of the user's access to managing members. + + Uses the `get_members_display` function to determine if the user is a + "Manager" (can edit members), a "Viewer" (can view members), or has "No access" + to member management. + + Returns: + str: The display name of the user's member management permissions. + """ + return get_members_display(self.roles, self.additional_permissions) + def clean(self): """Extends clean method to perform additional validation, which can raise errors in django admin.""" super().clean() diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 5feae1cc1..03733237e 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -79,6 +79,100 @@ class MemberPermissionDisplay(StrEnum): NONE = "None" +def get_role_display(roles): + """ + Returns a user-friendly display name for a given list of user roles. + + - If the user has the ORGANIZATION_ADMIN role, return "Admin". + - If the user has the ORGANIZATION_MEMBER role, return "Basic". + - If the user has neither role, return "-". + + Args: + roles (list): A list of role strings assigned to the user. + + Returns: + str: The display name for the highest applicable role. + """ + if UserPortfolioRoleChoices.ORGANIZATION_ADMIN in roles: + return "Admin" + elif UserPortfolioRoleChoices.ORGANIZATION_MEMBER in roles: + return "Basic" + else: + return "-" + + +def get_domains_display(roles, permissions): + """ + Determines the display name for a user's domain viewing permissions. + + - If the user has the VIEW_ALL_DOMAINS permission, return "Viewer". + - Otherwise, return "Viewer, limited". + + Args: + roles (list): A list of role strings assigned to the user. + permissions (list): A list of additional permissions assigned to the user. + + Returns: + str: A string representing the user's domain viewing access. + """ + UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission") + all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, permissions) + if UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS in all_permissions: + return "Viewer" + else: + return "Viewer, limited" + + +def get_domain_requests_display(roles, permissions): + """ + Determines the display name for a user's domain request permissions. + + - If the user has the EDIT_REQUESTS permission, return "Creator". + - If the user has the VIEW_ALL_REQUESTS permission, return "Viewer". + - Otherwise, return "No access". + + Args: + roles (list): A list of role strings assigned to the user. + permissions (list): A list of additional permissions assigned to the user. + + Returns: + str: A string representing the user's domain request access level. + """ + UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission") + all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, permissions) + if UserPortfolioPermissionChoices.EDIT_REQUESTS in all_permissions: + return "Creator" + elif UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in all_permissions: + return "Viewer" + else: + return "No access" + + +def get_members_display(roles, permissions): + """ + Determines the display name for a user's member management permissions. + + - If the user has the EDIT_MEMBERS permission, return "Manager". + - If the user has the VIEW_MEMBERS permission, return "Viewer". + - Otherwise, return "No access". + + Args: + roles (list): A list of role strings assigned to the user. + permissions (list): A list of additional permissions assigned to the user. + + Returns: + str: A string representing the user's member management access level. + """ + UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission") + all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, permissions) + if UserPortfolioPermissionChoices.EDIT_MEMBERS in all_permissions: + return "Manager" + elif UserPortfolioPermissionChoices.VIEW_MEMBERS in all_permissions: + return "Viewer" + else: + return "No access" + + def validate_user_portfolio_permission(user_portfolio_permission): """ Validates a UserPortfolioPermission instance. Located in portfolio_helper to avoid circular imports diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html index d07e5abf4..6624be95d 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html @@ -10,7 +10,6 @@
Admin
-{% elif permissions.roles and 'organization_member' in permissions.roles %} -Basic
-{% else %} -⎯
-{% endif %} +{{ permissions.role_display }}
Viewer, all
-{% else %} -Viewer, limited
-{% endif %} +{{ permissions.domains_display }}
Creator
-{% elif member_has_view_all_requests_portfolio_permission %} -Viewer
-{% else %} -No access
-{% endif %} +{{ permissions.domain_requests_display }}
Manager
-{% elif member_has_view_members_portfolio_permission %} -Viewer
-{% else %} -No access
-{% endif %} \ No newline at end of file +{{ permissions.members_display }}
diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index d21678d58..ff73e6dc1 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -251,15 +251,6 @@ def is_members_subpage(path): return get_url_name(path) in url_names -@register.filter(name="portfolio_role_summary") -def portfolio_role_summary(user, portfolio): - """Returns the value of user.portfolio_role_summary""" - if user and portfolio: - return user.portfolio_role_summary(portfolio) - else: - return [] - - @register.filter(name="display_requesting_entity") def display_requesting_entity(domain_request): """Workaround for a newline issue in .txt files (our emails) as if statements diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py index 77a8c402f..981bca6dd 100644 --- a/src/registrar/tests/test_email_invitations.py +++ b/src/registrar/tests/test_email_invitations.py @@ -16,6 +16,7 @@ from registrar.utility.email_invitations import ( send_portfolio_admin_addition_emails, send_portfolio_admin_removal_emails, send_portfolio_invitation_email, + send_portfolio_member_permission_update_email, ) from api.tests.common import less_console_noise_decorator @@ -522,7 +523,6 @@ class PortfolioInvitationEmailTests(unittest.TestCase): "registrar.utility.email_invitations._get_requestor_email", side_effect=MissingEmailError("Requestor has no email"), ) - @less_console_noise_decorator def test_send_portfolio_invitation_email_missing_requestor_email(self, mock_get_email): """Test when requestor has no email""" is_admin_invitation = False @@ -888,3 +888,78 @@ class SendPortfolioAdminRemovalEmailsTests(unittest.TestCase): mock_get_requestor_email.assert_called_once_with(self.requestor, portfolio=self.portfolio) mock_send_removal_emails.assert_called_once_with(self.email, self.requestor.email, self.portfolio) self.assertFalse(result) + + +class TestSendPortfolioMemberPermissionUpdateEmail(unittest.TestCase): + """Unit tests for send_portfolio_member_permission_update_email function.""" + + @patch("registrar.utility.email_invitations.send_templated_email") + @patch("registrar.utility.email_invitations._get_requestor_email") + def test_send_email_success(self, mock_get_requestor_email, mock_send_email): + """Test that the email is sent successfully when there are no errors.""" + # Mock data + requestor = MagicMock() + permissions = MagicMock(spec=UserPortfolioPermission) + permissions.user.email = "user@example.com" + permissions.portfolio.organization_name = "Test Portfolio" + + mock_get_requestor_email.return_value = "requestor@example.com" + + # Call function + result = send_portfolio_member_permission_update_email(requestor, permissions) + + # Assertions + mock_get_requestor_email.assert_called_once_with(requestor, portfolio=permissions.portfolio) + mock_send_email.assert_called_once_with( + "emails/portfolio_update.txt", + "emails/portfolio_update_subject.txt", + to_address="user@example.com", + context={ + "requested_user": permissions.user, + "portfolio": permissions.portfolio, + "requestor_email": "requestor@example.com", + "permissions": permissions, + "date": date.today(), + }, + ) + self.assertTrue(result) + + @patch("registrar.utility.email_invitations.send_templated_email", side_effect=EmailSendingError("Email failed")) + @patch("registrar.utility.email_invitations._get_requestor_email") + @patch("registrar.utility.email_invitations.logger") + def test_send_email_failure(self, mock_logger, mock_get_requestor_email, mock_send_email): + """Test that the function returns False and logs an error when email sending fails.""" + # Mock data + requestor = MagicMock() + permissions = MagicMock(spec=UserPortfolioPermission) + permissions.user.email = "user@example.com" + permissions.portfolio.organization_name = "Test Portfolio" + + mock_get_requestor_email.return_value = "requestor@example.com" + + # Call function + result = send_portfolio_member_permission_update_email(requestor, permissions) + + # Assertions + mock_logger.warning.assert_called_once_with( + "Could not send email organization member update notification to %s for portfolio: %s", + permissions.user.email, + permissions.portfolio.organization_name, + exc_info=True, + ) + self.assertFalse(result) + + @patch("registrar.utility.email_invitations._get_requestor_email", side_effect=Exception("Unexpected error")) + @patch("registrar.utility.email_invitations.logger") + def test_requestor_email_retrieval_failure(self, mock_logger, mock_get_requestor_email): + """Test that an exception in retrieving requestor email is logged.""" + # Mock data + requestor = MagicMock() + permissions = MagicMock(spec=UserPortfolioPermission) + + # Call function + with self.assertRaises(Exception): + send_portfolio_member_permission_update_email(requestor, permissions) + + # Assertions + mock_logger.warning.assert_not_called() # Function should fail before logging email failure diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index f39f11517..2b7f89ac9 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -108,6 +108,82 @@ class TestEmails(TestCase): self.assertEqual(["testy2@town.com", "mayor@igorville.gov"], kwargs["Destination"]["CcAddresses"]) + @boto3_mocking.patching + @override_settings(IS_PRODUCTION=True, BASE_URL="manage.get.gov") + def test_email_production_subject_and_url_check(self): + """Test sending an email in production that: + 1. Does not have a prefix in the email subject (no [MANAGE]) + 2. Uses the production URL in the email body of manage.get.gov still""" + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + send_templated_email( + "emails/update_to_approved_domain.txt", + "emails/update_to_approved_domain_subject.txt", + "doesnotexist@igorville.com", + context={"domain": "test", "user": "test", "date": 1, "changes": "test"}, + bcc_address=None, + cc_addresses=["testy2@town.com", "mayor@igorville.gov"], + ) + + # check that an email was sent + self.assertTrue(self.mock_client.send_email.called) + + # check the call sequence for the email + args, kwargs = self.mock_client.send_email.call_args + self.assertIn("Destination", kwargs) + self.assertIn("CcAddresses", kwargs["Destination"]) + + self.assertEqual(["testy2@town.com", "mayor@igorville.gov"], kwargs["Destination"]["CcAddresses"]) + + # Grab email subject + email_subject = kwargs["Content"]["Simple"]["Subject"]["Data"] + + # Check that the subject does NOT contain a prefix for production + self.assertNotIn("[MANAGE]", email_subject) + self.assertIn("An update was made to", email_subject) + + # Grab email body + email_body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + + # Check that manage_url is correctly set for production + self.assertIn("https://manage.get.gov", email_body) + + @boto3_mocking.patching + @override_settings(IS_PRODUCTION=False, BASE_URL="https://getgov-rh.app.cloud.gov") + def test_email_non_production_subject_and_url_check(self): + """Test sending an email in production that: + 1. Does prefix in the email subject (ie [GETGOV-RH]) + 2. Uses the sandbox url in the email body (ie getgov-rh.app.cloud.gov)""" + with boto3_mocking.clients.handler_for("sesv2", self.mock_client_class): + send_templated_email( + "emails/update_to_approved_domain.txt", + "emails/update_to_approved_domain_subject.txt", + "doesnotexist@igorville.com", + context={"domain": "test", "user": "test", "date": 1, "changes": "test"}, + bcc_address=None, + cc_addresses=["testy2@town.com", "mayor@igorville.gov"], + ) + + # check that an email was sent + self.assertTrue(self.mock_client.send_email.called) + + # check the call sequence for the email + args, kwargs = self.mock_client.send_email.call_args + self.assertIn("Destination", kwargs) + self.assertIn("CcAddresses", kwargs["Destination"]) + self.assertEqual(["testy2@town.com", "mayor@igorville.gov"], kwargs["Destination"]["CcAddresses"]) + + # Grab email subject + email_subject = kwargs["Content"]["Simple"]["Subject"]["Data"] + + # Check that the subject DOES contain a prefix of the current sandbox + self.assertIn("[GETGOV-RH]", email_subject) + + # Grab email body + email_body = kwargs["Content"]["Simple"]["Body"]["Text"]["Data"] + + # Check that manage_url is correctly set of the sandbox + self.assertIn("https://getgov-rh.app.cloud.gov", email_body) + @boto3_mocking.patching @less_console_noise_decorator def test_submission_confirmation(self): diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 4401b73e8..cd2fe868d 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1191,67 +1191,6 @@ class TestUser(TestCase): User.objects.all().delete() UserDomainRole.objects.all().delete() - @patch.object(User, "has_edit_portfolio_permission", return_value=True) - def test_portfolio_role_summary_admin(self, mock_edit_org): - # Test if the user is recognized as an Admin - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Admin"]) - - @patch.multiple( - User, - has_view_all_domains_portfolio_permission=lambda self, portfolio: True, - has_any_requests_portfolio_permission=lambda self, portfolio: True, - has_edit_request_portfolio_permission=lambda self, portfolio: True, - ) - def test_portfolio_role_summary_view_only_admin_and_domain_requestor(self): - # Test if the user has both 'View-only admin' and 'Domain requestor' roles - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["View-only admin", "Domain requestor"]) - - @patch.multiple( - User, - has_view_all_domains_portfolio_permission=lambda self, portfolio: True, - has_any_requests_portfolio_permission=lambda self, portfolio: True, - ) - def test_portfolio_role_summary_view_only_admin(self): - # Test if the user is recognized as a View-only admin - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["View-only admin"]) - - @patch.multiple( - User, - has_view_portfolio_permission=lambda self, portfolio: True, - has_edit_request_portfolio_permission=lambda self, portfolio: True, - has_any_domains_portfolio_permission=lambda self, portfolio: True, - ) - def test_portfolio_role_summary_member_domain_requestor_domain_manager(self): - # Test if the user has 'Member', 'Domain requestor', and 'Domain manager' roles - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor", "Domain manager"]) - - @patch.multiple( - User, - has_view_portfolio_permission=lambda self, portfolio: True, - has_edit_request_portfolio_permission=lambda self, portfolio: True, - ) - def test_portfolio_role_summary_member_domain_requestor(self): - # Test if the user has 'Member' and 'Domain requestor' roles - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain requestor"]) - - @patch.multiple( - User, - has_view_portfolio_permission=lambda self, portfolio: True, - has_any_domains_portfolio_permission=lambda self, portfolio: True, - ) - def test_portfolio_role_summary_member_domain_manager(self): - # Test if the user has 'Member' and 'Domain manager' roles - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Domain manager"]) - - @patch.multiple(User, has_view_portfolio_permission=lambda self, portfolio: True) - def test_portfolio_role_summary_member(self): - # Test if the user is recognized as a Member - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Member"]) - - def test_portfolio_role_summary_empty(self): - # Test if the user has no roles - self.assertEqual(self.user.portfolio_role_summary(self.portfolio), []) - @patch("registrar.models.User._has_portfolio_permission") def test_has_view_portfolio_permission(self, mock_has_permission): mock_has_permission.return_value = True diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 097aa1879..b9643f771 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -1063,7 +1063,7 @@ class TestPortfolio(WebTest): self.assertContains(response, "Invited") self.assertContains(response, portfolio_invitation.email) self.assertContains(response, "Admin") - self.assertContains(response, "Viewer, all") + self.assertContains(response, "Viewer") self.assertContains(response, "Creator") self.assertContains(response, "Manager") self.assertContains( @@ -3970,7 +3970,10 @@ class TestPortfolioMemberEditView(WebTest): @override_flag("organization_members", active=True) @patch("registrar.views.portfolios.send_portfolio_admin_addition_emails") @patch("registrar.views.portfolios.send_portfolio_admin_removal_emails") - def test_edit_member_permissions_basic_to_admin(self, mock_send_removal_emails, mock_send_addition_emails): + @patch("registrar.views.portfolios.send_portfolio_member_permission_update_email") + def test_edit_member_permissions_basic_to_admin( + self, mock_send_update_email, mock_send_removal_emails, mock_send_addition_emails + ): """Tests converting a basic member to admin with full permissions.""" self.client.force_login(self.user) @@ -3985,6 +3988,7 @@ class TestPortfolioMemberEditView(WebTest): # return indicator that notification emails sent successfully mock_send_addition_emails.return_value = True + mock_send_update_email.return_value = True response = self.client.post( reverse("member-permissions", kwargs={"pk": basic_permission.id}), @@ -4004,6 +4008,8 @@ class TestPortfolioMemberEditView(WebTest): mock_send_addition_emails.assert_called_once() # assert removal emails are not sent mock_send_removal_emails.assert_not_called() + # assert update email sent + mock_send_update_email.assert_called_once() # Get the arguments passed to send_portfolio_admin_addition_emails _, called_kwargs = mock_send_addition_emails.call_args @@ -4013,14 +4019,22 @@ class TestPortfolioMemberEditView(WebTest): self.assertEqual(called_kwargs["requestor"], self.user) self.assertEqual(called_kwargs["portfolio"], self.portfolio) + # Get the arguments passed to send_portfolio_member_permission_update_email + _, called_kwargs = mock_send_update_email.call_args + + # Assert the update notification email content + self.assertEqual(called_kwargs["requestor"], self.user) + self.assertEqual(called_kwargs["permissions"], basic_permission) + @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @patch("django.contrib.messages.warning") @patch("registrar.views.portfolios.send_portfolio_admin_addition_emails") @patch("registrar.views.portfolios.send_portfolio_admin_removal_emails") + @patch("registrar.views.portfolios.send_portfolio_member_permission_update_email") def test_edit_member_permissions_basic_to_admin_notification_fails( - self, mock_send_removal_emails, mock_send_addition_emails, mock_messages_warning + self, mock_send_update_email, mock_send_removal_emails, mock_send_addition_emails, mock_messages_warning ): """Tests converting a basic member to admin with full permissions. Handle when notification emails fail to send.""" @@ -4037,6 +4051,7 @@ class TestPortfolioMemberEditView(WebTest): # At least one notification email failed to send mock_send_addition_emails.return_value = False + mock_send_update_email.return_value = False response = self.client.post( reverse("member-permissions", kwargs={"pk": basic_permission.id}), @@ -4056,6 +4071,8 @@ class TestPortfolioMemberEditView(WebTest): mock_send_addition_emails.assert_called_once() # assert no removal emails are sent mock_send_removal_emails.assert_not_called() + # assert update email sent + mock_send_update_email.assert_called_once() # Get the arguments passed to send_portfolio_admin_addition_emails _, called_kwargs = mock_send_addition_emails.call_args @@ -4065,18 +4082,32 @@ class TestPortfolioMemberEditView(WebTest): self.assertEqual(called_kwargs["requestor"], self.user) self.assertEqual(called_kwargs["portfolio"], self.portfolio) - # Assert warning message is called correctly - mock_messages_warning.assert_called_once() - warning_args, _ = mock_messages_warning.call_args - self.assertIsInstance(warning_args[0], WSGIRequest) - self.assertEqual(warning_args[1], "Could not send email notification to existing organization admins.") + # Get the arguments passed to send_portfolio_member_permission_update_email + _, called_kwargs = mock_send_update_email.call_args + + # Assert the update notification email content + self.assertEqual(called_kwargs["requestor"], self.user) + self.assertEqual(called_kwargs["permissions"], basic_permission) + + # Assert that messages.warning is called twice + self.assertEqual(mock_messages_warning.call_count, 2) + + # Extract the actual messages sent + warning_messages = [call_args[0][1] for call_args in mock_messages_warning.call_args_list] + + # Check for the expected messages + self.assertIn("Could not send email notification to existing organization admins.", warning_messages) + self.assertIn(f"Could not send email notification to {basic_member.email}.", warning_messages) @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @patch("registrar.views.portfolios.send_portfolio_admin_addition_emails") @patch("registrar.views.portfolios.send_portfolio_admin_removal_emails") - def test_edit_member_permissions_admin_to_admin(self, mock_send_removal_emails, mock_send_addition_emails): + @patch("registrar.views.portfolios.send_portfolio_member_permission_update_email") + def test_edit_member_permissions_admin_to_admin( + self, mock_send_update_email, mock_send_removal_emails, mock_send_addition_emails + ): """Tests updating an admin without changing permissions.""" self.client.force_login(self.user) @@ -4086,6 +4117,7 @@ class TestPortfolioMemberEditView(WebTest): user=admin_member, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[], ) response = self.client.post( @@ -4098,16 +4130,20 @@ class TestPortfolioMemberEditView(WebTest): # Verify redirect and success message self.assertEqual(response.status_code, 302) - # assert addition and removal emails are not sent to portfolio admins + # assert update, addition and removal emails are not sent to portfolio admins mock_send_addition_emails.assert_not_called() mock_send_removal_emails.assert_not_called() + mock_send_update_email.assert_not_called() @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @patch("registrar.views.portfolios.send_portfolio_admin_addition_emails") @patch("registrar.views.portfolios.send_portfolio_admin_removal_emails") - def test_edit_member_permissions_basic_to_basic(self, mock_send_removal_emails, mock_send_addition_emails): + @patch("registrar.views.portfolios.send_portfolio_member_permission_update_email") + def test_edit_member_permissions_basic_to_basic( + self, mock_send_update_email, mock_send_removal_emails, mock_send_addition_emails + ): """Tests updating an admin without changing permissions.""" self.client.force_login(self.user) @@ -4120,6 +4156,8 @@ class TestPortfolioMemberEditView(WebTest): additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS], ) + mock_send_update_email.return_value = True + response = self.client.post( reverse("member-permissions", kwargs={"pk": basic_permission.id}), { @@ -4136,13 +4174,25 @@ class TestPortfolioMemberEditView(WebTest): # assert addition and removal emails are not sent to portfolio admins mock_send_addition_emails.assert_not_called() mock_send_removal_emails.assert_not_called() + # assert update email is sent to updated member + mock_send_update_email.assert_called_once() + + # Get the arguments passed to send_portfolio_member_permission_update_email + _, called_kwargs = mock_send_update_email.call_args + + # Assert the email content + self.assertEqual(called_kwargs["requestor"], self.user) + self.assertEqual(called_kwargs["permissions"], basic_permission) @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @patch("registrar.views.portfolios.send_portfolio_admin_addition_emails") @patch("registrar.views.portfolios.send_portfolio_admin_removal_emails") - def test_edit_member_permissions_admin_to_basic(self, mock_send_removal_emails, mock_send_addition_emails): + @patch("registrar.views.portfolios.send_portfolio_member_permission_update_email") + def test_edit_member_permissions_admin_to_basic( + self, mock_send_update_email, mock_send_removal_emails, mock_send_addition_emails + ): """Tests converting an admin to basic member.""" self.client.force_login(self.user) @@ -4153,8 +4203,9 @@ class TestPortfolioMemberEditView(WebTest): portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], ) - + print(admin_permission) mock_send_removal_emails.return_value = True + mock_send_update_email.return_value = True response = self.client.post( reverse("member-permissions", kwargs={"pk": admin_permission.id}), @@ -4173,7 +4224,8 @@ class TestPortfolioMemberEditView(WebTest): admin_permission.refresh_from_db() self.assertEqual(admin_permission.roles, [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]) - # assert removal emails are sent to portfolio admins + # assert removal emails and update email are sent to portfolio admins + mock_send_update_email.assert_called_once() mock_send_addition_emails.assert_not_called() mock_send_removal_emails.assert_called_once() @@ -4185,14 +4237,22 @@ class TestPortfolioMemberEditView(WebTest): self.assertEqual(called_kwargs["requestor"], self.user) self.assertEqual(called_kwargs["portfolio"], self.portfolio) + # Get the arguments passed to send_portfolio_member_permission_update_email + _, called_kwargs = mock_send_update_email.call_args + + # Assert the email content + self.assertEqual(called_kwargs["requestor"], self.user) + self.assertEqual(called_kwargs["permissions"], admin_permission) + @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @patch("django.contrib.messages.warning") @patch("registrar.views.portfolios.send_portfolio_admin_addition_emails") @patch("registrar.views.portfolios.send_portfolio_admin_removal_emails") + @patch("registrar.views.portfolios.send_portfolio_member_permission_update_email") def test_edit_member_permissions_admin_to_basic_notification_fails( - self, mock_send_removal_emails, mock_send_addition_emails, mock_messages_warning + self, mock_send_update_email, mock_send_removal_emails, mock_send_addition_emails, mock_messages_warning ): """Tests converting an admin to basic member.""" self.client.force_login(self.user) @@ -4208,6 +4268,7 @@ class TestPortfolioMemberEditView(WebTest): # False return indicates that at least one notification email failed to send mock_send_removal_emails.return_value = False + mock_send_update_email.return_value = False response = self.client.post( reverse("member-permissions", kwargs={"pk": admin_permission.id}), @@ -4226,9 +4287,10 @@ class TestPortfolioMemberEditView(WebTest): admin_permission.refresh_from_db() self.assertEqual(admin_permission.roles, [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]) - # assert removal emails are sent to portfolio admins + # assert update email and removal emails are sent to portfolio admins mock_send_addition_emails.assert_not_called() mock_send_removal_emails.assert_called_once() + mock_send_update_email.assert_called_once() # Get the arguments passed to send_portfolio_admin_removal_emails _, called_kwargs = mock_send_removal_emails.call_args @@ -4238,11 +4300,22 @@ class TestPortfolioMemberEditView(WebTest): self.assertEqual(called_kwargs["requestor"], self.user) self.assertEqual(called_kwargs["portfolio"], self.portfolio) - # Assert warning message is called correctly - mock_messages_warning.assert_called_once() - warning_args, _ = mock_messages_warning.call_args - self.assertIsInstance(warning_args[0], WSGIRequest) - self.assertEqual(warning_args[1], "Could not send email notification to existing organization admins.") + # Get the arguments passed to send_portfolio_member_permission_update_email + _, called_kwargs = mock_send_update_email.call_args + + # Assert the email content + self.assertEqual(called_kwargs["requestor"], self.user) + self.assertEqual(called_kwargs["permissions"], admin_permission) + + # Assert that messages.warning is called twice + self.assertEqual(mock_messages_warning.call_count, 2) + + # Extract the actual messages sent + warning_messages = [call_args[0][1] for call_args in mock_messages_warning.call_args_list] + + # Check for the expected messages + self.assertIn("Could not send email notification to existing organization admins.", warning_messages) + self.assertIn(f"Could not send email notification to {admin_member.email}.", warning_messages) @less_console_noise_decorator @override_flag("organization_feature", active=True) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 40601cdc7..94e87a96b 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -3,6 +3,7 @@ import boto3 import logging import textwrap +import re from datetime import datetime from django.apps import apps from django.conf import settings @@ -48,6 +49,21 @@ def send_templated_email( # noqa No valid recipient addresses are provided """ + if context is None: + context = {} + + env_base_url = settings.BASE_URL + # The regular expression is to get both http (localhost) and https (everything else) + env_name = re.sub(r"^https?://", "", env_base_url).split(".")[0] + # If NOT in prod, add env to the subject line + # IE adds [GETGOV-RH] if we are in the -RH sandbox + prefix = f"[{env_name.upper()}] " if not settings.IS_PRODUCTION else "" + # If NOT in prod, update instances of "manage.get.gov" links to point to + # current environment, ie "getgov-rh.app.cloud.gov" + manage_url = env_base_url if not settings.IS_PRODUCTION else "https://manage.get.gov" + + context["manage_url"] = manage_url + # by default assume we can send to all addresses (prod has no whitelist) sendable_cc_addresses = cc_addresses @@ -70,8 +86,10 @@ def send_templated_email( # noqa if email_body: email_body.strip().lstrip("\n") + # Update the subject to have prefix here versus every email subject_template = get_template(subject_template_name) subject = subject_template.render(context=context) + subject = f"{prefix}{subject}" try: ses_client = boto3.client( diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index de21b2a61..7ddab65f1 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -226,6 +226,49 @@ def send_portfolio_invitation_email(email: str, requestor, portfolio, is_admin_i return all_admin_emails_sent +def send_portfolio_member_permission_update_email(requestor, permissions: UserPortfolioPermission): + """ + Sends an email notification to a portfolio member when their permissions are updated. + + This function retrieves the requestor's email and sends a templated email to the affected user, + notifying them of changes to their portfolio permissions. + + Args: + requestor (User): The user initiating the permission update. + permissions (UserPortfolioPermission): The updated permissions object containing the affected user + and the portfolio details. + + Returns: + bool: True if the email was sent successfully, False if an EmailSendingError occurred. + + Raises: + MissingEmailError: If the requestor has no email associated with their account. + """ + requestor_email = _get_requestor_email(requestor, portfolio=permissions.portfolio) + try: + send_templated_email( + "emails/portfolio_update.txt", + "emails/portfolio_update_subject.txt", + to_address=permissions.user.email, + context={ + "requested_user": permissions.user, + "portfolio": permissions.portfolio, + "requestor_email": requestor_email, + "permissions": permissions, + "date": date.today(), + }, + ) + except EmailSendingError: + logger.warning( + "Could not send email organization member update notification to %s " "for portfolio: %s", + permissions.user.email, + permissions.portfolio.organization_name, + exc_info=True, + ) + return False + return True + + def send_portfolio_admin_addition_emails(email: str, requestor, portfolio: Portfolio): """ Notifies all portfolio admins of the provided portfolio of a newly invited portfolio admin diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 0f93ec8e1..c3f0f0152 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -20,6 +20,7 @@ from registrar.utility.email_invitations import ( send_portfolio_admin_addition_emails, send_portfolio_admin_removal_emails, send_portfolio_invitation_email, + send_portfolio_member_permission_update_email, ) from registrar.utility.errors import MissingEmailError from registrar.utility.enums import DefaultUserValues @@ -212,6 +213,11 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): removing_admin_role_on_self = False if form.is_valid(): try: + if form.is_change(): + if not send_portfolio_member_permission_update_email( + requestor=request.user, permissions=form.instance + ): + messages.warning(self.request, f"Could not send email notification to {user.email}.") if form.is_change_from_member_to_admin(): if not send_portfolio_admin_addition_emails( email=portfolio_permission.user.email,