From 0e6bc6f07f13122dda318a86ca5e85ea59d71ee3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 5 Feb 2025 06:29:10 -0500 Subject: [PATCH 01/23] added helpers for role and permissions displays in templates --- src/registrar/models/portfolio_invitation.py | 58 ++++++++++++ .../models/user_portfolio_permission.py | 58 ++++++++++++ .../models/utility/portfolio_helper.py | 89 +++++++++++++++++++ .../templates/emails/portfolio_update.txt | 35 ++++++++ .../emails/portfolio_update_subject.txt | 1 + .../includes/member_permissions_summary.html | 30 +------ 6 files changed, 245 insertions(+), 26 deletions(-) create mode 100644 src/registrar/templates/emails/portfolio_update.txt create mode 100644 src/registrar/templates/emails/portfolio_update_subject.txt diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 8feeb0794..dd80f946e 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, all" 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_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 11d9c56e3..372715db2 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 @@ -185,6 +189,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, all" 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 0864bded0..f1f9ef4f2 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -82,6 +82,95 @@ class MemberPermissionDisplay(StrEnum): VIEWER = "Viewer" 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, all". + - 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, all" + 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): """ diff --git a/src/registrar/templates/emails/portfolio_update.txt b/src/registrar/templates/emails/portfolio_update.txt new file mode 100644 index 000000000..aa13a9fb9 --- /dev/null +++ b/src/registrar/templates/emails/portfolio_update.txt @@ -0,0 +1,35 @@ +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +Hi,{% if requested_user and requested_user.first_name %} {{ requested_user.first_name }}.{% endif %} + +Your permissions were updated in the .gov registrar. + +ORGANIZATION: {{ portfolio.organization_name }} +UPDATED BY: {{ requestor_email }} +UPDATED ON: {{ date }} +YOUR PERMISSIONS: {{ permissions.role_display }} + Domains - {{ permissions.domains_display }} + Domain requests - {{ permissions.domain_requests_display }} + Members - {{ permissions.members_display }} + +Your updated permissions are now active in the .gov registrar . + +---------------------------------------------------------------- + +SOMETHING WRONG? +If you have questions or concerns, reach out to the person who updated your +permissions, or reply to this email. + + +THANK YOU +.Gov helps the public identify official, trusted information. Thank you for using a .gov +domain. + +---------------------------------------------------------------- + +The .gov team +Contact us: +Learn about .gov + +The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency +(CISA) +{% endautoescape %} diff --git a/src/registrar/templates/emails/portfolio_update_subject.txt b/src/registrar/templates/emails/portfolio_update_subject.txt new file mode 100644 index 000000000..2cd806a73 --- /dev/null +++ b/src/registrar/templates/emails/portfolio_update_subject.txt @@ -0,0 +1 @@ +Your permissions were updated in the .gov registrar \ No newline at end of file diff --git a/src/registrar/templates/includes/member_permissions_summary.html b/src/registrar/templates/includes/member_permissions_summary.html index 3a91d16f6..95eca0a7e 100644 --- a/src/registrar/templates/includes/member_permissions_summary.html +++ b/src/registrar/templates/includes/member_permissions_summary.html @@ -1,33 +1,11 @@

Member access

-{% if permissions.roles and 'organization_admin' in permissions.roles %} -

Admin

-{% elif permissions.roles and 'organization_member' in permissions.roles %} -

Basic

-{% else %} -

⎯

-{% endif %} +

{{ permissions.role_display }}

Domains

-{% if member_has_view_all_domains_portfolio_permission %} -

Viewer, all

-{% else %} -

Viewer, limited

-{% endif %} +

{{ permissions.domains_display }}

Domain requests

-{% if member_has_edit_request_portfolio_permission %} -

Creator

-{% elif member_has_view_all_requests_portfolio_permission %} -

Viewer

-{% else %} -

No access

-{% endif %} +

{{ permissions.domain_requests_display }}

Members

-{% if member_has_edit_members_portfolio_permission %} -

Manager

-{% elif member_has_view_members_portfolio_permission %} -

Viewer

-{% else %} -

No access

-{% endif %} \ No newline at end of file +

{{ permissions.members_display }}

From 350508f9c12f00576bfea74ebaf4d23506e336d7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 5 Feb 2025 16:31:22 -0500 Subject: [PATCH 02/23] email notification to portfolio member on permissions update --- src/registrar/forms/portfolio.py | 18 ++++++++++ src/registrar/utility/email_invitations.py | 40 ++++++++++++++++++++++ src/registrar/views/portfolios.py | 7 ++++ 3 files changed, 65 insertions(+) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 2725224f1..d244f9931 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -337,6 +337,24 @@ class BasePortfolioMemberForm(forms.ModelForm): UserPortfolioRoleChoices.ORGANIZATION_ADMIN in previous_roles 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", [])) + new_permissions = set(self.cleaned_data.get("additional_permissions", [])) + + return previous_roles != new_roles or previous_permissions != new_permissions class PortfolioMemberForm(BasePortfolioMemberForm): diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index de21b2a61..ed057a41b 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -225,6 +225,46 @@ 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, + } + ) + except EmailSendingError as err: + 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): """ diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index c5cc72c59..e2d031fca 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,12 @@ 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, From cbc9cdbe34dd22630a2aef2bd897bab3f168fb16 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 5 Feb 2025 17:13:02 -0500 Subject: [PATCH 03/23] additional tests for email_invitations --- src/registrar/forms/portfolio.py | 10 +-- src/registrar/models/portfolio_invitation.py | 16 ++-- .../models/user_portfolio_permission.py | 16 ++-- .../models/utility/portfolio_helper.py | 5 ++ src/registrar/tests/test_email_invitations.py | 76 ++++++++++++++++++- src/registrar/utility/email_invitations.py | 16 ++-- src/registrar/views/portfolios.py | 3 +- 7 files changed, 111 insertions(+), 31 deletions(-) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index d244f9931..a81f9d1e7 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -337,21 +337,21 @@ class BasePortfolioMemberForm(forms.ModelForm): UserPortfolioRoleChoices.ORGANIZATION_ADMIN in previous_roles and UserPortfolioRoleChoices.ORGANIZATION_ADMIN not in new_roles ) - + def is_change(self) -> bool: """ - Determines if the form has changed by comparing the initial data + 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", [])) + 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", [])) + previous_permissions = set(self.initial.get("additional_permissions", [])) new_permissions = set(self.cleaned_data.get("additional_permissions", [])) return previous_roles != new_roles or previous_permissions != new_permissions diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index dd80f946e..045cf2de4 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -101,41 +101,41 @@ class PortfolioInvitation(TimeStampedModel): 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 + Uses the `get_domains_display` function to determine whether the user has "Viewer, all" 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), + 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" + 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: diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 372715db2..7daa3bd11 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -201,41 +201,41 @@ class UserPortfolioPermission(TimeStampedModel): 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 + Uses the `get_domains_display` function to determine whether the user has "Viewer, all" 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), + 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" + 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: diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index f1f9ef4f2..67e546db8 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -82,6 +82,7 @@ class MemberPermissionDisplay(StrEnum): VIEWER = "Viewer" NONE = "None" + def get_role_display(roles): """ Returns a user-friendly display name for a given list of user roles. @@ -103,6 +104,7 @@ def get_role_display(roles): else: return "-" + def get_domains_display(roles, permissions): """ Determines the display name for a user's domain viewing permissions. @@ -124,6 +126,7 @@ def get_domains_display(roles, permissions): else: return "Viewer, limited" + def get_domain_requests_display(roles, permissions): """ Determines the display name for a user's domain request permissions. @@ -148,6 +151,7 @@ def get_domain_requests_display(roles, permissions): else: return "No access" + def get_members_display(roles, permissions): """ Determines the display name for a user's member management permissions. @@ -172,6 +176,7 @@ def get_members_display(roles, permissions): 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/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py index 77a8c402f..20ac4a565 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,77 @@ 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, + }, + ) + 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/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index ed057a41b..d206bf279 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -225,6 +225,7 @@ 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. @@ -234,7 +235,7 @@ def send_portfolio_member_permission_update_email(requestor, permissions: UserPo Args: requestor (User): The user initiating the permission update. - permissions (UserPortfolioPermission): The updated permissions object containing the affected user + permissions (UserPortfolioPermission): The updated permissions object containing the affected user and the portfolio details. Returns: @@ -254,18 +255,19 @@ def send_portfolio_member_permission_update_email(requestor, permissions: UserPo "portfolio": permissions.portfolio, "requestor_email": requestor_email, "permissions": permissions, - } + }, ) - except EmailSendingError as err: + 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, - ) + 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 e2d031fca..d63b5964e 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -215,8 +215,7 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): try: if form.is_change(): if not send_portfolio_member_permission_update_email( - requestor=request.user, - permissions=form.instance + 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(): From f5ef2002169a68be0beff835b11729e1b50800bf Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 5 Feb 2025 18:27:16 -0500 Subject: [PATCH 04/23] updated tests --- src/registrar/forms/portfolio.py | 4 +- src/registrar/tests/test_views_portfolio.py | 113 ++++++++++++++++---- 2 files changed, 95 insertions(+), 22 deletions(-) diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index a81f9d1e7..3a9074b2d 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -351,8 +351,8 @@ class BasePortfolioMemberForm(forms.ModelForm): new_roles = set(self.cleaned_data.get("roles", [])) # Compare additional permissions values - previous_permissions = set(self.initial.get("additional_permissions", [])) - new_permissions = set(self.cleaned_data.get("additional_permissions", [])) + 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 diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 0c7c56e74..65e0350ee 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -3891,7 +3891,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) @@ -3906,6 +3909,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}), @@ -3925,6 +3929,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 @@ -3934,14 +3940,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.""" @@ -3958,6 +3972,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}), @@ -3977,6 +3992,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 @@ -3986,18 +4003,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) @@ -4007,6 +4038,7 @@ class TestPortfolioMemberEditView(WebTest): user=admin_member, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[], ) response = self.client.post( @@ -4019,16 +4051,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) @@ -4041,6 +4077,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}), { @@ -4057,13 +4095,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) @@ -4074,8 +4124,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}), @@ -4094,7 +4145,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() @@ -4106,14 +4158,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) @@ -4129,6 +4189,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}), @@ -4147,9 +4208,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 @@ -4159,11 +4221,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) From 67ab988d1ee1ce8c4067cadda6d5c83f3c8fb90d Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 5 Feb 2025 22:50:01 -0700 Subject: [PATCH 05/23] removed role column --- .../admin/includes/portfolio/portfolio_members_table.html | 6 ------ 1 file changed, 6 deletions(-) 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 @@ Title Email Phone - Roles Action @@ -28,11 +27,6 @@ {% endif %} {{ member.user.phone }} - - {% for role in member.user|portfolio_role_summary:original %} - {{ role }} - {% endfor %} - {% if member.user.email %} From bb05f6f4657c1d6aaab06e3658b128ee86f744a8 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 11 Feb 2025 12:50:40 -0800 Subject: [PATCH 06/23] Add logic for getting envs and adding it to email subject and body --- src/registrar/config/settings.py | 1 + .../templates/emails/domain_invitation.txt | 2 +- .../emails/domain_invitation_subject.txt | 2 +- ...n_manager_deleted_notification_subject.txt | 2 +- .../emails/domain_manager_notification.txt | 2 +- .../domain_manager_notification_subject.txt | 2 +- .../emails/domain_request_withdrawn.txt | 2 +- .../domain_request_withdrawn_subject.txt | 2 +- .../templates/emails/metadata_body.txt | 2 +- .../templates/emails/metadata_subject.txt | 2 +- .../portfolio_admin_addition_notification.txt | 2 +- ...io_admin_addition_notification_subject.txt | 2 +- .../portfolio_admin_removal_notification.txt | 2 +- ...lio_admin_removal_notification_subject.txt | 2 +- .../templates/emails/portfolio_invitation.txt | 2 +- .../emails/portfolio_invitation_subject.txt | 2 +- .../emails/status_change_approved.txt | 2 +- .../emails/status_change_approved_subject.txt | 2 +- .../emails/status_change_subject.txt | 2 +- .../emails/submission_confirmation.txt | 2 +- .../submission_confirmation_subject.txt | 2 +- .../emails/transition_domain_invitation.txt | 2 +- .../transition_domain_invitation_subject.txt | 2 +- .../emails/update_to_approved_domain.txt | 4 ++-- .../update_to_approved_domain_subject.txt | 2 +- src/registrar/utility/email.py | 19 +++++++++++++++++++ 26 files changed, 45 insertions(+), 25 deletions(-) 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/templates/emails/domain_invitation.txt b/src/registrar/templates/emails/domain_invitation.txt index a077bff26..270786a7a 100644 --- a/src/registrar/templates/emails/domain_invitation.txt +++ b/src/registrar/templates/emails/domain_invitation.txt @@ -4,7 +4,7 @@ Hi,{% if requested_user and requested_user.first_name %} {{ requested_user.first {{ requestor_email }} has invited you to manage: {% for domain in domains %}{{ domain.name }} {% endfor %} -To manage domain information, visit the .gov registrar . +To manage domain information, visit the .gov registrar <{{ manage_url }}>. ---------------------------------------------------------------- {% if not requested_user %} diff --git a/src/registrar/templates/emails/domain_invitation_subject.txt b/src/registrar/templates/emails/domain_invitation_subject.txt index 9663346d0..9f15c38b4 100644 --- a/src/registrar/templates/emails/domain_invitation_subject.txt +++ b/src/registrar/templates/emails/domain_invitation_subject.txt @@ -1 +1 @@ -You've been invited to manage {% if domains|length > 1 %}.gov domains{% else %}{{ domains.0.name }}{% endif %} \ No newline at end of file +{{ prefix }}You've been invited to manage {% if domains|length > 1 %}.gov domains{% else %}{{ domains.0.name }}{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt b/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt index c84a20f18..7376bdb86 100644 --- a/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt +++ b/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt @@ -1 +1 @@ -A domain manager was removed from {{ domain.name }} \ No newline at end of file +{{ prefix }}A domain manager was removed from {{ domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_manager_notification.txt b/src/registrar/templates/emails/domain_manager_notification.txt index c253937e4..18e682329 100644 --- a/src/registrar/templates/emails/domain_manager_notification.txt +++ b/src/registrar/templates/emails/domain_manager_notification.txt @@ -15,7 +15,7 @@ The person who received the invitation will become a domain manager once they lo associated with the invited email address. If you need to cancel this invitation or remove the domain manager, you can do that by going to -this domain in the .gov registrar . +this domain in the .gov registrar <{{ manage_url }}. WHY DID YOU RECEIVE THIS EMAIL? diff --git a/src/registrar/templates/emails/domain_manager_notification_subject.txt b/src/registrar/templates/emails/domain_manager_notification_subject.txt index 0e9918de0..8560cb9fa 100644 --- a/src/registrar/templates/emails/domain_manager_notification_subject.txt +++ b/src/registrar/templates/emails/domain_manager_notification_subject.txt @@ -1 +1 @@ -A domain manager was invited to {{ domain.name }} \ No newline at end of file +{{ prefix }}A domain manager was invited to {{ domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_request_withdrawn.txt b/src/registrar/templates/emails/domain_request_withdrawn.txt index fbdf5b4f1..fe026027b 100644 --- a/src/registrar/templates/emails/domain_request_withdrawn.txt +++ b/src/registrar/templates/emails/domain_request_withdrawn.txt @@ -11,7 +11,7 @@ STATUS: Withdrawn ---------------------------------------------------------------- YOU CAN EDIT YOUR WITHDRAWN REQUEST -You can edit and resubmit this request by signing in to the registrar . +You can edit and resubmit this request by signing in to the registrar <{{ manage_url }}>. SOMETHING WRONG? diff --git a/src/registrar/templates/emails/domain_request_withdrawn_subject.txt b/src/registrar/templates/emails/domain_request_withdrawn_subject.txt index 51b2c745a..cc146643a 100644 --- a/src/registrar/templates/emails/domain_request_withdrawn_subject.txt +++ b/src/registrar/templates/emails/domain_request_withdrawn_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/metadata_body.txt b/src/registrar/templates/emails/metadata_body.txt index adf0a186c..a0a3682b7 100644 --- a/src/registrar/templates/emails/metadata_body.txt +++ b/src/registrar/templates/emails/metadata_body.txt @@ -1 +1 @@ -An export of all .gov metadata. +{{ prefix }}An export of all .gov metadata. diff --git a/src/registrar/templates/emails/metadata_subject.txt b/src/registrar/templates/emails/metadata_subject.txt index 5fdece7ef..c19b4c26e 100644 --- a/src/registrar/templates/emails/metadata_subject.txt +++ b/src/registrar/templates/emails/metadata_subject.txt @@ -1,2 +1,2 @@ -Domain metadata - {{current_date_str}} +{{ prefix }}Domain metadata - {{current_date_str}} diff --git a/src/registrar/templates/emails/portfolio_admin_addition_notification.txt b/src/registrar/templates/emails/portfolio_admin_addition_notification.txt index b8953aa67..9e6da3985 100644 --- a/src/registrar/templates/emails/portfolio_admin_addition_notification.txt +++ b/src/registrar/templates/emails/portfolio_admin_addition_notification.txt @@ -16,7 +16,7 @@ The person who received the invitation will become an admin once they log in to associated with the invited email address. If you need to cancel this invitation or remove the admin, you can do that by going to -the Members section for your organization . +the Members section for your organization <{{ manage_url }}>. WHY DID YOU RECEIVE THIS EMAIL? diff --git a/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt b/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt index 3d6b2a140..ee5987512 100644 --- a/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt +++ b/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt @@ -1 +1 @@ -An admin was invited to your .gov organization \ No newline at end of file +{{ prefix }}An admin was invited to your .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/portfolio_admin_removal_notification.txt b/src/registrar/templates/emails/portfolio_admin_removal_notification.txt index 6a536aa49..bf0338c03 100644 --- a/src/registrar/templates/emails/portfolio_admin_removal_notification.txt +++ b/src/registrar/templates/emails/portfolio_admin_removal_notification.txt @@ -8,7 +8,7 @@ REMOVED BY: {{ requestor_email }} REMOVED ON: {{date}} ADMIN REMOVED: {{ removed_email_address }} -You can view this update by going to the Members section for your .gov organization . +You can view this update by going to the Members section for your .gov organization <{{ manage_url }}>. ---------------------------------------------------------------- diff --git a/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt index e250b17f8..030d27ae7 100644 --- a/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt +++ b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt @@ -1 +1 @@ -An admin was removed from your .gov organization \ No newline at end of file +{{ prefix}}An admin was removed from your .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/portfolio_invitation.txt b/src/registrar/templates/emails/portfolio_invitation.txt index 775b74c7c..893da153d 100644 --- a/src/registrar/templates/emails/portfolio_invitation.txt +++ b/src/registrar/templates/emails/portfolio_invitation.txt @@ -3,7 +3,7 @@ Hi. {{ requestor_email }} has invited you to {{ portfolio.organization_name }}. -You can view this organization on the .gov registrar . +You can view this organization on the .gov registrar <{{ manage_url }}>. ---------------------------------------------------------------- diff --git a/src/registrar/templates/emails/portfolio_invitation_subject.txt b/src/registrar/templates/emails/portfolio_invitation_subject.txt index 552bb2bec..de9080196 100644 --- a/src/registrar/templates/emails/portfolio_invitation_subject.txt +++ b/src/registrar/templates/emails/portfolio_invitation_subject.txt @@ -1 +1 @@ -You’ve been invited to a .gov organization \ No newline at end of file +{{ prefix }}You’ve been invited to a .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/status_change_approved.txt b/src/registrar/templates/emails/status_change_approved.txt index 821e89e42..635b36cbd 100644 --- a/src/registrar/templates/emails/status_change_approved.txt +++ b/src/registrar/templates/emails/status_change_approved.txt @@ -8,7 +8,7 @@ REQUESTED BY: {{ domain_request.creator.email }} REQUEST RECEIVED ON: {{ domain_request.last_submitted_date|date }} STATUS: Approved -You can manage your approved domain on the .gov registrar . +You can manage your approved domain on the .gov registrar <{{ manage_url }}>. ---------------------------------------------------------------- diff --git a/src/registrar/templates/emails/status_change_approved_subject.txt b/src/registrar/templates/emails/status_change_approved_subject.txt index 51b2c745a..cc146643a 100644 --- a/src/registrar/templates/emails/status_change_approved_subject.txt +++ b/src/registrar/templates/emails/status_change_approved_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/status_change_subject.txt b/src/registrar/templates/emails/status_change_subject.txt index 51b2c745a..cc146643a 100644 --- a/src/registrar/templates/emails/status_change_subject.txt +++ b/src/registrar/templates/emails/status_change_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/submission_confirmation.txt b/src/registrar/templates/emails/submission_confirmation.txt index d9d01ec3e..afbde48d5 100644 --- a/src/registrar/templates/emails/submission_confirmation.txt +++ b/src/registrar/templates/emails/submission_confirmation.txt @@ -20,7 +20,7 @@ During our review, we’ll verify that: - You work at the organization and/or can make requests on its behalf - Your requested domain meets our naming requirements {% endif %} -We’ll email you if we have questions. We’ll also email you as soon as we complete our review. You can check the status of your request at any time on the registrar. . +We’ll email you if we have questions. We’ll also email you as soon as we complete our review. You can check the status of your request at any time on the registrar. <{{ manage_url }}>. NEED TO MAKE CHANGES? diff --git a/src/registrar/templates/emails/submission_confirmation_subject.txt b/src/registrar/templates/emails/submission_confirmation_subject.txt index 51b2c745a..cc146643a 100644 --- a/src/registrar/templates/emails/submission_confirmation_subject.txt +++ b/src/registrar/templates/emails/submission_confirmation_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/transition_domain_invitation.txt b/src/registrar/templates/emails/transition_domain_invitation.txt index b6773d9e9..dc812edf3 100644 --- a/src/registrar/templates/emails/transition_domain_invitation.txt +++ b/src/registrar/templates/emails/transition_domain_invitation.txt @@ -31,7 +31,7 @@ CHECK YOUR .GOV DOMAIN CONTACTS This is a good time to check who has access to your .gov domain{% if domains|length > 1 %}s{% endif %}. The admin, technical, and billing contacts listed for your domain{% if domains|length > 1 %}s{% endif %} in our old system also received this email. In our new registrar, these contacts are all considered “domain managers.” We no longer have the admin, technical, and billing roles, and you aren’t limited to three domain managers like in the old system. - 1. Once you have your Login.gov account, sign in to the new registrar at . + 1. Once you have your Login.gov account, sign in to the new registrar at <{{ manage_url }}>. 2. Click the “Manage” link next to your .gov domain, then click on “Domain managers” to see who has access to your domain. 3. If any of these users should not have access to your domain, let us know in a reply to this email. diff --git a/src/registrar/templates/emails/transition_domain_invitation_subject.txt b/src/registrar/templates/emails/transition_domain_invitation_subject.txt index 526c7714b..b162341d9 100644 --- a/src/registrar/templates/emails/transition_domain_invitation_subject.txt +++ b/src/registrar/templates/emails/transition_domain_invitation_subject.txt @@ -1 +1 @@ -(Action required) Manage your .gov domain{% if domains|length > 1 %}s{% endif %} in the new registrar \ No newline at end of file +{{ prefix }}(Action required) Manage your .gov domain{% if domains|length > 1 %}s{% endif %} in the new registrar \ No newline at end of file diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index 99f86ea54..fb0a442cb 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -1,4 +1,4 @@ -{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} + {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} Hi, An update was made to a domain you manage. @@ -8,7 +8,7 @@ UPDATED BY: {{user}} UPDATED ON: {{date}} INFORMATION UPDATED: {{changes}} -You can view this update in the .gov registrar . +You can view this update in the .gov registrar <{{ manage_url }}>. Get help with managing your .gov domain . diff --git a/src/registrar/templates/emails/update_to_approved_domain_subject.txt b/src/registrar/templates/emails/update_to_approved_domain_subject.txt index cf4c9a14c..d952999a0 100644 --- a/src/registrar/templates/emails/update_to_approved_domain_subject.txt +++ b/src/registrar/templates/emails/update_to_approved_domain_subject.txt @@ -1 +1 @@ -An update was made to {{domain}} \ No newline at end of file +{{ prefix }}An update was made to {{domain}} \ No newline at end of file diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 40601cdc7..535096b10 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,24 @@ def send_templated_email( # noqa No valid recipient addresses are provided """ + if context is None: + context = {} + + env_base_url = settings.BASE_URL + # The regular expresstion is to get both http (localhost) and https (everything else) + env_name = re.sub(r"^https?://", "", env_base_url).split(".")[0] + # To add to subject lines ie [GETGOV-RH] + prefix = f"[{env_name.upper()}] " if not settings.IS_PRODUCTION else "" + # For email links + manage_url = env_base_url if not settings.IS_PRODUCTION else "https://manage.get.gov" + + # Adding to context + context.update( + { + "prefix": prefix, + "manage_url": manage_url, + } + ) # by default assume we can send to all addresses (prod has no whitelist) sendable_cc_addresses = cc_addresses From c540a324c7df33c7011fcef4bebe158a43bf417a Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 12 Feb 2025 10:11:25 -0800 Subject: [PATCH 07/23] Add unit tests --- src/registrar/tests/test_emails.py | 76 ++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index f39f11517..c79038668 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 ([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): From 0725ab8e59e59ac63267fe1d008998d7fd7ba26c Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 12 Feb 2025 10:31:53 -0800 Subject: [PATCH 08/23] Clean up the comments --- src/registrar/tests/test_emails.py | 2 +- src/registrar/utility/email.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_emails.py b/src/registrar/tests/test_emails.py index c79038668..2b7f89ac9 100644 --- a/src/registrar/tests/test_emails.py +++ b/src/registrar/tests/test_emails.py @@ -151,7 +151,7 @@ class TestEmails(TestCase): @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 ([GETGOV-RH]) + 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( diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 535096b10..9323255af 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -57,7 +57,7 @@ def send_templated_email( # noqa env_name = re.sub(r"^https?://", "", env_base_url).split(".")[0] # To add to subject lines ie [GETGOV-RH] prefix = f"[{env_name.upper()}] " if not settings.IS_PRODUCTION else "" - # For email links + # For email links ie getgov-rh.app.cloud.gov manage_url = env_base_url if not settings.IS_PRODUCTION else "https://manage.get.gov" # Adding to context From 00732d0a64499224cc1e7da96ec35eba54970d51 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 12 Feb 2025 10:58:38 -0800 Subject: [PATCH 09/23] Fix carrot link --- src/registrar/templates/emails/domain_manager_notification.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/domain_manager_notification.txt b/src/registrar/templates/emails/domain_manager_notification.txt index 18e682329..b5096a9d8 100644 --- a/src/registrar/templates/emails/domain_manager_notification.txt +++ b/src/registrar/templates/emails/domain_manager_notification.txt @@ -15,7 +15,7 @@ The person who received the invitation will become a domain manager once they lo associated with the invited email address. If you need to cancel this invitation or remove the domain manager, you can do that by going to -this domain in the .gov registrar <{{ manage_url }}. +this domain in the .gov registrar <{{ manage_url }}>. WHY DID YOU RECEIVE THIS EMAIL? From 16f0ae6f627417f9162c29bfa7b832396e4c5951 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 12 Feb 2025 11:00:13 -0800 Subject: [PATCH 10/23] Fix more spacing --- .../emails/portfolio_admin_removal_notification_subject.txt | 2 +- src/registrar/templates/emails/update_to_approved_domain.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt index 030d27ae7..9a45a8bbc 100644 --- a/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt +++ b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt @@ -1 +1 @@ -{{ prefix}}An admin was removed from your .gov organization \ No newline at end of file +{{ prefix }}An admin was removed from your .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/update_to_approved_domain.txt b/src/registrar/templates/emails/update_to_approved_domain.txt index fb0a442cb..070096f62 100644 --- a/src/registrar/templates/emails/update_to_approved_domain.txt +++ b/src/registrar/templates/emails/update_to_approved_domain.txt @@ -1,4 +1,4 @@ - {% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} +{% autoescape off %}{# In a text file, we don't want to have HTML entities escaped #} Hi, An update was made to a domain you manage. From 3245cded0e81597974496799eace8a3ee6835969 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 13 Feb 2025 10:58:40 -0500 Subject: [PATCH 11/23] updated naming for workflow --- .github/workflows/delete-and-recreate-db.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index ecdf54bbc..e7cad783d 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: From 71a9e9515d99d5e4481c1993379bbeb74c5b178f Mon Sep 17 00:00:00 2001 From: asaki222 Date: Thu, 13 Feb 2025 11:09:32 -0500 Subject: [PATCH 12/23] updated the creds variable --- .github/workflows/delete-and-recreate-db.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/delete-and-recreate-db.yaml b/.github/workflows/delete-and-recreate-db.yaml index e7cad783d..979f20826 100644 --- a/.github/workflows/delete-and-recreate-db.yaml +++ b/.github/workflows/delete-and-recreate-db.yaml @@ -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 From c737daa8fd2f8d497edfb9c5499d74e66316dcd7 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 13 Feb 2025 10:28:49 -0800 Subject: [PATCH 13/23] Add prefix and manage url to the action needed reasons templates --- .../action_needed_reasons/already_has_domains_subject.txt | 2 +- .../templates/emails/action_needed_reasons/bad_name.txt | 2 +- .../templates/emails/action_needed_reasons/bad_name_subject.txt | 2 +- .../action_needed_reasons/eligibility_unclear_subject.txt | 2 +- .../action_needed_reasons/questionable_senior_official.txt | 2 +- .../questionable_senior_official_subject.txt | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt b/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt index 7ca332ddd..b29b8040c 100644 --- a/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/action_needed_reasons/bad_name.txt b/src/registrar/templates/emails/action_needed_reasons/bad_name.txt index ac563b549..40e5ed899 100644 --- a/src/registrar/templates/emails/action_needed_reasons/bad_name.txt +++ b/src/registrar/templates/emails/action_needed_reasons/bad_name.txt @@ -17,7 +17,7 @@ Domains should uniquely identify a government organization and be clear to the g ACTION NEEDED -First, we need you to identify a new domain name that meets our naming requirements for your type of organization. Then, log in to the registrar and update the name in your domain request. Once you submit your updated request, we’ll resume the adjudication process. +First, we need you to identify a new domain name that meets our naming requirements for your type of organization. Then, log in to the registrar and update the name in your domain request. <{{ manage_url }}> Once you submit your updated request, we’ll resume the adjudication process. If you have questions or want to discuss potential domain names, reply to this email. diff --git a/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt b/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt index 7ca332ddd..b29b8040c 100644 --- a/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt b/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt index 7ca332ddd..b29b8040c 100644 --- a/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt index ef05e17d7..40d068cd9 100644 --- a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt +++ b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official.txt @@ -21,7 +21,7 @@ We expect a senior official to be someone in a role of significant, executive re ACTION NEEDED Reply to this email with a justification for naming {{ domain_request.senior_official.get_formatted_name }} as the senior official. If you have questions or comments, include those in your reply. -Alternatively, you can log in to the registrar and enter a different senior official for this domain request. Once you submit your updated request, we’ll resume the adjudication process. +Alternatively, you can log in to the registrar and enter a different senior official for this domain request. <{{ manage_url }}> Once you submit your updated request, we’ll resume the adjudication process. THANK YOU diff --git a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt index 7ca332ddd..b29b8040c 100644 --- a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt @@ -1 +1 @@ -Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file From 856b71b6ce70e429a77a1342e20ac41db8a75b42 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Thu, 13 Feb 2025 10:30:48 -0800 Subject: [PATCH 14/23] Add for transition domain inv email --- src/registrar/templates/emails/transition_domain_invitation.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/emails/transition_domain_invitation.txt b/src/registrar/templates/emails/transition_domain_invitation.txt index dc812edf3..14dd626dd 100644 --- a/src/registrar/templates/emails/transition_domain_invitation.txt +++ b/src/registrar/templates/emails/transition_domain_invitation.txt @@ -57,7 +57,7 @@ THANK YOU The .gov team .Gov blog -Domain management +Domain management <{{ manage_url }}}> Get.gov The .gov registry is a part of the Cybersecurity and Infrastructure Security Agency (CISA) From 986bcc235125f627c6458a4b36b5d40ad227e106 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 14 Feb 2025 06:56:56 -0500 Subject: [PATCH 15/23] fixed date in email --- src/registrar/utility/email_invitations.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/utility/email_invitations.py b/src/registrar/utility/email_invitations.py index d206bf279..7ddab65f1 100644 --- a/src/registrar/utility/email_invitations.py +++ b/src/registrar/utility/email_invitations.py @@ -255,6 +255,7 @@ def send_portfolio_member_permission_update_email(requestor, permissions: UserPo "portfolio": permissions.portfolio, "requestor_email": requestor_email, "permissions": permissions, + "date": date.today(), }, ) except EmailSendingError: From 8020bd6f7461b71fa663fc5fff8c4a797eeed907 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 14 Feb 2025 08:02:47 -0800 Subject: [PATCH 16/23] Update for prefix to be in the subject dynamically --- .../already_has_domains_subject.txt | 2 +- .../emails/action_needed_reasons/bad_name_subject.txt | 2 +- .../eligibility_unclear_subject.txt | 2 +- .../questionable_senior_official_subject.txt | 2 +- .../templates/emails/domain_invitation_subject.txt | 2 +- .../domain_manager_deleted_notification_subject.txt | 2 +- .../emails/domain_manager_notification_subject.txt | 2 +- .../emails/domain_request_withdrawn_subject.txt | 2 +- src/registrar/templates/emails/metadata_body.txt | 2 +- src/registrar/templates/emails/metadata_subject.txt | 2 +- .../portfolio_admin_addition_notification_subject.txt | 2 +- .../portfolio_admin_removal_notification_subject.txt | 2 +- .../templates/emails/portfolio_invitation_subject.txt | 2 +- .../emails/status_change_approved_subject.txt | 2 +- .../templates/emails/status_change_subject.txt | 2 +- .../emails/submission_confirmation_subject.txt | 2 +- .../emails/transition_domain_invitation_subject.txt | 2 +- .../emails/update_to_approved_domain_subject.txt | 2 +- src/registrar/utility/email.py | 11 +++++++---- 19 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt b/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt index b29b8040c..7ca332ddd 100644 --- a/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/already_has_domains_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt b/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt index b29b8040c..7ca332ddd 100644 --- a/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/bad_name_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt b/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt index b29b8040c..7ca332ddd 100644 --- a/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/eligibility_unclear_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt index b29b8040c..7ca332ddd 100644 --- a/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt +++ b/src/registrar/templates/emails/action_needed_reasons/questionable_senior_official_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file +Update on your .gov request: {{ domain_request.requested_domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_invitation_subject.txt b/src/registrar/templates/emails/domain_invitation_subject.txt index 9f15c38b4..9663346d0 100644 --- a/src/registrar/templates/emails/domain_invitation_subject.txt +++ b/src/registrar/templates/emails/domain_invitation_subject.txt @@ -1 +1 @@ -{{ prefix }}You've been invited to manage {% if domains|length > 1 %}.gov domains{% else %}{{ domains.0.name }}{% endif %} \ No newline at end of file +You've been invited to manage {% if domains|length > 1 %}.gov domains{% else %}{{ domains.0.name }}{% endif %} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt b/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt index 7376bdb86..c84a20f18 100644 --- a/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt +++ b/src/registrar/templates/emails/domain_manager_deleted_notification_subject.txt @@ -1 +1 @@ -{{ prefix }}A domain manager was removed from {{ domain.name }} \ No newline at end of file +A domain manager was removed from {{ domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_manager_notification_subject.txt b/src/registrar/templates/emails/domain_manager_notification_subject.txt index 8560cb9fa..0e9918de0 100644 --- a/src/registrar/templates/emails/domain_manager_notification_subject.txt +++ b/src/registrar/templates/emails/domain_manager_notification_subject.txt @@ -1 +1 @@ -{{ prefix }}A domain manager was invited to {{ domain.name }} \ No newline at end of file +A domain manager was invited to {{ domain.name }} \ No newline at end of file diff --git a/src/registrar/templates/emails/domain_request_withdrawn_subject.txt b/src/registrar/templates/emails/domain_request_withdrawn_subject.txt index cc146643a..51b2c745a 100644 --- a/src/registrar/templates/emails/domain_request_withdrawn_subject.txt +++ b/src/registrar/templates/emails/domain_request_withdrawn_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} +Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/metadata_body.txt b/src/registrar/templates/emails/metadata_body.txt index a0a3682b7..adf0a186c 100644 --- a/src/registrar/templates/emails/metadata_body.txt +++ b/src/registrar/templates/emails/metadata_body.txt @@ -1 +1 @@ -{{ prefix }}An export of all .gov metadata. +An export of all .gov metadata. diff --git a/src/registrar/templates/emails/metadata_subject.txt b/src/registrar/templates/emails/metadata_subject.txt index c19b4c26e..5fdece7ef 100644 --- a/src/registrar/templates/emails/metadata_subject.txt +++ b/src/registrar/templates/emails/metadata_subject.txt @@ -1,2 +1,2 @@ -{{ prefix }}Domain metadata - {{current_date_str}} +Domain metadata - {{current_date_str}} diff --git a/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt b/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt index ee5987512..3d6b2a140 100644 --- a/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt +++ b/src/registrar/templates/emails/portfolio_admin_addition_notification_subject.txt @@ -1 +1 @@ -{{ prefix }}An admin was invited to your .gov organization \ No newline at end of file +An admin was invited to your .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt index 9a45a8bbc..e250b17f8 100644 --- a/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt +++ b/src/registrar/templates/emails/portfolio_admin_removal_notification_subject.txt @@ -1 +1 @@ -{{ prefix }}An admin was removed from your .gov organization \ No newline at end of file +An admin was removed from your .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/portfolio_invitation_subject.txt b/src/registrar/templates/emails/portfolio_invitation_subject.txt index de9080196..552bb2bec 100644 --- a/src/registrar/templates/emails/portfolio_invitation_subject.txt +++ b/src/registrar/templates/emails/portfolio_invitation_subject.txt @@ -1 +1 @@ -{{ prefix }}You’ve been invited to a .gov organization \ No newline at end of file +You’ve been invited to a .gov organization \ No newline at end of file diff --git a/src/registrar/templates/emails/status_change_approved_subject.txt b/src/registrar/templates/emails/status_change_approved_subject.txt index cc146643a..51b2c745a 100644 --- a/src/registrar/templates/emails/status_change_approved_subject.txt +++ b/src/registrar/templates/emails/status_change_approved_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} +Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/status_change_subject.txt b/src/registrar/templates/emails/status_change_subject.txt index cc146643a..51b2c745a 100644 --- a/src/registrar/templates/emails/status_change_subject.txt +++ b/src/registrar/templates/emails/status_change_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} +Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/submission_confirmation_subject.txt b/src/registrar/templates/emails/submission_confirmation_subject.txt index cc146643a..51b2c745a 100644 --- a/src/registrar/templates/emails/submission_confirmation_subject.txt +++ b/src/registrar/templates/emails/submission_confirmation_subject.txt @@ -1 +1 @@ -{{ prefix }}Update on your .gov request: {{ domain_request.requested_domain.name }} +Update on your .gov request: {{ domain_request.requested_domain.name }} diff --git a/src/registrar/templates/emails/transition_domain_invitation_subject.txt b/src/registrar/templates/emails/transition_domain_invitation_subject.txt index b162341d9..526c7714b 100644 --- a/src/registrar/templates/emails/transition_domain_invitation_subject.txt +++ b/src/registrar/templates/emails/transition_domain_invitation_subject.txt @@ -1 +1 @@ -{{ prefix }}(Action required) Manage your .gov domain{% if domains|length > 1 %}s{% endif %} in the new registrar \ No newline at end of file +(Action required) Manage your .gov domain{% if domains|length > 1 %}s{% endif %} in the new registrar \ No newline at end of file diff --git a/src/registrar/templates/emails/update_to_approved_domain_subject.txt b/src/registrar/templates/emails/update_to_approved_domain_subject.txt index d952999a0..cf4c9a14c 100644 --- a/src/registrar/templates/emails/update_to_approved_domain_subject.txt +++ b/src/registrar/templates/emails/update_to_approved_domain_subject.txt @@ -1 +1 @@ -{{ prefix }}An update was made to {{domain}} \ No newline at end of file +An update was made to {{domain}} \ No newline at end of file diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 9323255af..b4caf42a5 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -60,13 +60,19 @@ def send_templated_email( # noqa # For email links ie getgov-rh.app.cloud.gov manage_url = env_base_url if not settings.IS_PRODUCTION else "https://manage.get.gov" + # 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}" + # Adding to context context.update( { - "prefix": prefix, + "subject": subject, "manage_url": manage_url, } ) + # by default assume we can send to all addresses (prod has no whitelist) sendable_cc_addresses = cc_addresses @@ -89,9 +95,6 @@ def send_templated_email( # noqa if email_body: email_body.strip().lstrip("\n") - subject_template = get_template(subject_template_name) - subject = subject_template.render(context=context) - try: ses_client = boto3.client( "sesv2", From 33bf1988f1700ca149403555e3125697199c9128 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 14 Feb 2025 08:28:50 -0800 Subject: [PATCH 17/23] Fix placement of where email subject is edited --- src/registrar/utility/email.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index b4caf42a5..39c7f21ac 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -60,18 +60,7 @@ def send_templated_email( # noqa # For email links ie getgov-rh.app.cloud.gov manage_url = env_base_url if not settings.IS_PRODUCTION else "https://manage.get.gov" - # 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}" - - # Adding to context - context.update( - { - "subject": subject, - "manage_url": manage_url, - } - ) + context["manage_url"] = manage_url # by default assume we can send to all addresses (prod has no whitelist) sendable_cc_addresses = cc_addresses @@ -95,6 +84,13 @@ 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}" + + context["subject"] = subject + try: ses_client = boto3.client( "sesv2", From c81c229be23e3883e445ecfaba3f7ff01b2ba347 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 14 Feb 2025 12:01:25 -0500 Subject: [PATCH 18/23] fixed test --- src/registrar/tests/test_email_invitations.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/tests/test_email_invitations.py b/src/registrar/tests/test_email_invitations.py index 20ac4a565..981bca6dd 100644 --- a/src/registrar/tests/test_email_invitations.py +++ b/src/registrar/tests/test_email_invitations.py @@ -919,6 +919,7 @@ class TestSendPortfolioMemberPermissionUpdateEmail(unittest.TestCase): "portfolio": permissions.portfolio, "requestor_email": "requestor@example.com", "permissions": permissions, + "date": date.today(), }, ) self.assertTrue(result) From 7d8249923e88a0fa4571d6290e5409d792bb42eb Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 14 Feb 2025 09:17:28 -0800 Subject: [PATCH 19/23] Update comments --- src/registrar/utility/email.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 39c7f21ac..797ad4aa9 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -53,11 +53,13 @@ def send_templated_email( # noqa context = {} env_base_url = settings.BASE_URL - # The regular expresstion is to get both http (localhost) and https (everything else) + # The regular expression is to get both http (localhost) and https (everything else) env_name = re.sub(r"^https?://", "", env_base_url).split(".")[0] - # To add to subject lines ie [GETGOV-RH] + # 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 "" - # For email links ie getgov-rh.app.cloud.gov + # 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 From 026326b6eca501f9d00782c02ac7a947ac9338f6 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 14 Feb 2025 11:15:14 -0800 Subject: [PATCH 20/23] Remove unneeded additional context udpate --- src/registrar/utility/email.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/utility/email.py b/src/registrar/utility/email.py index 797ad4aa9..94e87a96b 100644 --- a/src/registrar/utility/email.py +++ b/src/registrar/utility/email.py @@ -91,8 +91,6 @@ def send_templated_email( # noqa subject = subject_template.render(context=context) subject = f"{prefix}{subject}" - context["subject"] = subject - try: ses_client = boto3.client( "sesv2", From 62626636aeba4a03ff7520ffd4b66143e7090a06 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Sat, 15 Feb 2025 08:16:17 -0500 Subject: [PATCH 21/23] Viewer, all => Viewer --- src/registrar/assets/src/js/getgov/portfolio-member-page.js | 2 +- src/registrar/forms/portfolio.py | 2 +- src/registrar/models/portfolio_invitation.py | 2 +- src/registrar/models/user_portfolio_permission.py | 2 +- src/registrar/models/utility/portfolio_helper.py | 4 ++-- src/registrar/tests/test_views_portfolio.py | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) 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/forms/portfolio.py b/src/registrar/forms/portfolio.py index 3a9074b2d..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, diff --git a/src/registrar/models/portfolio_invitation.py b/src/registrar/models/portfolio_invitation.py index 045cf2de4..99febc92e 100644 --- a/src/registrar/models/portfolio_invitation.py +++ b/src/registrar/models/portfolio_invitation.py @@ -108,7 +108,7 @@ class PortfolioInvitation(TimeStampedModel): Returns a string representation of the user's domain access level. Uses the `get_domains_display` function to determine whether the user has - "Viewer, all" access (can view all domains) or "Viewer, limited" access. + "Viewer" access (can view all domains) or "Viewer, limited" access. Returns: str: The display name of the user's domain permissions. diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 202fc2e8d..e077daa57 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -204,7 +204,7 @@ class UserPortfolioPermission(TimeStampedModel): Returns a string representation of the user's domain access level. Uses the `get_domains_display` function to determine whether the user has - "Viewer, all" access (can view all domains) or "Viewer, limited" access. + "Viewer" access (can view all domains) or "Viewer, limited" access. Returns: str: The display name of the user's domain permissions. diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 00c5ce2b8..03733237e 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -105,7 +105,7 @@ 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, all". + - If the user has the VIEW_ALL_DOMAINS permission, return "Viewer". - Otherwise, return "Viewer, limited". Args: @@ -118,7 +118,7 @@ def get_domains_display(roles, permissions): UserPortfolioPermission = apps.get_model("registrar.UserPortfolioPermission") all_permissions = UserPortfolioPermission.get_portfolio_permissions(roles, permissions) if UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS in all_permissions: - return "Viewer, all" + return "Viewer" else: return "Viewer, limited" diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 65e0350ee..3ce1cfdfa 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( From 1b7871c4063233d8a8b8e6f5f13164edf5393eb4 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 16 Feb 2025 18:56:20 -0700 Subject: [PATCH 22/23] stripped dead code --- src/registrar/models/user.py | 45 +-------------- src/registrar/templatetags/custom_filters.py | 7 --- src/registrar/tests/test_models.py | 61 -------------------- 3 files changed, 1 insertion(+), 112 deletions(-) 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/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index d21678d58..c0204b4ca 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -251,13 +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") 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 From 70f5a655a8ae9a7b27aa778ecf68d9c1f401d815 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 16 Feb 2025 19:00:23 -0700 Subject: [PATCH 23/23] linted --- src/registrar/templatetags/custom_filters.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index c0204b4ca..ff73e6dc1 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -251,8 +251,6 @@ def is_members_subpage(path): return get_url_name(path) in url_names - - @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