From f10cfe2c9bd143029af27270807619eab41be630 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 4 Feb 2025 14:45:08 -0600 Subject: [PATCH 001/226] delete ds data --- src/registrar/models/domain.py | 15 +++++++++- src/registrar/tests/test_models_domain.py | 36 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 0f0b3f112..134da7ead 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1053,13 +1053,15 @@ class Domain(TimeStampedModel, DomainHelper): note=f"Host {host.name} is in use by {host.domain}", ) + # set hosts to empty list so nameservers are deleted ( deleted_values, updated_values, new_values, oldNameservers, ) = self.getNameserverChanges(hosts=[]) - + + # update the hosts _ = self._update_host_values(updated_values, oldNameservers) # returns nothing, just need to be run and errors addToDomainList, _ = self.createNewHostList(new_values) deleteHostList, _ = self.createDeleteHostList(deleted_values) @@ -1073,6 +1075,7 @@ class Domain(TimeStampedModel, DomainHelper): # but we still need to delete the object themselves self._delete_hosts_if_not_used(hostsToDelete=deleted_values) + # delete the non-registrant contacts logger.debug("Deleting non-registrant contacts for %s", self.name) contacts = PublicContact.objects.filter(domain=self) for contact in contacts: @@ -1081,6 +1084,16 @@ class Domain(TimeStampedModel, DomainHelper): request = commands.DeleteContact(contact.registry_id) registry.send(request, cleaned=True) + # delete ds data if it exists + if self.dnssecdata: + logger.debug("Deleting ds data for %s", self.name) + try: + self.dnssecdata = None + except RegistryError as e: + logger.error("Error deleting ds data for %s: %s", self.name, e) + e.note = "Error deleting ds data for %s" % self.name + raise e + logger.info("Deleting domain %s", self.name) request = commands.DeleteDomain(name=self.name) registry.send(request, cleaned=True) diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 083725a55..5bb783fd0 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2863,6 +2863,42 @@ class TestAnalystDelete(MockEppLib): # State should have changed self.assertEqual(self.domain_with_contacts.state, Domain.State.DELETED) + def test_analyst_deletes_domain_with_ds_data(self): + """ + Scenario: Domain with DS data is deleted + When `domain.deletedInEpp()` is called + Then `commands.DeleteDomain` is sent to the registry + And `state` is set to `DELETED` + """ + # Create a domain with DS data + domain, _ = Domain.objects.get_or_create(name="dsdomain.gov", state=Domain.State.READY) + domain.dnssecdata = extensions.DNSSECExtension( + dsdata=[extensions.DSData(keytag=1, algorithm=1, digest_type=1, digest="1234567890")], + keydata=[extensions.DNSSECKeyData(keytag=1, algorithm=1, digest_type=1, digest="1234567890")], + ) + domain.save() + + # Delete the domain + domain.deletedInEpp() + domain.save() + + # Check that dsdata is None + self.assertEqual(domain.dnssecdata, None) + + # Check that the UpdateDomain command was sent to the registry with the correct extension + self.mockedSendFunction.assert_has_calls( + [ + call( + commands.UpdateDomain(name="dsdomain.gov", add=[], rem=[], nsset=None, keyset=None, registrant=None, auth_info=None), + cleaned=True, + ), + ] + ) + + # Check that the domain was deleted + self.assertEqual(domain.state, Domain.State.DELETED) + + @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): """ From 59c22643316dd87b7ed7910eb62de07962f8e6fe Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 4 Feb 2025 14:55:19 -0600 Subject: [PATCH 002/226] remove raise error on delete_hosts_if_not_used --- src/registrar/models/domain.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 134da7ead..44eb1cde0 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -750,11 +750,7 @@ class Domain(TimeStampedModel, DomainHelper): successTotalNameservers = len(oldNameservers) - deleteCount + addToDomainCount - try: - self._delete_hosts_if_not_used(hostsToDelete=deleted_values) - except Exception as e: - # we don't need this part to succeed in order to continue. - logger.error("Failed to delete nameserver hosts: %s", e) + self._delete_hosts_if_not_used(hostsToDelete=deleted_values) if successTotalNameservers < 2: try: @@ -1849,8 +1845,6 @@ class Domain(TimeStampedModel, DomainHelper): else: logger.error("Error _delete_hosts_if_not_used, code was %s error was %s" % (e.code, e)) - raise e - def _fix_unknown_state(self, cleaned): """ _fix_unknown_state: Calls _add_missing_contacts_if_unknown From 77ddc6700aef46fd224c07cf6bbf260c332f8d81 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 4 Feb 2025 14:34:00 -0800 Subject: [PATCH 003/226] Adding all the status filter and custom filter set up for dropdown --- src/registrar/admin.py | 87 ++++++++++++++++++++--- src/registrar/models/domain_invitation.py | 4 ++ 2 files changed, 80 insertions(+), 11 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 31c75e05e..9d4fe80f3 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1478,9 +1478,31 @@ class BaseInvitationAdmin(ListHeaderAdmin): return response +# class DomainInvitationAdminForm(forms.ModelForm): +# """Custom form for DomainInvitation in admin to only allow cancellations.""" + +# STATUS_CHOICES = [ +# ("", "------"), # no action +# ("canceled", "Canceled"), +# ] + +# status = forms.ChoiceField(choices=STATUS_CHOICES, required=False, label="Status") + +# class Meta: +# model = models.DomainInvitation +# fields = "__all__" + +# def clean_status(self): +# # Clean status - we purposely dont edit anything so we dont mess with the state +# status = self.cleaned_data.get("status") +# return status + + class DomainInvitationAdmin(BaseInvitationAdmin): """Custom domain invitation admin class.""" + # form = DomainInvitationAdminForm + class Meta: model = models.DomainInvitation fields = "__all__" @@ -1505,23 +1527,49 @@ class DomainInvitationAdmin(BaseInvitationAdmin): search_help_text = "Search by email or domain." - # Mark the FSM field 'status' as readonly - # to allow admin users to create Domain Invitations - # without triggering the FSM Transition Not Allowed - # error. + # # Mark the FSM field 'status' as readonly + # # to allow admin users to create Domain Invitations + # # without triggering the FSM Transition Not Allowed + # # error. + # readonly_fields = ["status"] + + # Now it can be edited readonly_fields = ["status"] autocomplete_fields = ["domain"] change_form_template = "django/admin/domain_invitation_change_form.html" - # Select domain invitations to change -> Domain invitations - def changelist_view(self, request, extra_context=None): - if extra_context is None: - extra_context = {} - extra_context["tabtitle"] = "Domain invitations" - # Get the filtered values - return super().changelist_view(request, extra_context=extra_context) + # # Custom status filter within DomainInvitationAdmin + # class StatusListFilter(admin.SimpleListFilter): + # # custom filter for status field + + # title = _("status") + # parameter_name = "status" + + # def lookups(self, request, model_admin): + # # only return cancel as option + # return [ + # ('canceled', _('Canceled')), + # ('invited', _('Invited')), + # ('retrieved', _('Retrieved')), + # ] + + # def queryset(self, request, queryset): + # """Filter the queryset based on the selected status.""" + # if self.value(): + # return queryset.filter(status=self.value()) # Apply the filter based on the selected status + # return queryset + + # list_filter = (StatusListFilter,) # Apply the custom filter to the list view + + # # Select domain invitations to change -> Domain invitations + # def changelist_view(self, request, extra_context=None): + # if extra_context is None: + # extra_context = {} + # extra_context["tabtitle"] = "Domain invitations" + # # Get the filtered values + # return super().changelist_view(request, extra_context=extra_context) def save_model(self, request, obj, form, change): """ @@ -1531,6 +1579,23 @@ class DomainInvitationAdmin(BaseInvitationAdmin): which will be successful if a single User exists for that email; otherwise, will just continue to create the invitation. """ + + # print("***** IN SAVE_MODEL, OUTSIDE OF CHANGE") + + # # If there is a change and it's related to status, look for canceled + # if change and "status" in form.changed_data: + # print("********* DO WE COME INTO THE CHANGE SECTION") + # if obj.status == DomainInvitation.DomainInvitationStatus.CANCELED: + # # Call the transition method to change the status + # obj.cancel_invitation() + # messages.success(request, f"Invitation for {obj.email} has been canceled.") + # return super().save_model(request, obj, form, change) + + # # if invited/retrieved dont alow manual changes + # if obj.status not in [DomainInvitation.DomainInvitationStatus.INVITED, DomainInvitation.DomainInvitationStatus.RETRIEVED]: + # messages.error(request, "You cannot manually set the status to anything other than 'invited' or 'retrieved'.") + # return + if not change: domain = obj.domain domain_org = getattr(domain.domain_info, "portfolio", None) diff --git a/src/registrar/models/domain_invitation.py b/src/registrar/models/domain_invitation.py index 28089dcb5..3954dea7e 100644 --- a/src/registrar/models/domain_invitation.py +++ b/src/registrar/models/domain_invitation.py @@ -78,6 +78,10 @@ class DomainInvitation(TimeStampedModel): @transition(field="status", source=DomainInvitationStatus.INVITED, target=DomainInvitationStatus.CANCELED) def cancel_invitation(self): """When an invitation is canceled, change the status to canceled""" + # print("***** IN CANCEL_INVITATION SECTION") + # logger.info(f"Invitation for {self.email} to {self.domain} has been canceled.") + # print("WHEN INVITATION IS CANCELED > CHANGE STATUS TO CANCELED") + # Send email here maybe? pass @transition(field="status", source=DomainInvitationStatus.CANCELED, target=DomainInvitationStatus.INVITED) From 4b523a75ceab37fe36c5e117871ab722c1cef749 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 4 Feb 2025 16:13:07 -0800 Subject: [PATCH 004/226] This is if we had wanted the cancel invitation by the status --- src/registrar/admin.py | 50 +------------------ .../admin/status_with_clipboard.html | 22 ++++++++ .../admin/domain_invitation_change_form.html | 2 +- .../includes/email_clipboard_fieldset.html | 3 ++ 4 files changed, 27 insertions(+), 50 deletions(-) create mode 100644 src/registrar/templates/admin/status_with_clipboard.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9d4fe80f3..feb3b1883 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1533,44 +1533,12 @@ class DomainInvitationAdmin(BaseInvitationAdmin): # # error. # readonly_fields = ["status"] - # Now it can be edited - readonly_fields = ["status"] + readonly_fields = [] autocomplete_fields = ["domain"] change_form_template = "django/admin/domain_invitation_change_form.html" - # # Custom status filter within DomainInvitationAdmin - # class StatusListFilter(admin.SimpleListFilter): - # # custom filter for status field - - # title = _("status") - # parameter_name = "status" - - # def lookups(self, request, model_admin): - # # only return cancel as option - # return [ - # ('canceled', _('Canceled')), - # ('invited', _('Invited')), - # ('retrieved', _('Retrieved')), - # ] - - # def queryset(self, request, queryset): - # """Filter the queryset based on the selected status.""" - # if self.value(): - # return queryset.filter(status=self.value()) # Apply the filter based on the selected status - # return queryset - - # list_filter = (StatusListFilter,) # Apply the custom filter to the list view - - # # Select domain invitations to change -> Domain invitations - # def changelist_view(self, request, extra_context=None): - # if extra_context is None: - # extra_context = {} - # extra_context["tabtitle"] = "Domain invitations" - # # Get the filtered values - # return super().changelist_view(request, extra_context=extra_context) - def save_model(self, request, obj, form, change): """ Override the save_model method. @@ -1580,22 +1548,6 @@ class DomainInvitationAdmin(BaseInvitationAdmin): just continue to create the invitation. """ - # print("***** IN SAVE_MODEL, OUTSIDE OF CHANGE") - - # # If there is a change and it's related to status, look for canceled - # if change and "status" in form.changed_data: - # print("********* DO WE COME INTO THE CHANGE SECTION") - # if obj.status == DomainInvitation.DomainInvitationStatus.CANCELED: - # # Call the transition method to change the status - # obj.cancel_invitation() - # messages.success(request, f"Invitation for {obj.email} has been canceled.") - # return super().save_model(request, obj, form, change) - - # # if invited/retrieved dont alow manual changes - # if obj.status not in [DomainInvitation.DomainInvitationStatus.INVITED, DomainInvitation.DomainInvitationStatus.RETRIEVED]: - # messages.error(request, "You cannot manually set the status to anything other than 'invited' or 'retrieved'.") - # return - if not change: domain = obj.domain domain_org = getattr(domain.domain_info, "portfolio", None) diff --git a/src/registrar/templates/admin/status_with_clipboard.html b/src/registrar/templates/admin/status_with_clipboard.html new file mode 100644 index 000000000..a62ca5055 --- /dev/null +++ b/src/registrar/templates/admin/status_with_clipboard.html @@ -0,0 +1,22 @@ +{% load static %} + +
+ {{ field.value | capfirst }} + + + +
+ diff --git a/src/registrar/templates/django/admin/domain_invitation_change_form.html b/src/registrar/templates/django/admin/domain_invitation_change_form.html index 6ce6ed0d1..699760fa8 100644 --- a/src/registrar/templates/django/admin/domain_invitation_change_form.html +++ b/src/registrar/templates/django/admin/domain_invitation_change_form.html @@ -11,4 +11,4 @@ {{ block.super }} -{% endblock %} +{% endblock %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html b/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html index f959f8edf..4c0e63d66 100644 --- a/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html +++ b/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html @@ -7,7 +7,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "email" %} {% include "admin/input_with_clipboard.html" with field=field.field %} + {% elif field.field.name == "status" %} + {% include "admin/status_with_clipboard.html" with field=field.field %} {% else %} {{ block.super }} {% endif %} {% endblock field_other %} + From 0e6bc6f07f13122dda318a86ca5e85ea59d71ee3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 5 Feb 2025 06:29:10 -0500 Subject: [PATCH 005/226] 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 dc3351e8c0daec1b1a0fc8e7e629de44ad4ba3d4 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 5 Feb 2025 16:05:49 -0500 Subject: [PATCH 006/226] revise UI of search bars with labels, create component --- .../includes/member_domains_edit_table.html | 46 ++-------------- .../includes/member_domains_table.html | 52 +++++-------------- src/registrar/templates/includes/search.html | 34 ++++++++++++ 3 files changed, 50 insertions(+), 82 deletions(-) create mode 100644 src/registrar/templates/includes/search.html diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html index 0b8ff005a..cefc30c15 100644 --- a/src/registrar/templates/includes/member_domains_edit_table.html +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -36,47 +36,9 @@
- + {% with label_text="Search all domains" item_name="edit-member-domains" aria_label_text="Member domains search component" %} + {% include "includes/search.html" %} + {% endwith %}
@@ -85,7 +47,7 @@ member domains - Assigned domains + Assigned domains Domains diff --git a/src/registrar/templates/includes/member_domains_table.html b/src/registrar/templates/includes/member_domains_table.html index d7839e485..4e63fdbc3 100644 --- a/src/registrar/templates/includes/member_domains_table.html +++ b/src/registrar/templates/includes/member_domains_table.html @@ -1,5 +1,3 @@ -{% load static %} - {% if member %} - diff --git a/src/registrar/templates/admin/analytics_graph_table.html b/src/registrar/templates/admin/analytics_graph_table.html index c339c63c2..58132d023 100644 --- a/src/registrar/templates/admin/analytics_graph_table.html +++ b/src/registrar/templates/admin/analytics_graph_table.html @@ -1,62 +1,26 @@ -{{ data }} - - + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + {% comment %} + This ugly notation is equivalent to data.property_name.start_date_count.index. + Or represented in the pure python way: data[property_name]["start_date_count"][index] + {% endcomment %} + {% with start_counts=data|get_item:property_name|get_item:"start_date_count" end_counts=data|get_item:property_name|get_item:"end_date_count" %} + {% for org_count_type in data.org_count_types %} + {% with index=forloop.counter %} + + + + + + {% endwith %} + {% endfor %} + {% endwith %}
TypeFor start datefor end dateStart date {{ data.start_date }}End date {{ data.end_date }}
Totaldatadata
Federaldatadata
Interstatedatadata
State/Territorydatadata
Tribaldatadata
Countydatadata
Citydatadata
Special Districtdatadata
School Districtdatadata
Election Boarddatadata
{{ org_count_type }}{{ start_counts|slice:index|last }}{{ end_counts|slice:index|last }}
\ No newline at end of file diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index 1b9198c79..596e69a5c 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -126,28 +126,54 @@ class AnalyticsView(View): # include it in the larger context dictionary so it's available in the template rendering context. # This ensures that the admin interface styling and behavior are consistent with other admin pages. **admin.site.each_context(request), - data=dict( - user_count=models.User.objects.all().count(), - domain_count=models.Domain.objects.all().count(), - ready_domain_count=models.Domain.objects.filter(state=models.Domain.State.READY).count(), - last_30_days_applications=last_30_days_applications.count(), - last_30_days_approved_applications=last_30_days_approved_applications.count(), - average_application_approval_time_last_30_days=avg_approval_time_display, - managed_domains_sliced_at_start_date=managed_domains_sliced_at_start_date, - unmanaged_domains_sliced_at_start_date=unmanaged_domains_sliced_at_start_date, - managed_domains_sliced_at_end_date=managed_domains_sliced_at_end_date, - unmanaged_domains_sliced_at_end_date=unmanaged_domains_sliced_at_end_date, - ready_domains_sliced_at_start_date=ready_domains_sliced_at_start_date, - deleted_domains_sliced_at_start_date=deleted_domains_sliced_at_start_date, - ready_domains_sliced_at_end_date=ready_domains_sliced_at_end_date, - deleted_domains_sliced_at_end_date=deleted_domains_sliced_at_end_date, - requests_sliced_at_start_date=requests_sliced_at_start_date, - submitted_requests_sliced_at_start_date=submitted_requests_sliced_at_start_date, - requests_sliced_at_end_date=requests_sliced_at_end_date, - submitted_requests_sliced_at_end_date=submitted_requests_sliced_at_end_date, - start_date=start_date, - end_date=end_date, - ), + data={ + # Tracks what kind of orgs we are keeping count of. + # Used for the details table beneath the graph. + "org_count_types": [ + "Total", + "Federal", + "Interstate", + "State/Territory", + "Tribal", + "County", + "City", + "Special District", + "School District", + "Election Board", + ], + "user_count": models.User.objects.all().count(), + "domain_count": models.Domain.objects.all().count(), + "ready_domain_count": models.Domain.objects.filter(state=models.Domain.State.READY).count(), + "last_30_days_applications": last_30_days_applications.count(), + "last_30_days_approved_applications": last_30_days_approved_applications.count(), + "average_application_approval_time_last_30_days": avg_approval_time_display, + "managed_domains": { + "start_date_count": managed_domains_sliced_at_start_date, + "end_date_count": managed_domains_sliced_at_end_date, + }, + "unmanaged_domains": { + "start_date_count": unmanaged_domains_sliced_at_start_date, + "end_date_count": unmanaged_domains_sliced_at_end_date, + }, + "ready_domains": { + "start_date_count": ready_domains_sliced_at_start_date, + "end_date_count": ready_domains_sliced_at_end_date, + }, + "deleted_domains": { + "start_date_count": deleted_domains_sliced_at_start_date, + "end_date_count": deleted_domains_sliced_at_end_date, + }, + "requests": { + "start_date_count": requests_sliced_at_start_date, + "end_date_count": requests_sliced_at_end_date, + }, + "submitted_requests": { + "start_date_count": submitted_requests_sliced_at_start_date, + "end_date_count": submitted_requests_sliced_at_end_date, + }, + "start_date": start_date, + "end_date": end_date, + }, ) return render(request, "admin/analytics.html", context) From f539ede376626cb475aff9a6f42a48988436b16a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 7 Feb 2025 11:53:27 -0700 Subject: [PATCH 036/226] fix style and andi stuff --- src/registrar/assets/src/sass/_theme/_admin.scss | 8 +++++++- src/registrar/templates/admin/analytics_graph_table.html | 8 ++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/registrar/assets/src/sass/_theme/_admin.scss b/src/registrar/assets/src/sass/_theme/_admin.scss index 4f75fd2fb..035768dad 100644 --- a/src/registrar/assets/src/sass/_theme/_admin.scss +++ b/src/registrar/assets/src/sass/_theme/_admin.scss @@ -536,9 +536,15 @@ details.dja-detail-table { background-color: transparent; } + thead tr { + background-color: var(--darkened-bg); + } + td, th { padding-left: 12px; - border: none + border: none; + background-color: var(--darkened-bg); + color: var(--body-quiet-color); } thead > tr > th { diff --git a/src/registrar/templates/admin/analytics_graph_table.html b/src/registrar/templates/admin/analytics_graph_table.html index 58132d023..88b538745 100644 --- a/src/registrar/templates/admin/analytics_graph_table.html +++ b/src/registrar/templates/admin/analytics_graph_table.html @@ -1,9 +1,9 @@ - +
- - - + + + From f154e9b870335fd6f8c31c755ad2e3f01b92cd1e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 7 Feb 2025 14:37:23 -0700 Subject: [PATCH 037/226] Add desktop / mobile view --- .../assets/src/sass/_theme/_admin.scss | 30 +- src/registrar/templates/admin/analytics.html | 529 +++++++++--------- 2 files changed, 303 insertions(+), 256 deletions(-) diff --git a/src/registrar/assets/src/sass/_theme/_admin.scss b/src/registrar/assets/src/sass/_theme/_admin.scss index 035768dad..05df9a3c2 100644 --- a/src/registrar/assets/src/sass/_theme/_admin.scss +++ b/src/registrar/assets/src/sass/_theme/_admin.scss @@ -548,7 +548,6 @@ details.dja-detail-table { } thead > tr > th { - border-radius: 4px; border-top: none; border-bottom: none; } @@ -930,3 +929,32 @@ ul.add-list-reset { background-color: transparent !important; } } + +@media (min-width: 1024px) { + .analytics-dashboard-charts { + // Desktop layout - charts in top row, details in bottom row + display: grid; + gap: 2rem; + grid-template-columns: 1fr 1fr; + grid-template-areas: + "chart1 chart2" + "details1 details2" + "chart3 chart4" + "details3 details4" + "chart5 chart6" + "details5 details6"; + + .chart-1 { grid-area: chart1; } + .details-1 { grid-area: details1; } + .chart-2 { grid-area: chart2; } + .details-2 { grid-area: details2; } + .chart-3 { grid-area: chart3; } + .details-3 { grid-area: details3; } + .chart-4 { grid-area: chart4; } + .details-4 { grid-area: details4; } + .chart-5 { grid-area: chart5; } + .details-5 { grid-area: details5; } + .chart-6 { grid-area: chart6; } + .details-6 { grid-area: details6; } + } +} diff --git a/src/registrar/templates/admin/analytics.html b/src/registrar/templates/admin/analytics.html index 014f948d1..297f27d46 100644 --- a/src/registrar/templates/admin/analytics.html +++ b/src/registrar/templates/admin/analytics.html @@ -1,258 +1,277 @@ {% extends "admin/base_site.html" %} {% load static %} - -{% block content_title %}

Registrar Analytics

{% endblock %} - -{% block content %} - -
- -
-
-
-

At a glance

-
-
    -
  • User Count: {{ data.user_count }}
  • -
  • Domain Count: {{ data.domain_count }}
  • -
  • Domains in READY state: {{ data.ready_domain_count }}
  • -
  • Domain applications (last 30 days): {{ data.last_30_days_applications }}
  • -
  • Approved applications (last 30 days): {{ data.last_30_days_approved_applications }}
  • -
  • Average approval time for applications (last 30 days): {{ data.average_application_approval_time_last_30_days }}
  • -
-
-
-
- -
- -
-
-
-

Growth reports

-
- {% comment %} - Inputs of type date suck for accessibility. - We'll need to replace those guys with a django form once we figure out how to hook one onto this page. - See the commit "Review for ticket #999" - {% endcomment %} -
-
- - -
-
- - -
-
-
    -
  • - -
  • -
  • - -
  • -
  • - -
  • -
  • - -
  • -
  • - -
  • -
- -
-
- -

Chart: Managed domains

-

{{ data.managed_domains.end_date_count.0 }} managed domains for {{ data.end_date }}

-
-
-
- -

Chart: Unmanaged domains

-

{{ data.unmanaged_domains.end_date_count.0 }} unmanaged domains for {{ data.end_date }}

-
-
-
- -
-
-
- Details for managed domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="managed_domains" %} -
-
-
-
-
- Details for unmanaged domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="unmanaged_domains" %} -
-
-
-
- -
-
- -

Chart: Deleted domains

-

{{ data.deleted_domains.end_date_count.0 }} deleted domains for {{ data.end_date }}

-
-
-
- -

Chart: Ready domains

-

{{ data.ready_domains.end_date_count.0 }} ready domains for {{ data.end_date }}

-
-
-
- -
-
-
- Details for deleted domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="deleted_domains" %} -
-
-
-
-
- Details for ready domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="ready_domains" %} -
-
-
-
- -
-
- -

Chart: Submitted requests

-

{{ data.submitted_requests.end_date_count.0 }} submitted requests for {{ data.end_date }}

-
-
-
- -

Chart: All requests

-

{{ data.requests.end_date_count.0 }} requests for {{ data.end_date }}

-
-
-
- -
-
-
- Details for submitted requests -
- {% include "admin/analytics_graph_table.html" with data=data property_name="submitted_requests" %} -
-
-
-
-
- Details for all requests -
- {% include "admin/analytics_graph_table.html" with data=data property_name="requests" %} -
-
-
-
-
-
-
-
+{% block content_title %} +

Registrar Analytics

{% endblock %} +{% block content %} +
+
+
+
+

At a glance

+
+
    +
  • User Count: {{ data.user_count }}
  • +
  • Domain Count: {{ data.domain_count }}
  • +
  • Domains in READY state: {{ data.ready_domain_count }}
  • +
  • Domain applications (last 30 days): {{ data.last_30_days_applications }}
  • +
  • Approved applications (last 30 days): {{ data.last_30_days_approved_applications }}
  • +
  • Average approval time for applications (last 30 days): {{ data.average_application_approval_time_last_30_days }}
  • +
+
+
+
+ +
+
+
+
+

Growth reports

+
+ {% comment %} + Inputs of type date suck for accessibility. + We'll need to replace those guys with a django form once we figure out how to hook one onto this page. + See the commit "Review for ticket #999" + {% endcomment %} +
+
+ + +
+
+ + +
+
+
    +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
  • + +
  • +
+
+ {% comment %} Managed/Unmanaged domains {% endcomment %} +
+
+ +

Chart: Managed domains

+

{{ data.managed_domains.end_date_count.0 }} managed domains for {{ data.end_date }}

+
+
+
+
+
+
+ Details for managed domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="managed_domains" %} +
+
+
+
+
+
+ +

Chart: Unmanaged domains

+

{{ data.unmanaged_domains.end_date_count.0 }} unmanaged domains for {{ data.end_date }}

+
+
+
+
+
+
+ Details for unmanaged domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="unmanaged_domains" %} +
+
+
+
+ + {% comment %} Deleted/Ready domains {% endcomment %} +
+
+ +

Chart: Deleted domains

+

{{ data.deleted_domains.end_date_count.0 }} deleted domains for {{ data.end_date }}

+
+
+
+
+
+
+ Details for deleted domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="deleted_domains" %} +
+
+
+
+
+
+ +

Chart: Ready domains

+

{{ data.ready_domains.end_date_count.0 }} ready domains for {{ data.end_date }}

+
+
+
+
+
+
+ Details for ready domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="ready_domains" %} +
+
+
+
+ + {% comment %} Requests {% endcomment %} +
+
+ +

Chart: Submitted requests

+

{{ data.submitted_requests.end_date_count.0 }} submitted requests for {{ data.end_date }}

+
+
+
+
+
+
+ Details for submitted requests +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="submitted_requests" %} +
+
+
+
+
+
+ +

Chart: All requests

+

{{ data.requests.end_date_count.0 }} requests for {{ data.end_date }}

+
+
+
+
+
+
+ Details for all requests +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="requests" %} +
+
+
+
+
+
+
+
+
+{% endblock %} \ No newline at end of file From 6fc5a76e923a98a6180bf94771d22e3d94eca557 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 9 Feb 2025 18:20:38 -0700 Subject: [PATCH 038/226] Removed header markup for "clear all" filters link --- .../templates/admin/change_list.html | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/admin/change_list.html b/src/registrar/templates/admin/change_list.html index 43abf0861..43e6fa5b5 100644 --- a/src/registrar/templates/admin/change_list.html +++ b/src/registrar/templates/admin/change_list.html @@ -1,4 +1,5 @@ {% extends "admin/change_list.html" %} +{% load i18n admin_urls static admin_list %} {% block content_title %}

{{ title }}

@@ -46,4 +47,25 @@ {{ block.super }} {% endblock %}
-{% endblock %} \ No newline at end of file +{% endblock %} + + +{% comment %} Replace the Django header markup for clearing all filters with a div. {% endcomment %} +{% block filters %} +{% if cl.has_filters %} + +{% endif %} +{% endblock %} + From f90fe6c4629696616d50cee8bcc87064a18a7c39 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 9 Feb 2025 18:20:51 -0700 Subject: [PATCH 039/226] Removed header markup for "By Status" multiple choice filter --- .../admin/multiple_choice_list_filter.html | 70 ++++++++++--------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/src/registrar/templates/django/admin/multiple_choice_list_filter.html b/src/registrar/templates/django/admin/multiple_choice_list_filter.html index 167059594..2a61fee93 100644 --- a/src/registrar/templates/django/admin/multiple_choice_list_filter.html +++ b/src/registrar/templates/django/admin/multiple_choice_list_filter.html @@ -1,37 +1,41 @@ {% load i18n %} {% load static field_helpers url_helpers %} +
+ + {% blocktrans with filter_title=title %} By {{ filter_title }} {% endblocktrans %} + +
    + {% for choice in choices %} + {% if choice.reset %} + + {{ choice.display }} + + {% endif %} + {% endfor %} -

    {% blocktrans with filter_title=title %} By {{ filter_title }} {% endblocktrans %}

    - + {% for choice in choices %} + {% if not choice.reset %} + + {% if choice.selected and choice.exclude_query_string %} + {{ choice.display }} + + + + {% endif %} + {% if not choice.selected and choice.include_query_string %} + {{ choice.display }} + + + {% endif %} + + {% endif %} + {% endfor %} +
+
From 2c74654544c9ae41223ca527d393a2efc71045d5 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 9 Feb 2025 19:25:37 -0700 Subject: [PATCH 040/226] add skip-to-filters link for admin tables --- src/registrar/templates/admin/change_list.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/templates/admin/change_list.html b/src/registrar/templates/admin/change_list.html index 43abf0861..7256f2af2 100644 --- a/src/registrar/templates/admin/change_list.html +++ b/src/registrar/templates/admin/change_list.html @@ -37,6 +37,8 @@ for {{ search_query }} {% endif %} + + Skip to filters {% endblock %} {% comment %} Replace the Django ul markup with a div. We'll replace the li with a p in change_list_object_tools {% endcomment %} From d684fea8edbe091367b992d5bfad85630622df5c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 10 Feb 2025 09:16:42 -0700 Subject: [PATCH 041/226] test changes --- src/registrar/assets/js/get-gov-reports.js | 35 +++++++++++++++--- .../assets/src/sass/_theme/_admin.scss | 15 ++++---- src/registrar/templates/admin/analytics.html | 36 +++++++++---------- 3 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js index 8bfe32fdd..650a2c6c7 100644 --- a/src/registrar/assets/js/get-gov-reports.js +++ b/src/registrar/assets/js/get-gov-reports.js @@ -59,6 +59,8 @@ /** An IIFE to initialize the analytics page */ (function () { + // Store chart instances globally within this IIFE + const chartInstances = new Map(); function createComparativeColumnChart(canvasId, title, labelOne, labelTwo) { var canvas = document.getElementById(canvasId); if (!canvas) { @@ -80,17 +82,16 @@ borderWidth: 1, data: listOne, backgroundColor: [ - pattern.draw('zigzag-vertical', '#1f77b4'), + pattern.draw("zigzag-vertical", "#1f77b4"), ] }, { label: labelTwo, - backgroundColor: "rgba(75, 192, 192, 0.2)", borderColor: "rgba(75, 192, 192, 1)", borderWidth: 1, data: listTwo, backgroundColor: [ - pattern.draw('diagonal', '#1f77b4'), + pattern.draw("diagonal", "#1f77b4"), ] }, ], @@ -98,7 +99,6 @@ var options = { responsive: true, - maintainAspectRatio: false, plugins: { legend: { position: 'top', @@ -115,11 +115,34 @@ }, }; - new Chart(ctx, { + // Destroy existing chart instance if it exists + if (chartInstances.has(canvasId)) { + chartInstances.get(canvasId).destroy(); + } + + // Create and store new chart instance + const chart = new Chart(ctx, { type: "bar", data: data, options: options, }); + + chartInstances.set(canvasId, chart); + } + + function handleResize() { + // Debounce the resize handler + if (handleResize.timeout) { + clearTimeout(handleResize.timeout); + } + + handleResize.timeout = setTimeout(() => { + chartInstances.forEach((chart, canvasId) => { + if (chart && chart.canvas) { + chart.resize(); + } + }); + }, 100); } function initComparativeColumnCharts() { @@ -130,6 +153,8 @@ createComparativeColumnChart("myChart4", "Ready domains", "Start Date", "End Date"); createComparativeColumnChart("myChart5", "Submitted requests", "Start Date", "End Date"); createComparativeColumnChart("myChart6", "All requests", "Start Date", "End Date"); + + //window.addEventListener("resize", handleResize); }); }; diff --git a/src/registrar/assets/src/sass/_theme/_admin.scss b/src/registrar/assets/src/sass/_theme/_admin.scss index 05df9a3c2..ba7d447ee 100644 --- a/src/registrar/assets/src/sass/_theme/_admin.scss +++ b/src/registrar/assets/src/sass/_theme/_admin.scss @@ -930,11 +930,11 @@ ul.add-list-reset { } } -@media (min-width: 1024px) { +// Break at tablet view +@media (min-width: 768px) { .analytics-dashboard-charts { // Desktop layout - charts in top row, details in bottom row display: grid; - gap: 2rem; grid-template-columns: 1fr 1fr; grid-template-areas: "chart1 chart2" @@ -945,16 +945,17 @@ ul.add-list-reset { "details5 details6"; .chart-1 { grid-area: chart1; } - .details-1 { grid-area: details1; } .chart-2 { grid-area: chart2; } - .details-2 { grid-area: details2; } .chart-3 { grid-area: chart3; } - .details-3 { grid-area: details3; } .chart-4 { grid-area: chart4; } - .details-4 { grid-area: details4; } .chart-5 { grid-area: chart5; } - .details-5 { grid-area: details5; } .chart-6 { grid-area: chart6; } + .details-1 { grid-area: details1; } + .details-2 { grid-area: details2; } + .details-3 { grid-area: details3; } + .details-4 { grid-area: details4; } + .details-5 { grid-area: details5; } .details-6 { grid-area: details6; } } + } diff --git a/src/registrar/templates/admin/analytics.html b/src/registrar/templates/admin/analytics.html index 297f27d46..855f7f870 100644 --- a/src/registrar/templates/admin/analytics.html +++ b/src/registrar/templates/admin/analytics.html @@ -140,10 +140,10 @@
-
+
-
- Details for managed domains +
+ Details for managed domains
{% include "admin/analytics_graph_table.html" with data=data property_name="managed_domains" %}
@@ -163,10 +163,10 @@
-
+
-
- Details for unmanaged domains +
+ Details for unmanaged domains
{% include "admin/analytics_graph_table.html" with data=data property_name="unmanaged_domains" %}
@@ -188,10 +188,10 @@
-
+
-
- Details for deleted domains +
+ Details for deleted domains
{% include "admin/analytics_graph_table.html" with data=data property_name="deleted_domains" %}
@@ -211,10 +211,10 @@
-
+
-
- Details for ready domains +
+ Details for ready domains
{% include "admin/analytics_graph_table.html" with data=data property_name="ready_domains" %}
@@ -236,10 +236,10 @@
-
+
-
- Details for submitted requests +
+ Details for submitted requests
{% include "admin/analytics_graph_table.html" with data=data property_name="submitted_requests" %}
@@ -259,10 +259,10 @@
-
+
-
- Details for all requests +
+ Details for all requests
{% include "admin/analytics_graph_table.html" with data=data property_name="requests" %}
From f08c2ff4ad39d9c97477b225490b3e8edf848d63 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 10 Feb 2025 10:30:50 -0600 Subject: [PATCH 042/226] linter fixes --- src/registrar/models/domain.py | 14 ++++++++------ src/registrar/tests/test_models_domain.py | 10 +++------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index adaa8703b..812cc3894 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1034,7 +1034,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.error(f"registry error removing client hold: {err}") raise (err) - def _delete_domain(self): + def _delete_domain(self): # noqa """This domain should be deleted from the registry may raises RegistryError, should be caught or handled correctly by caller""" @@ -1056,9 +1056,11 @@ class Domain(TimeStampedModel, DomainHelper): new_values, oldNameservers, ) = self.getNameserverChanges(hosts=[]) - + # update the hosts - _ = self._update_host_values(updated_values, oldNameservers) # returns nothing, just need to be run and errors + _ = self._update_host_values( + updated_values, oldNameservers + ) # returns nothing, just need to be run and errors addToDomainList, _ = self.createNewHostList(new_values) deleteHostList, _ = self.createDeleteHostList(deleted_values) responseCode = self.addAndRemoveHostsFromDomain(hostsToAdd=addToDomainList, hostsToDelete=deleteHostList) @@ -1068,7 +1070,7 @@ class Domain(TimeStampedModel, DomainHelper): # if unable to update domain raise error and stop if responseCode != ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY: raise NameserverError(code=nsErrorCodes.BAD_DATA) - + logger.info("Finished removing nameservers from domain") # addAndRemoveHostsFromDomain removes the hosts from the domain object, @@ -1080,7 +1082,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.debug("Deleting non-registrant contacts for %s", self.name) contacts = PublicContact.objects.filter(domain=self) logger.info(f"retrieved contacts for domain: {contacts}") - + for contact in contacts: try: if contact.contact_type != PublicContact.ContactTypeChoices.REGISTRANT: @@ -1094,7 +1096,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.info(f"sent DeleteContact for {contact}") except RegistryError as e: logger.error(f"Error deleting contact: {contact}, {e}", exc_info=True) - + logger.info("Finished deleting contacts") # delete ds data if it exists diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 973a5ad39..8bbd6d60f 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -2887,16 +2887,13 @@ class TestAnalystDelete(MockEppLib): # Check that dsdata is None self.assertEqual(domain.dnssecdata, None) - # Print out all calls from the mockedSendFunction - print("\nAll calls to mockedSendFunction:") - for call in self.mockedSendFunction.call_args_list: - print(f"- {call}") - # Check that the UpdateDomain command was sent to the registry with the correct extension self.mockedSendFunction.assert_has_calls( [ call( - commands.UpdateDomain(name="dsdomain.gov", add=[], rem=[], nsset=None, keyset=None, registrant=None, auth_info=None), + commands.UpdateDomain( + name="dsdomain.gov", add=[], rem=[], nsset=None, keyset=None, registrant=None, auth_info=None + ), cleaned=True, ), ] @@ -2904,7 +2901,6 @@ class TestAnalystDelete(MockEppLib): # Check that the domain was deleted self.assertEqual(domain.state, Domain.State.DELETED) - @less_console_noise_decorator def test_deletion_ready_fsm_failure(self): From e90e31acb713017ee587c1622b7f4425b6cacce9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 10 Feb 2025 09:52:09 -0700 Subject: [PATCH 043/226] Closing in --- src/registrar/assets/js/get-gov-reports.js | 2 +- src/registrar/templates/admin/analytics.html | 226 +++++++++---------- 2 files changed, 102 insertions(+), 126 deletions(-) diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js index 650a2c6c7..7af40be45 100644 --- a/src/registrar/assets/js/get-gov-reports.js +++ b/src/registrar/assets/js/get-gov-reports.js @@ -154,7 +154,7 @@ createComparativeColumnChart("myChart5", "Submitted requests", "Start Date", "End Date"); createComparativeColumnChart("myChart6", "All requests", "Start Date", "End Date"); - //window.addEventListener("resize", handleResize); + window.addEventListener("resize", handleResize); }); }; diff --git a/src/registrar/templates/admin/analytics.html b/src/registrar/templates/admin/analytics.html index 855f7f870..4628ecfb4 100644 --- a/src/registrar/templates/admin/analytics.html +++ b/src/registrar/templates/admin/analytics.html @@ -127,147 +127,123 @@
{% comment %} Managed/Unmanaged domains {% endcomment %} -
-
- -

Chart: Managed domains

-

{{ data.managed_domains.end_date_count.0 }} managed domains for {{ data.end_date }}

-
-
+
+ +

Chart: Managed domains

+

{{ data.managed_domains.end_date_count.0 }} managed domains for {{ data.end_date }}

+
-
-
-
- Details for managed domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="managed_domains" %} -
-
-
+
+
+ Details for managed domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="managed_domains" %} +
+
-
- -

Chart: Unmanaged domains

-

{{ data.unmanaged_domains.end_date_count.0 }} unmanaged domains for {{ data.end_date }}

-
-
+ +

Chart: Unmanaged domains

+

{{ data.unmanaged_domains.end_date_count.0 }} unmanaged domains for {{ data.end_date }}

+
-
-
-
- Details for unmanaged domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="unmanaged_domains" %} -
-
-
+
+
+ Details for unmanaged domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="unmanaged_domains" %} +
+
{% comment %} Deleted/Ready domains {% endcomment %} -
-
- -

Chart: Deleted domains

-

{{ data.deleted_domains.end_date_count.0 }} deleted domains for {{ data.end_date }}

-
-
+
+ +

Chart: Deleted domains

+

{{ data.deleted_domains.end_date_count.0 }} deleted domains for {{ data.end_date }}

+
-
-
-
- Details for deleted domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="deleted_domains" %} -
-
-
+
+
+ Details for deleted domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="deleted_domains" %} +
+
-
-
- -

Chart: Ready domains

-

{{ data.ready_domains.end_date_count.0 }} ready domains for {{ data.end_date }}

-
-
+
+ +

Chart: Ready domains

+

{{ data.ready_domains.end_date_count.0 }} ready domains for {{ data.end_date }}

+
-
-
-
- Details for ready domains -
- {% include "admin/analytics_graph_table.html" with data=data property_name="ready_domains" %} -
-
-
+
+
+ Details for ready domains +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="ready_domains" %} +
+
{% comment %} Requests {% endcomment %} -
-
- -

Chart: Submitted requests

-

{{ data.submitted_requests.end_date_count.0 }} submitted requests for {{ data.end_date }}

-
-
+
+ +

Chart: Submitted requests

+

{{ data.submitted_requests.end_date_count.0 }} submitted requests for {{ data.end_date }}

+
-
-
-
- Details for submitted requests -
- {% include "admin/analytics_graph_table.html" with data=data property_name="submitted_requests" %} -
-
-
+
+
+ Details for submitted requests +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="submitted_requests" %} +
+
-
-
- -

Chart: All requests

-

{{ data.requests.end_date_count.0 }} requests for {{ data.end_date }}

-
-
+
+ +

Chart: All requests

+

{{ data.requests.end_date_count.0 }} requests for {{ data.end_date }}

+
-
-
-
- Details for all requests -
- {% include "admin/analytics_graph_table.html" with data=data property_name="requests" %} -
-
-
+
+
+ Details for all requests +
+ {% include "admin/analytics_graph_table.html" with data=data property_name="requests" %} +
+
From e45f9f9525a92d34199916e250fd40cc9ef91a67 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 10 Feb 2025 10:40:43 -0700 Subject: [PATCH 044/226] Fix uneven scaling --- src/registrar/assets/src/sass/_theme/_admin.scss | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/src/sass/_theme/_admin.scss b/src/registrar/assets/src/sass/_theme/_admin.scss index ba7d447ee..c06c9c926 100644 --- a/src/registrar/assets/src/sass/_theme/_admin.scss +++ b/src/registrar/assets/src/sass/_theme/_admin.scss @@ -930,12 +930,12 @@ ul.add-list-reset { } } -// Break at tablet view -@media (min-width: 768px) { +@media (min-width: 1080px) { .analytics-dashboard-charts { // Desktop layout - charts in top row, details in bottom row display: grid; - grid-template-columns: 1fr 1fr; + // Equal columns each gets 1/2 of the space + grid-template-columns: minmax(0, 1fr) minmax(0, 1fr); grid-template-areas: "chart1 chart2" "details1 details2" From 78be172ee4fd3bf6744cffad46420573047d1dbb Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Mon, 10 Feb 2025 13:20:46 -0600 Subject: [PATCH 045/226] add retry mechanism to domain deletion --- src/registrar/models/domain.py | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 812cc3894..8366014a3 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -2,7 +2,7 @@ from itertools import zip_longest import logging import ipaddress import re -from datetime import date, timedelta +from datetime import date, time, timedelta from typing import Optional from django.db import transaction from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore @@ -1114,12 +1114,34 @@ class Domain(TimeStampedModel, DomainHelper): e.note = "Error deleting ds data for %s" % self.name raise e + # Previous deletions have to complete before we can delete the domain + # This is a polling mechanism to ensure that the domain is deleted try: - logger.info("Deleting domain %s", self.name) - request = commands.DeleteDomain(name=self.name) - registry.send(request, cleaned=True) + logger.info("Attempting to delete domain %s", self.name) + delete_request = commands.DeleteDomain(name=self.name) + max_attempts = 5 # maximum number of retries + wait_interval = 1 # seconds to wait between attempts + + for attempt in range(max_attempts): + try: + registry.send(delete_request, cleaned=True) + logger.info("Domain %s deleted successfully.", self.name) + break # exit the loop on success + except RegistryError as e: + error = e + logger.warning( + "Attempt %d of %d: Domain deletion not possible yet: %s. Retrying in %d seconds.", + attempt + 1, + max_attempts, + e, + wait_interval, + ) + time.sleep(wait_interval) + else: + logger.error("Exceeded maximum attempts to delete domain %s", self.name) + raise error except RegistryError as e: - logger.error(f"Error deleting domain {self}: {e}") + logger.error("Error deleting domain %s after polling: %s", self.name, e) raise e def __str__(self) -> str: From 6d180eae71cf9a4a73c96b08ea69e5496a6ee3cf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 10 Feb 2025 13:08:58 -0700 Subject: [PATCH 046/226] Update analytics.html --- src/registrar/templates/admin/analytics.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/admin/analytics.html b/src/registrar/templates/admin/analytics.html index 4628ecfb4..b36511206 100644 --- a/src/registrar/templates/admin/analytics.html +++ b/src/registrar/templates/admin/analytics.html @@ -125,7 +125,7 @@ -
+
{% comment %} Managed/Unmanaged domains {% endcomment %}
Date: Mon, 10 Feb 2025 13:21:47 -0700 Subject: [PATCH 047/226] cleanup --- .../assets/src/sass/_theme/_admin.scss | 1 + src/registrar/templates/admin/analytics.html | 24 +++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/registrar/assets/src/sass/_theme/_admin.scss b/src/registrar/assets/src/sass/_theme/_admin.scss index c06c9c926..07749d2dd 100644 --- a/src/registrar/assets/src/sass/_theme/_admin.scss +++ b/src/registrar/assets/src/sass/_theme/_admin.scss @@ -934,6 +934,7 @@ ul.add-list-reset { .analytics-dashboard-charts { // Desktop layout - charts in top row, details in bottom row display: grid; + gap: 2rem; // Equal columns each gets 1/2 of the space grid-template-columns: minmax(0, 1fr) minmax(0, 1fr); grid-template-areas: diff --git a/src/registrar/templates/admin/analytics.html b/src/registrar/templates/admin/analytics.html index b36511206..3747dc7b6 100644 --- a/src/registrar/templates/admin/analytics.html +++ b/src/registrar/templates/admin/analytics.html @@ -127,7 +127,7 @@
{% comment %} Managed/Unmanaged domains {% endcomment %} -
+
{{ data.managed_domains.end_date_count.0 }} managed domains for {{ data.end_date }}

-
+
Details for managed domains
@@ -146,7 +146,7 @@
-
+
{{ data.unmanaged_domains.end_date_count.0 }} unmanaged domains for {{ data.end_date }}

-
+
Details for unmanaged domains
@@ -167,7 +167,7 @@
{% comment %} Deleted/Ready domains {% endcomment %} -
+
{{ data.deleted_domains.end_date_count.0 }} deleted domains for {{ data.end_date }}

-
+
Details for deleted domains
@@ -186,7 +186,7 @@
-
+
{{ data.ready_domains.end_date_count.0 }} ready domains for {{ data.end_date }}

-
+
Details for ready domains
@@ -207,7 +207,7 @@
{% comment %} Requests {% endcomment %} -
+
{{ data.submitted_requests.end_date_count.0 }} submitted requests for {{ data.end_date }}

-
+
Details for submitted requests
@@ -226,7 +226,7 @@
-
+
{{ data.requests.end_date_count.0 }} requests for {{ data.end_date }}

-
+
Details for all requests
From 6f3888f57aa97c223bd67957bb55f53bb27439fc Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 15:27:31 -0500 Subject: [PATCH 048/226] expanded row --- .../assets/src/js/getgov/table-members.js | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/src/registrar/assets/src/js/getgov/table-members.js b/src/registrar/assets/src/js/getgov/table-members.js index 75a7c29ac..5e93ecab8 100644 --- a/src/registrar/assets/src/js/getgov/table-members.js +++ b/src/registrar/assets/src/js/getgov/table-members.js @@ -87,7 +87,7 @@ export class MembersTable extends BaseTable { // generate html blocks for domains and permissions for the member let domainsHTML = this.generateDomainsHTML(num_domains, member.domain_names, member.domain_urls, member.action_url); - let permissionsHTML = this.generatePermissionsHTML(member.permissions, customTableOptions.UserPortfolioPermissionChoices); + let permissionsHTML = this.generatePermissionsHTML(member.is_admin, member.permissions, customTableOptions.UserPortfolioPermissionChoices); // domainsHTML block and permissionsHTML block need to be wrapped with hide/show toggle, Expand let showMoreButton = ''; @@ -257,10 +257,11 @@ export class MembersTable extends BaseTable { let domainsHTML = ''; // Only generate HTML if the member has one or more assigned domains + + domainsHTML += "
"; + domainsHTML += "

Domains assigned

"; if (num_domains > 0) { - domainsHTML += "
"; - domainsHTML += "

Domains assigned

"; - domainsHTML += `

This member is assigned to ${num_domains} domain${num_domains > 1 ? 's' : ''}:

`; + domainsHTML += `

This member is assigned to ${num_domains} domain${num_domains > 1 ? 's' : ''}:

`; domainsHTML += "
    "; // Display up to 6 domains with their URLs @@ -269,13 +270,15 @@ export class MembersTable extends BaseTable { } domainsHTML += "
"; - - // If there are more than 6 domains, display a "View assigned domains" link - domainsHTML += `

View assigned domains

`; - - domainsHTML += "
"; + } else { + domainsHTML += `

This member is assigned to 0 domains.

`; } + // If there are more than 6 domains, display a "View assigned domains" link + domainsHTML += `

View domains assigned

`; + + domainsHTML += "
"; + return domainsHTML; } @@ -387,40 +390,51 @@ export class MembersTable extends BaseTable { * - If no relevant permissions are found, the function returns a message stating that the user has no additional permissions. * - The resulting HTML always includes a header "Additional permissions for this member" and appends the relevant permission descriptions. */ - generatePermissionsHTML(member_permissions, UserPortfolioPermissionChoices) { + generatePermissionsHTML(is_admin, member_permissions, UserPortfolioPermissionChoices) { let permissionsHTML = ''; // Define shared classes across elements for easier refactoring - let sharedParagraphClasses = "font-body-xs text-base-dark margin-top-1 p--blockquote"; + let sharedParagraphClasses = "font-body-xs text-base-darker margin-top-1 p--blockquote"; + + // Member access + if (is_admin) { + permissionsHTML += `

Member access: Admin

`; + } else { + permissionsHTML += `

Member access: Basic

`; + } // Check domain-related permissions if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS)) { - permissionsHTML += `

Domains: Can view all organization domains. Can manage domains they are assigned to and edit information about the domain (including DNS settings).

`; + permissionsHTML += `

Domains: Viewer

`; } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS)) { - permissionsHTML += `

Domains: Can manage domains they are assigned to and edit information about the domain (including DNS settings).

`; + permissionsHTML += `

Domains: Viewer, limited

`; } // Check request-related permissions if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_REQUESTS)) { - permissionsHTML += `

Domain requests: Can view all organization domain requests. Can create domain requests and modify their own requests.

`; + permissionsHTML += `

Domain requests: Creator

`; } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS)) { - permissionsHTML += `

Domain requests (view-only): Can view all organization domain requests. Can't create or modify any domain requests.

`; + permissionsHTML += `

Domain requests: Viewer

`; + } else { + permissionsHTML += `

Domain requests: No access

`; } // Check member-related permissions if (member_permissions.includes(UserPortfolioPermissionChoices.EDIT_MEMBERS)) { - permissionsHTML += `

Members: Can manage members including inviting new members, removing current members, and assigning domains to members.

`; + permissionsHTML += `

Members: Manager

`; } else if (member_permissions.includes(UserPortfolioPermissionChoices.VIEW_MEMBERS)) { - permissionsHTML += `

Members (view-only): Can view all organizational members. Can't manage any members.

`; + permissionsHTML += `

Members: Viewer

`; + } else { + permissionsHTML += `

Members: No access

`; } // If no specific permissions are assigned, display a message indicating no additional permissions if (!permissionsHTML) { - permissionsHTML += `

No additional permissions: There are no additional permissions for this member.

`; + permissionsHTML += `

No additional permissions: There are no additional permissions for this member.

`; } // Add a permissions header and wrap the entire output in a container - permissionsHTML = `

Additional permissions for this member

${permissionsHTML}
`; + permissionsHTML = `

Member access and permissions

${permissionsHTML}
`; return permissionsHTML; } From d9d751d6aede506125a2d61f539aa0831c660154 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 10 Feb 2025 13:34:26 -0700 Subject: [PATCH 049/226] fix color --- src/registrar/assets/js/get-gov-reports.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js index 8bfe32fdd..403a9c41e 100644 --- a/src/registrar/assets/js/get-gov-reports.js +++ b/src/registrar/assets/js/get-gov-reports.js @@ -75,22 +75,22 @@ datasets: [ { label: labelOne, - backgroundColor: "rgba(255, 99, 132, 0.2)", + backgroundColor: "rgba(255, 99, 132, 0.3)", borderColor: "rgba(255, 99, 132, 1)", borderWidth: 1, data: listOne, backgroundColor: [ - pattern.draw('zigzag-vertical', '#1f77b4'), + pattern.draw('diagonal-right-left', 'rgba(255, 99, 132, 0.3)'), ] }, { label: labelTwo, - backgroundColor: "rgba(75, 192, 192, 0.2)", + backgroundColor: "rgba(75, 192, 192, 0.3)", borderColor: "rgba(75, 192, 192, 1)", borderWidth: 1, data: listTwo, backgroundColor: [ - pattern.draw('diagonal', '#1f77b4'), + pattern.draw('diagonal', 'rgba(75, 192, 192, 0.3)'), ] }, ], From 16e251c4fc25ec985697de0d9269e249fea1a229 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 10 Feb 2025 12:47:53 -0800 Subject: [PATCH 050/226] Fix logic and additional unit tests --- src/registrar/admin.py | 12 +++- .../admin/change_form_object_tools.html | 26 ++++--- src/registrar/tests/test_admin_domain.py | 70 +++++++++++++++---- 3 files changed, 82 insertions(+), 26 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b1b3d8adb..71c672dd7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1519,8 +1519,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin): change_form_template = "django/admin/domain_invitation_change_form.html" def change_view(self, request, object_id, form_url="", extra_context=None): - """Override the change_view to add the invitation obj for the - change_form_object_tools template""" + """Override the change_view to add the invitation obj for the change_form_object_tools template""" if extra_context is None: extra_context = {} @@ -1529,6 +1528,15 @@ class DomainInvitationAdmin(BaseInvitationAdmin): invitation = get_object_or_404(DomainInvitation, id=object_id) extra_context["invitation"] = invitation + if request.method == "POST" and "cancel_invitation" in request.POST: + if invitation.status == DomainInvitation.DomainInvitationStatus.INVITED: + invitation.cancel_invitation() + invitation.save(update_fields=["status"]) + messages.success(request, _("Invitation canceled successfully.")) + + # Redirect back to the change view + return redirect(reverse("admin:registrar_domaininvitation_change", args=[object_id])) + return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): diff --git a/src/registrar/templates/admin/change_form_object_tools.html b/src/registrar/templates/admin/change_form_object_tools.html index 541f4d162..a76609538 100644 --- a/src/registrar/templates/admin/change_form_object_tools.html +++ b/src/registrar/templates/admin/change_form_object_tools.html @@ -16,18 +16,22 @@ {% else %}
    {% if opts.model_name == 'domaininvitation' %} -
  • -
    - {% csrf_token %} - - -
  • + {% if invitation.status == invitation.DomainInvitationStatus.INVITED %} +
  • +
    + {% csrf_token %} + + + +
  • + {% endif %} {% endif %} +
  • {% translate "History" %} diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 07940e202..c192b082a 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -514,7 +514,7 @@ class DomainInvitationAdminTest(TestCase): self.client.force_login(self.staffuser) super().setUp() - def test_cancel_invitation_flow_in_admin(self): + def test_successful_cancel_invitation_flow_in_admin(self): """Testing canceling a domain invitation in Django Admin.""" # 1. Create a domain and assign staff user role + domain manager @@ -539,21 +539,65 @@ class DomainInvitationAdminTest(TestCase): self.assertEqual(response.status_code, 200) self.assertContains(response, "Cancel invitation") - # 5. Click the "Cancel invitation" button (a POST) - cancel_invitation_url = reverse("invitation-cancel", args=[invitation.id]) - response = self.client.post(cancel_invitation_url, follow=True) + # 5. Click the cancel invitation button + response = self.client.post(domain_invitation_change_url, {"cancel_invitation": "true"}, follow=True) - # 6.Confirm we're redirected to the domain managers page for the domain - expected_redirect_url = reverse("domain-users", args=[domain.id]) - self.assertRedirects(response, expected_redirect_url) + # 6. Make sure we're redirect back to the change view page in /admin + self.assertRedirects(response, domain_invitation_change_url) - # 7. Get the messages - messages = list(get_messages(response.wsgi_request)) - message_texts = [str(message) for message in messages] + # 7. Confirm cancellation confirmation message appears + expected_message = f"Invitation for {invitation.email} on {domain.name} is canceled" + self.assertContains(response, expected_message) - # 8. Check that the success banner text is in the messages - expected_message = f"Canceled invitation to {invitation.email}." - self.assertIn(expected_message, message_texts) + def test_no_cancel_invitation_button_in_retrieved_state(self): + """Shouldn't be able to see the "Cancel invitation" button if invitation is RETRIEVED state""" + + # 1. Create a domain and assign staff user role + domain manager + domain = Domain.objects.create(name="retrieved.gov") + UserDomainRole.objects.create(user=self.staffuser, domain=domain, role="manager") + + # 2. Invite a domain manager to the above domain and NOT in invited state + invitation = DomainInvitation.objects.create( + email="retrievedinvitation@meoward.com", + domain=domain, + status=DomainInvitation.DomainInvitationStatus.RETRIEVED, + ) + + # 3. Go to the Domain Invitations list in /admin + domain_invitation_list_url = reverse("admin:registrar_domaininvitation_changelist") + response = self.client.get(domain_invitation_list_url) + self.assertEqual(response.status_code, 200) + + # 4. Go to the change view of that invitation and make sure you CANNOT see the button + domain_invitation_change_url = reverse("admin:registrar_domaininvitation_change", args=[invitation.id]) + response = self.client.get(domain_invitation_change_url) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, "Cancel invitation") + + def test_no_cancel_invitation_button_in_canceled_state(self): + """Shouldn't be able to see the "Cancel invitation" button if invitation is CANCELED state""" + + # 1. Create a domain and assign staff user role + domain manager + domain = Domain.objects.create(name="canceled.gov") + UserDomainRole.objects.create(user=self.staffuser, domain=domain, role="manager") + + # 2. Invite a domain manager to the above domain and NOT in invited state + invitation = DomainInvitation.objects.create( + email="canceledinvitation@meoward.com", + domain=domain, + status=DomainInvitation.DomainInvitationStatus.CANCELED, + ) + + # 3. Go to the Domain Invitations list in /admin + domain_invitation_list_url = reverse("admin:registrar_domaininvitation_changelist") + response = self.client.get(domain_invitation_list_url) + self.assertEqual(response.status_code, 200) + + # 4. Go to the change view of that invitation and make sure you CANNOT see the button + domain_invitation_change_url = reverse("admin:registrar_domaininvitation_change", args=[invitation.id]) + response = self.client.get(domain_invitation_change_url) + self.assertEqual(response.status_code, 200) + self.assertNotContains(response, "Cancel invitation") class TestDomainAdminWithClient(TestCase): From 8464cf7bdae63f633c6de27d0181193b0da0a45b Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 10 Feb 2025 12:54:52 -0800 Subject: [PATCH 051/226] Fix spacing --- src/registrar/admin.py | 8 ++++++++ .../django/admin/includes/email_clipboard_fieldset.html | 3 +-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 71c672dd7..57590aaf5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1518,6 +1518,14 @@ class DomainInvitationAdmin(BaseInvitationAdmin): change_form_template = "django/admin/domain_invitation_change_form.html" + # Select domain invitations to change -> Domain invitations + def changelist_view(self, request, extra_context=None): + if extra_context is None: + extra_context = {} + extra_context["tabtitle"] = "Domain invitations" + # Get the filtered values + return super().changelist_view(request, extra_context=extra_context) + def change_view(self, request, object_id, form_url="", extra_context=None): """Override the change_view to add the invitation obj for the change_form_object_tools template""" diff --git a/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html b/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html index 7907b5180..733fc3787 100644 --- a/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html +++ b/src/registrar/templates/django/admin/includes/email_clipboard_fieldset.html @@ -10,5 +10,4 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% else %} {{ block.super }} {% endif %} -{% endblock field_other %} - +{% endblock field_other %} \ No newline at end of file From 898168f44f53b8c19ab571b166592316dbbde193 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 10 Feb 2025 12:55:31 -0800 Subject: [PATCH 052/226] Remove extraneous comment --- src/registrar/admin.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 57590aaf5..d1ca834d7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1482,8 +1482,6 @@ class BaseInvitationAdmin(ListHeaderAdmin): class DomainInvitationAdmin(BaseInvitationAdmin): """Custom domain invitation admin class.""" - # form = DomainInvitationAdminForm - class Meta: model = models.DomainInvitation fields = "__all__" From e751388533a40176dea8285d783b1078f7747989 Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Mon, 10 Feb 2025 15:58:14 -0500 Subject: [PATCH 053/226] better error handling in domain deletion --- src/registrar/admin.py | 4 ++-- src/registrar/models/domain.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7dbe7abb0..d22bff58f 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3679,10 +3679,10 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): ), messages.ERROR, ) - except Exception: + except Exception as e: self.message_user( request, - "Could not delete: An unspecified error occured", + f"Could not delete: An unspecified error occured: {e}", messages.ERROR, ) else: diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 8366014a3..23ffb2ad6 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1097,7 +1097,7 @@ class Domain(TimeStampedModel, DomainHelper): except RegistryError as e: logger.error(f"Error deleting contact: {contact}, {e}", exc_info=True) - logger.info("Finished deleting contacts") + logger.info(f"Finished deleting contacts for {self.name}") # delete ds data if it exists if self.dnssecdata: From 663e34fd5b6024b3f1306d981a7fc4b43e29dfcd Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 10 Feb 2025 13:02:43 -0800 Subject: [PATCH 054/226] Remove unused import for linting --- src/registrar/tests/test_admin_domain.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index c192b082a..24ade9302 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -31,9 +31,6 @@ from .common import ( ) from unittest.mock import ANY, call, patch -from django.contrib.messages import get_messages - - import boto3_mocking # type: ignore import logging From e900255e681b35b3208bc85457d97bcbb4df25f9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 10 Feb 2025 14:06:10 -0700 Subject: [PATCH 055/226] fix cutoff bug --- src/registrar/assets/js/get-gov-reports.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js index 428b576ee..2cc8157b5 100644 --- a/src/registrar/assets/js/get-gov-reports.js +++ b/src/registrar/assets/js/get-gov-reports.js @@ -100,6 +100,7 @@ var options = { responsive: true, + maintainAspectRatio: false, plugins: { legend: { position: 'top', From 12fa9548a136521c910926994ee7e18cc53bd750 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 10 Feb 2025 14:07:41 -0700 Subject: [PATCH 056/226] Update get-gov-reports.js --- src/registrar/assets/js/get-gov-reports.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js index 2cc8157b5..d458ae05a 100644 --- a/src/registrar/assets/js/get-gov-reports.js +++ b/src/registrar/assets/js/get-gov-reports.js @@ -59,7 +59,6 @@ /** An IIFE to initialize the analytics page */ (function () { - // Store chart instances globally within this IIFE const chartInstances = new Map(); function createComparativeColumnChart(canvasId, title, labelOne, labelTwo) { var canvas = document.getElementById(canvasId); @@ -117,12 +116,10 @@ }, }; - // Destroy existing chart instance if it exists if (chartInstances.has(canvasId)) { chartInstances.get(canvasId).destroy(); } - // Create and store new chart instance const chart = new Chart(ctx, { type: "bar", data: data, From 72a2f6305104119e6f9c138118254e92918aa51f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 16:18:03 -0500 Subject: [PATCH 057/226] Member profile page --- .../src/js/getgov/portfolio-member-page.js | 2 +- src/registrar/forms/portfolio.py | 2 +- .../includes/member_domain_management.html | 4 +-- .../includes/member_permissions_summary.html | 16 ++++++------ .../templates/includes/summary_item.html | 26 ++++++++++++------- src/registrar/templates/portfolio_member.html | 6 ++--- src/registrar/tests/test_views_portfolio.py | 2 +- 7 files changed, 33 insertions(+), 25 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 2725224f1..76a5032da 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/templates/includes/member_domain_management.html b/src/registrar/templates/includes/member_domain_management.html index 1e5b29994..2adc3f950 100644 --- a/src/registrar/templates/includes/member_domain_management.html +++ b/src/registrar/templates/includes/member_domain_management.html @@ -1,6 +1,6 @@ -

    Assigned domains

    {% if domain_count > 0 %} +

    Domains assigned

    {{domain_count}}

    {% else %} -

    This member does not manage any domains.{% if manage_button %} To assign this member a domain, click "Manage".{% endif %}

    +

    This member does not manage any domains.{% if manage_button %} To assign this member a domain, click "Edit".{% endif %}

    {% endif %} diff --git a/src/registrar/templates/includes/member_permissions_summary.html b/src/registrar/templates/includes/member_permissions_summary.html index 3a91d16f6..8e3dad528 100644 --- a/src/registrar/templates/includes/member_permissions_summary.html +++ b/src/registrar/templates/includes/member_permissions_summary.html @@ -9,25 +9,25 @@

    Domains

    {% if member_has_view_all_domains_portfolio_permission %} -

    Viewer, all

    +

    Viewer: Can view all domains for the organization

    {% else %} -

    Viewer, limited

    +

    Viewer, limited: Can view only the domains they manage

    {% endif %}

    Domain requests

    {% if member_has_edit_request_portfolio_permission %} -

    Creator

    +

    Creator: Can view all domain requests for the organization and create requests

    {% elif member_has_view_all_requests_portfolio_permission %} -

    Viewer

    +

    Viewer: Can view all domain requests for the organization

    {% else %} -

    No access

    +

    No access: Cannot view or create domain requests

    {% endif %}

    Members

    {% if member_has_edit_members_portfolio_permission %} -

    Manager

    +

    Manager: Can view and manage all member permissions

    {% elif member_has_view_members_portfolio_permission %} -

    Viewer

    +

    Viewer: Can view all members permissions

    {% else %} -

    No access

    +

    No access: Cannot view member permissions

    {% endif %} \ No newline at end of file diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index d062a7b4e..a091e5ab7 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -134,18 +134,26 @@ {% endif %}
- {% if editable and edit_link %} + {% if editable and edit_link or view_button %}
- - - {% if manage_button %}Manage{% elif view_button %}View{% else %}Edit{% endif %} {{ title }} - + > + + {% if manage_button %} + Manage + {% elif editable and edit_link %} + Edit + {% else %} + View + {% endif %} + {{ title|default:"Page" }} +
{% endif %} +
diff --git a/src/registrar/templates/portfolio_member.html b/src/registrar/templates/portfolio_member.html index 709582e71..477599d49 100644 --- a/src/registrar/templates/portfolio_member.html +++ b/src/registrar/templates/portfolio_member.html @@ -101,11 +101,11 @@ Organization member {% include "includes/summary_item.html" with title='Member access and permissions' permissions=True value=portfolio_invitation edit_link=edit_url editable=has_edit_members_portfolio_permission %} {% endif %} - {% comment %}view_button is passed below as true in all cases. This is because manage_button logic will trump view_button logic; ie. if manage_button is true, view_button will never be looked at{% endcomment %} + {% comment %}view_button is passed below as true in all cases. This is because editable logic will trump view_button logic; ie. if editable is true, view_button will never be looked at{% endcomment %} {% if portfolio_permission %} - {% include "includes/summary_item.html" with title='Domain management' domain_mgmt=True value=portfolio_permission.get_managed_domains_count edit_link=domains_url editable=True manage_button=has_edit_members_portfolio_permission view_button=True %} + {% include "includes/summary_item.html" with title='Domain assignment' domain_mgmt=True value=portfolio_permission.get_managed_domains_count edit_link=domains_url editable=has_edit_members_portfolio_permission view_button=True %} {% elif portfolio_invitation %} - {% include "includes/summary_item.html" with title='Domain management' domain_mgmt=True value=portfolio_invitation.get_managed_domains_count edit_link=domains_url editable=True manage_button=has_edit_members_portfolio_permission view_button=True %} + {% include "includes/summary_item.html" with title='Domain assignment' domain_mgmt=True value=portfolio_invitation.get_managed_domains_count edit_link=domains_url editable=has_edit_members_portfolio_permission view_button=True %} {% endif %}
diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 097aa1879..7a664a8c6 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 891d0f3dbb11bb13d434adb03f47feca1003a894 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 16:21:17 -0500 Subject: [PATCH 058/226] domain assignments page --- src/registrar/templates/includes/member_domains_edit_table.html | 2 +- src/registrar/templates/includes/member_domains_table.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/includes/member_domains_edit_table.html b/src/registrar/templates/includes/member_domains_edit_table.html index 0b8ff005a..ac05dc14f 100644 --- a/src/registrar/templates/includes/member_domains_edit_table.html +++ b/src/registrar/templates/includes/member_domains_edit_table.html @@ -100,7 +100,7 @@ >
From 77711894b3061e36ba63867f8c99d150b075304a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 16:29:40 -0500 Subject: [PATCH 060/226] Add a new member page --- src/registrar/templates/portfolio_members_add_new.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/portfolio_members_add_new.html b/src/registrar/templates/portfolio_members_add_new.html index 464eaefce..c3a648bdc 100644 --- a/src/registrar/templates/portfolio_members_add_new.html +++ b/src/registrar/templates/portfolio_members_add_new.html @@ -55,7 +55,7 @@

What level of access would you like to grant this member?

-

Select one *

+

Select the level of access for this member. *

{% with add_class="usa-radio__input--tile" add_legend_class="usa-sr-only" %} {% input_with_errors form.role %} @@ -88,7 +88,7 @@ aria-controls="invite-member-modal" data-open-modal >Trigger invite member modal - +
From 586d83adc172a058e288c1bba7207123ee734cbe Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 16:44:57 -0500 Subject: [PATCH 061/226] Fix unit tests --- src/registrar/tests/test_views_portfolio.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 7a664a8c6..7d56e1650 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -952,7 +952,7 @@ class TestPortfolio(WebTest): self.assertContains(response, "Creator") self.assertContains(response, "Manager") self.assertContains( - response, 'This member does not manage any domains. To assign this member a domain, click "Manage"' + response, 'This member does not manage any domains. To assign this member a domain, click "Edit"' ) # Assert buttons and links within the page are correct @@ -1067,7 +1067,7 @@ class TestPortfolio(WebTest): self.assertContains(response, "Creator") self.assertContains(response, "Manager") self.assertContains( - response, 'This member does not manage any domains. To assign this member a domain, click "Manage"' + response, 'This member does not manage any domains. To assign this member a domain, click "Edit"' ) # Assert buttons and links within the page are correct self.assertContains(response, "wrapper-delete-action") # test that 3 dot is present From 1d54fe6b43e1716a20c39ee269408b86c13437a8 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 16:50:54 -0500 Subject: [PATCH 062/226] fix unit tests --- src/registrar/tests/test_views_portfolio.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 7d56e1650..c00892eca 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -952,7 +952,7 @@ class TestPortfolio(WebTest): self.assertContains(response, "Creator") self.assertContains(response, "Manager") self.assertContains( - response, 'This member does not manage any domains. To assign this member a domain, click "Edit"' + response, 'This member does not manage any domains.' ) # Assert buttons and links within the page are correct @@ -1067,7 +1067,7 @@ class TestPortfolio(WebTest): self.assertContains(response, "Creator") self.assertContains(response, "Manager") self.assertContains( - response, 'This member does not manage any domains. To assign this member a domain, click "Edit"' + response, 'This member does not manage any domains.' ) # Assert buttons and links within the page are correct self.assertContains(response, "wrapper-delete-action") # test that 3 dot is present From c44c66ae96eebbff82ac1883b7fb79c69f4daf1a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 17:01:43 -0500 Subject: [PATCH 063/226] wip --- src/registrar/assets/src/js/getgov/table-members.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/src/js/getgov/table-members.js b/src/registrar/assets/src/js/getgov/table-members.js index 5e93ecab8..e876ce0f9 100644 --- a/src/registrar/assets/src/js/getgov/table-members.js +++ b/src/registrar/assets/src/js/getgov/table-members.js @@ -108,7 +108,7 @@ export class MembersTable extends BaseTable { `; - showMoreRow.innerHTML = `
`; + showMoreRow.innerHTML = ``; showMoreRow.classList.add('show-more-content'); showMoreRow.classList.add('display-none'); showMoreRow.id = unique_id; @@ -258,7 +258,7 @@ export class MembersTable extends BaseTable { // Only generate HTML if the member has one or more assigned domains - domainsHTML += "
"; + domainsHTML += "
"; domainsHTML += "

Domains assigned

"; if (num_domains > 0) { domainsHTML += `

This member is assigned to ${num_domains} domain${num_domains > 1 ? 's' : ''}:

`; @@ -275,7 +275,7 @@ export class MembersTable extends BaseTable { } // If there are more than 6 domains, display a "View assigned domains" link - domainsHTML += `

View domains assigned

`; + domainsHTML += `

View domain assignments

`; domainsHTML += "
"; @@ -434,7 +434,7 @@ export class MembersTable extends BaseTable { } // Add a permissions header and wrap the entire output in a container - permissionsHTML = `

Member access and permissions

${permissionsHTML}
`; + permissionsHTML = `

Member access and permissions

${permissionsHTML}
`; return permissionsHTML; } From d7c76b6d20e612c661a413eace0f150ec25550bd Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 17:03:34 -0500 Subject: [PATCH 064/226] Domain assignments with s --- src/registrar/templates/portfolio_member.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/portfolio_member.html b/src/registrar/templates/portfolio_member.html index 477599d49..46e0cced6 100644 --- a/src/registrar/templates/portfolio_member.html +++ b/src/registrar/templates/portfolio_member.html @@ -103,9 +103,9 @@ Organization member {% comment %}view_button is passed below as true in all cases. This is because editable logic will trump view_button logic; ie. if editable is true, view_button will never be looked at{% endcomment %} {% if portfolio_permission %} - {% include "includes/summary_item.html" with title='Domain assignment' domain_mgmt=True value=portfolio_permission.get_managed_domains_count edit_link=domains_url editable=has_edit_members_portfolio_permission view_button=True %} + {% include "includes/summary_item.html" with title='Domain assignments' domain_mgmt=True value=portfolio_permission.get_managed_domains_count edit_link=domains_url editable=has_edit_members_portfolio_permission view_button=True %} {% elif portfolio_invitation %} - {% include "includes/summary_item.html" with title='Domain assignment' domain_mgmt=True value=portfolio_invitation.get_managed_domains_count edit_link=domains_url editable=has_edit_members_portfolio_permission view_button=True %} + {% include "includes/summary_item.html" with title='Domain assignments' domain_mgmt=True value=portfolio_invitation.get_managed_domains_count edit_link=domains_url editable=has_edit_members_portfolio_permission view_button=True %} {% endif %}
From fe1780fbb921138e88a2faea6a734b5a2681f30c Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 17:07:56 -0500 Subject: [PATCH 065/226] wip --- src/registrar/templates/portfolio_member.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/portfolio_member.html b/src/registrar/templates/portfolio_member.html index 46e0cced6..b270e03fb 100644 --- a/src/registrar/templates/portfolio_member.html +++ b/src/registrar/templates/portfolio_member.html @@ -65,7 +65,7 @@ Organization member
{% csrf_token %}
- Last active: + Last active: {% if member and member.last_login %} {{ member.last_login }} {% elif portfolio_invitation %} @@ -75,7 +75,7 @@ Organization member {% endif %}
- Full name: + Full name: {% if member %} {% if member.first_name or member.last_name %} {{ member.get_formatted_name }} @@ -87,7 +87,7 @@ Organization member {% endif %}
- Title or organization role: + Title or organization role: {% if member and member.title %} {{ member.title }} {% else %} From 5216cb4a53ca9e8dc4e4b1aeaed01c60f12ff4ac Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 17:15:28 -0500 Subject: [PATCH 066/226] wip --- src/registrar/templates/portfolio_members_add_new.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/portfolio_members_add_new.html b/src/registrar/templates/portfolio_members_add_new.html index c3a648bdc..715ad53ab 100644 --- a/src/registrar/templates/portfolio_members_add_new.html +++ b/src/registrar/templates/portfolio_members_add_new.html @@ -67,7 +67,7 @@ {% include "includes/member_basic_permissions.html" %} -

Domain management

+

Domain assignments

After you invite this person to your organization, you can assign domain management permissions on their member profile.

From e031d04fa5cdb9559a318dcce806b412b35ced6a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 17:23:18 -0500 Subject: [PATCH 067/226] lint --- src/registrar/tests/test_views_portfolio.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index c00892eca..e45f65f6f 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -951,9 +951,7 @@ class TestPortfolio(WebTest): self.assertContains(response, "Admin") self.assertContains(response, "Creator") self.assertContains(response, "Manager") - self.assertContains( - response, 'This member does not manage any domains.' - ) + self.assertContains(response, "This member does not manage any domains.") # Assert buttons and links within the page are correct self.assertContains(response, "wrapper-delete-action") # test that 3 dot is present @@ -1066,9 +1064,7 @@ class TestPortfolio(WebTest): self.assertContains(response, "Viewer") self.assertContains(response, "Creator") self.assertContains(response, "Manager") - self.assertContains( - response, 'This member does not manage any domains.' - ) + self.assertContains(response, "This member does not manage any domains.") # Assert buttons and links within the page are correct self.assertContains(response, "wrapper-delete-action") # test that 3 dot is present self.assertContains(response, "sprite.svg#edit") # test that Edit link is present From 819fd35e591f2db7af19191e792d0c73e1a9780e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 18:04:17 -0500 Subject: [PATCH 068/226] remove last active on member page --- src/registrar/templates/portfolio_member.html | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/registrar/templates/portfolio_member.html b/src/registrar/templates/portfolio_member.html index b270e03fb..b4b54291b 100644 --- a/src/registrar/templates/portfolio_member.html +++ b/src/registrar/templates/portfolio_member.html @@ -65,16 +65,6 @@ Organization member
{% csrf_token %}
- Last active: - {% if member and member.last_login %} - {{ member.last_login }} - {% elif portfolio_invitation %} - Invited - {% else %} - ⎯ - {% endif %} -
- Full name: {% if member %} {% if member.first_name or member.last_name %} From aa253a85dad9602c82e84786a4b3efb2c560c4d1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 10 Feb 2025 18:09:13 -0500 Subject: [PATCH 069/226] wip --- src/registrar/assets/src/js/getgov/table-members.js | 2 +- src/registrar/tests/test_views_portfolio.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/src/js/getgov/table-members.js b/src/registrar/assets/src/js/getgov/table-members.js index e876ce0f9..bb2d8c185 100644 --- a/src/registrar/assets/src/js/getgov/table-members.js +++ b/src/registrar/assets/src/js/getgov/table-members.js @@ -108,7 +108,7 @@ export class MembersTable extends BaseTable { `; - showMoreRow.innerHTML = `
`; + showMoreRow.innerHTML = ``; showMoreRow.classList.add('show-more-content'); showMoreRow.classList.add('display-none'); showMoreRow.id = unique_id; diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index e45f65f6f..052155bb0 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -956,7 +956,7 @@ class TestPortfolio(WebTest): # Assert buttons and links within the page are correct self.assertContains(response, "wrapper-delete-action") # test that 3 dot is present self.assertContains(response, "sprite.svg#edit") # test that Edit link is present - self.assertContains(response, "sprite.svg#settings") # test that Manage link is present + self.assertContains(response, "sprite.svg#edit") # test that Manage link is present self.assertNotContains(response, "sprite.svg#visibility") # test that View link is not present @less_console_noise_decorator @@ -1068,7 +1068,7 @@ class TestPortfolio(WebTest): # Assert buttons and links within the page are correct self.assertContains(response, "wrapper-delete-action") # test that 3 dot is present self.assertContains(response, "sprite.svg#edit") # test that Edit link is present - self.assertContains(response, "sprite.svg#settings") # test that Manage link is present + self.assertContains(response, "sprite.svg#edit") # test that Manage link is present self.assertNotContains(response, "sprite.svg#visibility") # test that View link is not present @less_console_noise_decorator From d84aa421d90ba2d3a26bb998f428f3643ecf1841 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 10 Feb 2025 19:23:50 -0500 Subject: [PATCH 070/226] wip --- src/djangooidc/views.py | 2 + src/registrar/config/settings.py | 2 + src/registrar/decorators.py | 72 ++++++++++++++++++++ src/registrar/registrar_middleware.py | 39 +++++++++++ src/registrar/utility/domain_cache_helper.py | 0 src/registrar/views/domain_requests_json.py | 3 +- src/registrar/views/domains_json.py | 3 +- src/registrar/views/index.py | 2 + 8 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 src/registrar/decorators.py create mode 100644 src/registrar/utility/domain_cache_helper.py diff --git a/src/djangooidc/views.py b/src/djangooidc/views.py index 815df4ecf..984936a4c 100644 --- a/src/djangooidc/views.py +++ b/src/djangooidc/views.py @@ -5,12 +5,14 @@ import logging from django.conf import settings from django.contrib.auth import logout as auth_logout from django.contrib.auth import authenticate, login +from login_required import login_not_required from django.http import HttpResponseRedirect from django.shortcuts import redirect from urllib.parse import parse_qs, urlencode from djangooidc.oidc import Client from djangooidc import exceptions as o_e +from registrar.decorators import grant_access from registrar.models import User from registrar.views.utility.error_views import custom_500_error_view, custom_401_error_view diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 78439188e..9b51383f3 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -200,6 +200,8 @@ MIDDLEWARE = [ "waffle.middleware.WaffleMiddleware", "registrar.registrar_middleware.CheckUserProfileMiddleware", "registrar.registrar_middleware.CheckPortfolioMiddleware", + # Restrict access using Opt-Out approach + "registrar.registrar_middleware.RestrictAccessMiddleware", ] # application object used by Django's built-in servers (e.g. `runserver`) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py new file mode 100644 index 000000000..08214f89c --- /dev/null +++ b/src/registrar/decorators.py @@ -0,0 +1,72 @@ +from functools import wraps +from django.http import JsonResponse +from django.core.exceptions import ObjectDoesNotExist + +from registrar.models.domain import Domain +from registrar.models.user_domain_role import UserDomainRole + +# Constants for clarity +ALL = "all" +IS_SUPERUSER = "is_superuser" +IS_STAFF = "is_staff" +IS_DOMAIN_MANAGER = "is_domain_manager" + +def grant_access(*rules): + """ + Allows multiple rules in a single decorator call: + @grant_access(IS_STAFF, IS_SUPERUSER, IS_DOMAIN_MANAGER) + or multiple stacked decorators: + @grant_access(IS_SUPERUSER) + @grant_access(IS_DOMAIN_MANAGER) + """ + + def decorator(view_func): + view_func.has_explicit_access = True # Mark as explicitly access-controlled + existing_rules = getattr(view_func, "_access_rules", set()) + existing_rules.update(rules) # Support multiple rules in one call + view_func._access_rules = existing_rules # Store rules on the function + + @wraps(view_func) + def wrapper(request, *args, **kwargs): + user = request.user + + # Skip authentication if @login_not_required is applied + if getattr(view_func, "login_not_required", False): + return view_func(request, *args, **kwargs) + + # Allow everyone if `ALL` is in rules + if ALL in view_func._access_rules: + return view_func(request, *args, **kwargs) + + # Ensure user is authenticated + if not user.is_authenticated: + return JsonResponse({"error": "Authentication required"}, status=403) + + conditions_met = [] + + if IS_STAFF in view_func._access_rules: + conditions_met.append(user.is_staff) + + if not any(conditions_met) and IS_SUPERUSER in view_func._access_rules: + conditions_met.append(user.is_superuser) + + if not any(conditions_met) and IS_DOMAIN_MANAGER in view_func._access_rules: + domain_id = kwargs.get('pk') or kwargs.get('domain_id') + if not domain_id: + return JsonResponse({"error": "Domain ID missing"}, status=400) + try: + domain = Domain.objects.get(pk=domain_id) + has_permission = UserDomainRole.objects.filter( + user=user, domain=domain + ).exists() + conditions_met.append(has_permission) + except ObjectDoesNotExist: + return JsonResponse({"error": "Invalid Domain"}, status=404) + + if not any(conditions_met): + return JsonResponse({"error": "Access Denied"}, status=403) + + return view_func(request, *args, **kwargs) + + return wrapper + return decorator diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index ed7a4dffc..c23fb0515 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -3,9 +3,13 @@ Contains middleware used in settings.py """ import logging +import re from urllib.parse import parse_qs +from django.conf import settings from django.urls import reverse from django.http import HttpResponseRedirect +from django.http import JsonResponse +from django.urls import resolve from registrar.models import User from waffle.decorators import flag_is_active @@ -170,3 +174,38 @@ class CheckPortfolioMiddleware: request.session["portfolio"] = request.user.get_first_portfolio() else: request.session["portfolio"] = request.user.get_first_portfolio() + + +class RestrictAccessMiddleware: + """ Middleware that blocks all views unless explicitly permitted """ + + def __init__(self, get_response): + self.get_response = get_response + self.ignored_paths = [re.compile(pattern) for pattern in getattr(settings, "LOGIN_REQUIRED_IGNORE_PATHS", [])] + + def __call__(self, request): + # Allow requests that match LOGIN_REQUIRED_IGNORE_PATHS + if any(pattern.match(request.path) for pattern in self.ignored_paths): + return self.get_response(request) + + # Try to resolve the view function + try: + resolver_match = resolve(request.path_info) + view_func = resolver_match.func + app_name = resolver_match.app_name # Get app name of resolved view + except Exception: + return JsonResponse({"error": "Not Found"}, status=404) + + # Auto-allow Django's built-in admin views (but NOT custom /admin/* views) + if app_name == "admin": + return self.get_response(request) + + # Skip access restriction if the view explicitly allows unauthenticated access + if getattr(view_func, "login_required", True) is False: + return self.get_response(request) + + # Enforce explicit access fules for other views + if not getattr(view_func, "has_explicit_access", False): + return JsonResponse({"error": "Access Denied"}, status=403) + + return self.get_response(request) \ No newline at end of file diff --git a/src/registrar/utility/domain_cache_helper.py b/src/registrar/utility/domain_cache_helper.py new file mode 100644 index 000000000..e69de29bb diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py index 88590010e..b6a9d1072 100644 --- a/src/registrar/views/domain_requests_json.py +++ b/src/registrar/views/domain_requests_json.py @@ -1,5 +1,6 @@ from django.http import JsonResponse from django.core.paginator import Paginator +from registrar.decorators import grant_access, ALL from registrar.models import DomainRequest from django.utils.dateformat import format from django.contrib.auth.decorators import login_required @@ -7,7 +8,7 @@ from django.urls import reverse from django.db.models import Q -@login_required +@grant_access(ALL) def get_domain_requests_json(request): """Given the current request, get all domain requests that are associated with the request user and exclude the APPROVED ones. diff --git a/src/registrar/views/domains_json.py b/src/registrar/views/domains_json.py index 8734ef89c..e0081450d 100644 --- a/src/registrar/views/domains_json.py +++ b/src/registrar/views/domains_json.py @@ -1,6 +1,7 @@ import logging from django.http import JsonResponse from django.core.paginator import Paginator +from registrar.decorators import grant_access, ALL from registrar.models import UserDomainRole, Domain, DomainInformation, User from django.contrib.auth.decorators import login_required from django.urls import reverse @@ -9,7 +10,7 @@ from django.db.models import Q logger = logging.getLogger(__name__) -@login_required +@grant_access(ALL) def get_domains_json(request): """Given the current request, get all domains that are associated with the UserDomainRole object""" diff --git a/src/registrar/views/index.py b/src/registrar/views/index.py index be7149018..d9ff9b209 100644 --- a/src/registrar/views/index.py +++ b/src/registrar/views/index.py @@ -1,6 +1,8 @@ from django.shortcuts import render +from registrar.decorators import grant_access, ALL +@grant_access(ALL) def index(request): """This page is available to anyone without logging in.""" context = {} From a334f7dc3ef02c2aaa2a8bdb7f9fa43d5ac9e60d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 11 Feb 2025 10:06:24 -0500 Subject: [PATCH 071/226] wip --- src/registrar/config/urls.py | 29 +-- src/registrar/decorators.py | 176 +++++++++++++----- src/registrar/registrar_middleware.py | 8 +- src/registrar/templates/domain_add_user.html | 6 +- src/registrar/templates/domain_detail.html | 20 +- src/registrar/templates/domain_dns.html | 8 +- src/registrar/templates/domain_dnssec.html | 6 +- src/registrar/templates/domain_dsdata.html | 6 +- .../templates/domain_nameservers.html | 4 +- src/registrar/templates/domain_renewal.html | 8 +- .../templates/domain_security_email.html | 2 +- src/registrar/templates/domain_sidebar.html | 8 +- .../templates/domain_suborganization.html | 2 +- src/registrar/templates/domain_users.html | 8 +- src/registrar/views/domain.py | 41 ++-- src/registrar/views/domains_json.py | 2 +- src/registrar/views/utility/error_views.py | 8 + src/registrar/views/utility/mixins.py | 4 +- .../views/utility/permission_views.py | 1 + 19 files changed, 216 insertions(+), 131 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index eb095c5ca..27ad01dc8 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -296,56 +296,56 @@ urlpatterns = [ lambda r: always_404(r, "We forgot to include this link, sorry."), name="todo", ), - path("domain/", views.DomainView.as_view(), name="domain"), - path("domain//prototype-dns", views.PrototypeDomainDNSRecordView.as_view(), name="prototype-domain-dns"), - path("domain//users", views.DomainUsersView.as_view(), name="domain-users"), + path("domain/", views.DomainView.as_view(), name="domain"), + path("domain//prototype-dns", views.PrototypeDomainDNSRecordView.as_view(), name="prototype-domain-dns"), + path("domain//users", views.DomainUsersView.as_view(), name="domain-users"), path( - "domain//dns", + "domain//dns", views.DomainDNSView.as_view(), name="domain-dns", ), path( - "domain//dns/nameservers", + "domain//dns/nameservers", views.DomainNameserversView.as_view(), name="domain-dns-nameservers", ), path( - "domain//dns/dnssec", + "domain//dns/dnssec", views.DomainDNSSECView.as_view(), name="domain-dns-dnssec", ), path( - "domain//dns/dnssec/dsdata", + "domain//dns/dnssec/dsdata", views.DomainDsDataView.as_view(), name="domain-dns-dnssec-dsdata", ), path( - "domain//org-name-address", + "domain//org-name-address", views.DomainOrgNameAddressView.as_view(), name="domain-org-name-address", ), path( - "domain//suborganization", + "domain//suborganization", views.DomainSubOrganizationView.as_view(), name="domain-suborganization", ), path( - "domain//senior-official", + "domain//senior-official", views.DomainSeniorOfficialView.as_view(), name="domain-senior-official", ), path( - "domain//security-email", + "domain//security-email", views.DomainSecurityEmailView.as_view(), name="domain-security-email", ), path( - "domain//renewal", + "domain//renewal", views.DomainRenewalView.as_view(), name="domain-renewal", ), path( - "domain//users/add", + "domain//users/add", views.DomainAddUserView.as_view(), name="domain-users-add", ), @@ -370,7 +370,7 @@ urlpatterns = [ name="domain-request-delete", ), path( - "domain//users//delete", + "domain//users//delete", views.DomainDeleteUserView.as_view(http_method_names=["post"]), name="domain-user-delete", ), @@ -392,6 +392,7 @@ urlpatterns = [ # This way, we can share a view for djangooidc, and other pages as we see fit. handler500 = "registrar.views.utility.error_views.custom_500_error_view" handler403 = "registrar.views.utility.error_views.custom_403_error_view" +handler404 = "registrar.views.utility.error_views.custom_404_error_view" # we normally would guard these with `if settings.DEBUG` but tests run with # DEBUG = False even when these apps have been loaded because settings.DEBUG diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 08214f89c..177dfab62 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -1,15 +1,14 @@ -from functools import wraps -from django.http import JsonResponse -from django.core.exceptions import ObjectDoesNotExist - -from registrar.models.domain import Domain -from registrar.models.user_domain_role import UserDomainRole +import functools +from django.core.exceptions import PermissionDenied +from django.utils.decorators import method_decorator +from registrar.models import DomainInformation, DomainRequest, UserDomainRole # Constants for clarity ALL = "all" IS_SUPERUSER = "is_superuser" IS_STAFF = "is_staff" IS_DOMAIN_MANAGER = "is_domain_manager" +IS_STAFF_MANAGING_DOMAIN = "is_staff_managing_domain" def grant_access(*rules): """ @@ -20,53 +19,128 @@ def grant_access(*rules): @grant_access(IS_DOMAIN_MANAGER) """ - def decorator(view_func): - view_func.has_explicit_access = True # Mark as explicitly access-controlled - existing_rules = getattr(view_func, "_access_rules", set()) - existing_rules.update(rules) # Support multiple rules in one call - view_func._access_rules = existing_rules # Store rules on the function + def decorator(view): + if isinstance(view, type): # If decorating a class-based view (CBV) + original_dispatch = view.dispatch # save original dispatch method - @wraps(view_func) - def wrapper(request, *args, **kwargs): - user = request.user - - # Skip authentication if @login_not_required is applied - if getattr(view_func, "login_not_required", False): - return view_func(request, *args, **kwargs) - - # Allow everyone if `ALL` is in rules - if ALL in view_func._access_rules: - return view_func(request, *args, **kwargs) + @method_decorator(grant_access(*rules)) # apply the decorator to dispatch + def wrapped_dispatch(self, request, *args, **kwargs): + if not _user_has_permission(request.user, request, rules, **kwargs): + raise PermissionDenied + return original_dispatch(self, request, *args, **kwargs) - # Ensure user is authenticated - if not user.is_authenticated: - return JsonResponse({"error": "Authentication required"}, status=403) + view.dispatch = wrapped_dispatch # replace dispatch with wrapped version + return view - conditions_met = [] + else: # If decorating a function-based view (FBV) + view.has_explicit_access = True + existing_rules = getattr(view, "_access_rules", set()) + existing_rules.update(rules) + view._access_rules = existing_rules - if IS_STAFF in view_func._access_rules: - conditions_met.append(user.is_staff) - - if not any(conditions_met) and IS_SUPERUSER in view_func._access_rules: - conditions_met.append(user.is_superuser) - - if not any(conditions_met) and IS_DOMAIN_MANAGER in view_func._access_rules: - domain_id = kwargs.get('pk') or kwargs.get('domain_id') - if not domain_id: - return JsonResponse({"error": "Domain ID missing"}, status=400) - try: - domain = Domain.objects.get(pk=domain_id) - has_permission = UserDomainRole.objects.filter( - user=user, domain=domain - ).exists() - conditions_met.append(has_permission) - except ObjectDoesNotExist: - return JsonResponse({"error": "Invalid Domain"}, status=404) - - if not any(conditions_met): - return JsonResponse({"error": "Access Denied"}, status=403) - - return view_func(request, *args, **kwargs) - - return wrapper + @functools.wraps(view) + def wrapper(request, *args, **kwargs): + if not _user_has_permission(request.user, request, rules, **kwargs): + raise PermissionDenied + return view(request, *args, **kwargs) + + return wrapper + return decorator + + +def _user_has_permission(user, request, rules, **kwargs): + """ + Checks if the user meets the permission requirements. + """ + + # Skip authentication if @login_not_required is applied + if getattr(request, "login_not_required", False): + return True + + # Allow everyone if `ALL` is in rules + if ALL in rules: + return True + + # Ensure user is authenticated + if not user.is_authenticated: + return False + + conditions_met = [] + + if IS_STAFF in rules: + conditions_met.append(user.is_staff) + + if not any(conditions_met) and IS_SUPERUSER in rules: + conditions_met.append(user.is_superuser) + + if not any(conditions_met) and IS_DOMAIN_MANAGER in rules: + domain_id = kwargs.get('domain_pk') + # Check UserDomainRole directly instead of fetching Domain + has_permission = UserDomainRole.objects.filter(user=user, domain_id=domain_id).exists() + conditions_met.append(has_permission) + + if not any(conditions_met) and IS_STAFF_MANAGING_DOMAIN in rules: + domain_id = kwargs.get('domain_pk') + has_permission = _can_access_other_user_domains(request, domain_id) + conditions_met.append(has_permission) + + return any(conditions_met) + + +def _can_access_other_user_domains(request, domain_pk): + """Checks to see if an authorized user (staff or superuser) + can access a domain that they did not create or were invited to. + """ + + # Check if the request user is permissioned... + user_is_analyst_or_superuser = request.user.has_perm( + "registrar.analyst_access_permission" + ) or request.user.has_perm("registrar.full_access_permission") + + if not user_is_analyst_or_superuser: + return False + + # Check if the user is attempting a valid edit action. + # In other words, if the analyst/admin did not click + # the 'Manage Domain' button in /admin, + # then they cannot access this page. + session = request.session + can_do_action = ( + "analyst_action" in session + and "analyst_action_location" in session + and session["analyst_action_location"] == domain_pk + ) + + if not can_do_action: + return False + + # Analysts may manage domains, when they are in these statuses: + valid_domain_statuses = [ + DomainRequest.DomainRequestStatus.APPROVED, + DomainRequest.DomainRequestStatus.IN_REVIEW, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.DomainRequestStatus.ACTION_NEEDED, + # Edge case - some domains do not have + # a status or DomainInformation... aka a status of 'None'. + # It is necessary to access those to correct errors. + None, + ] + + requested_domain = DomainInformation.objects.filter(domain_id=domain_pk).first() + + # if no domain information or domain request exist, the user + # should be able to manage the domain; however, if domain information + # and domain request exist, and domain request is not in valid status, + # user should not be able to manage domain + if ( + requested_domain + and requested_domain.domain_request + and requested_domain.domain_request.status not in valid_domain_statuses + ): + return False + + # Valid session keys exist, + # the user is permissioned, + # and it is in a valid status + return True diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index c23fb0515..e3dcbe788 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -6,9 +6,9 @@ import logging import re from urllib.parse import parse_qs from django.conf import settings +from django.core.exceptions import PermissionDenied from django.urls import reverse from django.http import HttpResponseRedirect -from django.http import JsonResponse from django.urls import resolve from registrar.models import User from waffle.decorators import flag_is_active @@ -182,7 +182,7 @@ class RestrictAccessMiddleware: def __init__(self, get_response): self.get_response = get_response self.ignored_paths = [re.compile(pattern) for pattern in getattr(settings, "LOGIN_REQUIRED_IGNORE_PATHS", [])] - + def __call__(self, request): # Allow requests that match LOGIN_REQUIRED_IGNORE_PATHS if any(pattern.match(request.path) for pattern in self.ignored_paths): @@ -194,7 +194,7 @@ class RestrictAccessMiddleware: view_func = resolver_match.func app_name = resolver_match.app_name # Get app name of resolved view except Exception: - return JsonResponse({"error": "Not Found"}, status=404) + return self.get_response(request) # Auto-allow Django's built-in admin views (but NOT custom /admin/* views) if app_name == "admin": @@ -206,6 +206,6 @@ class RestrictAccessMiddleware: # Enforce explicit access fules for other views if not getattr(view_func, "has_explicit_access", False): - return JsonResponse({"error": "Access Denied"}, status=403) + raise PermissionDenied return self.get_response(request) \ No newline at end of file diff --git a/src/registrar/templates/domain_add_user.html b/src/registrar/templates/domain_add_user.html index 04565f61e..abc549a82 100644 --- a/src/registrar/templates/domain_add_user.html +++ b/src/registrar/templates/domain_add_user.html @@ -16,10 +16,10 @@ Domains
  • - {{ domain.name }} + {{ domain.name }}
  • - Domain managers + Domain managers
  • Add a domain manager @@ -27,7 +27,7 @@ {% else %} - {% url 'domain-users' pk=domain.id as url %} + {% url 'domain-users' domain_pk=domain.id as url %} {% elif steps.prev %} - + Previous step diff --git a/src/registrar/templates/domain_request_sidebar.html b/src/registrar/templates/domain_request_sidebar.html index 1af54bb24..e7a0a307c 100644 --- a/src/registrar/templates/domain_request_sidebar.html +++ b/src/registrar/templates/domain_request_sidebar.html @@ -15,7 +15,7 @@ {% endif %} {% endif %} - If you withdraw your request, we won't review it. Once you withdraw your request, you can edit it and submit it again.

    -

    Withdraw request - Cancel

    +

    Withdraw request + Cancel

    diff --git a/src/registrar/templates/includes/portfolio_request_review_steps.html b/src/registrar/templates/includes/portfolio_request_review_steps.html index 89c8e652a..53ad36a3f 100644 --- a/src/registrar/templates/includes/portfolio_request_review_steps.html +++ b/src/registrar/templates/includes/portfolio_request_review_steps.html @@ -4,7 +4,7 @@ {% for step in steps %}
    {% if is_editable %} - {% namespaced_url 'domain-request' step id=domain_request_id as domain_request_url %} + {% namespaced_url 'domain-request' step domain_request_pk=domain_request_id as domain_request_url %} {% endif %} {% if step == Step.REQUESTING_ENTITY %} diff --git a/src/registrar/templates/includes/request_review_steps.html b/src/registrar/templates/includes/request_review_steps.html index f1b13f890..5e00c2160 100644 --- a/src/registrar/templates/includes/request_review_steps.html +++ b/src/registrar/templates/includes/request_review_steps.html @@ -4,7 +4,7 @@ {% for step in steps %}
    {% if is_editable %} - {% namespaced_url 'domain-request' step id=domain_request_id as domain_request_url %} + {% namespaced_url 'domain-request' step domain_request_pk=domain_request_id as domain_request_url %} {% endif %} {% if step == Step.ORGANIZATION_TYPE %} diff --git a/src/registrar/templates/includes/request_status_manage.html b/src/registrar/templates/includes/request_status_manage.html index d96adedf6..f616522a7 100644 --- a/src/registrar/templates/includes/request_status_manage.html +++ b/src/registrar/templates/includes/request_status_manage.html @@ -114,7 +114,7 @@ {% block modify_request %} {% if DomainRequest.is_withdrawable %} -

    +

    Withdraw request

    {% endif %} diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 3248c1368..bcefbbc77 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -5,28 +5,27 @@ from django.shortcuts import redirect, render from django.urls import resolve, reverse from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ -from django.views.generic import TemplateView +from django.views.generic import DeleteView, DetailView, TemplateView from django.contrib import messages +from registrar.decorators import ( + HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT, + HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL, + IS_DOMAIN_REQUEST_CREATOR, + grant_access, +) from registrar.forms import domain_request_wizard as forms from registrar.forms.utility.wizard_form_helper import request_step_list from registrar.models import DomainRequest from registrar.models.contact import Contact from registrar.models.user import User from registrar.views.utility import StepsHelper -from registrar.views.utility.permission_views import DomainRequestPermissionDeleteView from registrar.utility.enums import Step, PortfolioDomainRequestStep -from .utility import ( - DomainRequestPermissionView, - DomainRequestPermissionWithdrawView, - DomainRequestWizardPermissionView, - DomainRequestPortfolioViewonlyView, -) - logger = logging.getLogger(__name__) -class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): +@grant_access(IS_DOMAIN_REQUEST_CREATOR, HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT) +class DomainRequestWizard(TemplateView): """ A common set of methods and configuration. @@ -51,7 +50,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): # NB: this is included here for reference. Do not change it without # also changing the many places it is hardcoded in the HTML templates URL_NAMESPACE = "domain-request" - # name for accessing /domain-request//edit + # name for accessing /domain-request//edit EDIT_URL_NAME = "edit-domain-request" NEW_URL_NAME = "start" FINISHED_URL_NAME = "finished" @@ -174,7 +173,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): def has_pk(self): """Does this wizard know about a DomainRequest database record?""" - return bool(self.kwargs.get("id") is not None) + return bool(self.kwargs.get("domain_request_pk") is not None) def get_step_enum(self): """Determines which step enum we should use for the wizard""" @@ -209,11 +208,11 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): try: self._domain_request = DomainRequest.objects.get( creator=creator, - pk=self.kwargs.get("id"), + pk=self.kwargs.get("domain_request_pk"), ) return self._domain_request except DomainRequest.DoesNotExist: - logger.debug("DomainRequest id %s did not have a DomainRequest" % id) + logger.debug("DomainRequest id %s did not have a DomainRequest" % self.kwargs.get("domain_request_pk")) # If a user is creating a request, we assume that perms are handled upstream if self.request.user.is_org_user(self.request): @@ -292,10 +291,10 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): current_url = resolve(request.path_info).url_name - # if user visited via an "edit" url, associate the id of the + # if user visited via an "edit" url, associate the pk of the # domain request they are trying to edit to this wizard instance # and remove any prior wizard data from their session - if current_url == self.EDIT_URL_NAME and "id" in kwargs: + if current_url == self.EDIT_URL_NAME and "domain_request_pk" in kwargs: del self.storage # if accessing this class directly, redirect to either to an acknowledgement @@ -474,7 +473,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): def goto(self, step): self.steps.current = step - return redirect(reverse(f"{self.URL_NAMESPACE}:{step}", kwargs={"id": self.domain_request.id})) + return redirect(reverse(f"{self.URL_NAMESPACE}:{step}", kwargs={"domain_request_pk": self.domain_request.id})) def goto_next_step(self): """Redirects to the next step.""" @@ -823,23 +822,12 @@ class Finished(DomainRequestWizard): return render(self.request, self.template_name, context) -class DomainRequestStatus(DomainRequestPermissionView): +@grant_access(IS_DOMAIN_REQUEST_CREATOR, HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT) +class DomainRequestStatus(DetailView): template_name = "domain_request_status.html" - - def has_permission(self): - """ - Override of the base has_permission class to account for portfolio permissions - """ - has_base_perms = super().has_permission() - if not has_base_perms: - return False - - if self.request.user.is_org_user(self.request): - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_edit_request_portfolio_permission(portfolio): - return False - - return True + model = DomainRequest + pk_url_kwarg = "domain_request_pk" + context_object_name = "DomainRequest" def get_context_data(self, **kwargs): """Context override to add a step list to the context""" @@ -854,19 +842,27 @@ class DomainRequestStatus(DomainRequestPermissionView): return context -class DomainRequestWithdrawConfirmation(DomainRequestPermissionWithdrawView): +@grant_access(IS_DOMAIN_REQUEST_CREATOR) +class DomainRequestWithdrawConfirmation(DetailView): """This page will ask user to confirm if they want to withdraw - The DomainRequestPermissionView restricts access so that only the + Access is restricted so that only the `creator` of the domain request may withdraw it. """ - template_name = "domain_request_withdraw_confirmation.html" + template_name = "domain_request_withdraw_confirmation.html" # DetailView property for what model this is viewing + model = DomainRequest + pk_url_kwarg = "domain_request_pk" + context_object_name = "DomainRequest" -class DomainRequestWithdrawn(DomainRequestPermissionWithdrawView): +@grant_access(IS_DOMAIN_REQUEST_CREATOR) +class DomainRequestWithdrawn(DetailView): # this view renders no template template_name = "" + model = DomainRequest + pk_url_kwarg = "domain_request_pk" + context_object_name = "DomainRequest" def get(self, *args, **kwargs): """View class that does the actual withdrawing. @@ -874,7 +870,7 @@ class DomainRequestWithdrawn(DomainRequestPermissionWithdrawView): If user click on withdraw confirm button, this view updates the status to withdraw and send back to homepage. """ - domain_request = DomainRequest.objects.get(id=self.kwargs["pk"]) + domain_request = DomainRequest.objects.get(id=self.kwargs["domain_request_pk"]) domain_request.withdraw() domain_request.save() if self.request.user.is_org_user(self.request): @@ -883,28 +879,22 @@ class DomainRequestWithdrawn(DomainRequestPermissionWithdrawView): return HttpResponseRedirect(reverse("home")) -class DomainRequestDeleteView(DomainRequestPermissionDeleteView): +@grant_access(IS_DOMAIN_REQUEST_CREATOR, HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT) +class DomainRequestDeleteView(DeleteView): """Delete view for home that allows the end user to delete DomainRequests""" object: DomainRequest # workaround for type mismatch in DeleteView + model = DomainRequest + pk_url_kwarg = "domain_request_pk" def has_permission(self): """Custom override for has_permission to exclude all statuses, except WITHDRAWN and STARTED""" - has_perm = super().has_permission() - if not has_perm: - return False status = self.get_object().status valid_statuses = [DomainRequest.DomainRequestStatus.WITHDRAWN, DomainRequest.DomainRequestStatus.STARTED] if status not in valid_statuses: return False - # Portfolio users cannot delete their requests if they aren't permissioned to do so - if self.request.user.is_org_user(self.request): - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_edit_request_portfolio_permission(portfolio): - return False - return True def get_success_url(self): @@ -989,8 +979,12 @@ class DomainRequestDeleteView(DomainRequestPermissionDeleteView): # region Portfolio views -class PortfolioDomainRequestStatusViewOnly(DomainRequestPortfolioViewonlyView): +@grant_access(HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL) +class PortfolioDomainRequestStatusViewOnly(DetailView): template_name = "portfolio_domain_request_status_viewonly.html" + model = DomainRequest + pk_url_kwarg = "domain_request_pk" + context_object_name = "DomainRequest" def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) diff --git a/src/registrar/views/domain_requests_json.py b/src/registrar/views/domain_requests_json.py index b6a9d1072..4e475321c 100644 --- a/src/registrar/views/domain_requests_json.py +++ b/src/registrar/views/domain_requests_json.py @@ -155,9 +155,9 @@ def serialize_domain_request(request, domain_request, user): # Map the action label to corresponding URLs and icons action_url_map = { - "Edit": reverse("edit-domain-request", kwargs={"id": domain_request.id}), - "Manage": reverse("domain-request-status", kwargs={"pk": domain_request.id}), - "View": reverse("domain-request-status-viewonly", kwargs={"pk": domain_request.id}), + "Edit": reverse("edit-domain-request", kwargs={"domain_request_pk": domain_request.id}), + "Manage": reverse("domain-request-status", kwargs={"domain_request_pk": domain_request.id}), + "View": reverse("domain-request-status-viewonly", kwargs={"domain_request_pk": domain_request.id}), } svg_icon_map = {"Edit": "edit", "Manage": "settings", "View": "visibility"} diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 0f93ec8e1..9f864324f 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -6,6 +6,7 @@ from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils.safestring import mark_safe from django.contrib import messages +from registrar.decorators import HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM, grant_access from registrar.forms import portfolio as portfolioForms from registrar.models import Portfolio, User from registrar.models.domain import Domain @@ -25,7 +26,6 @@ from registrar.utility.errors import MissingEmailError from registrar.utility.enums import DefaultUserValues from registrar.views.utility.mixins import PortfolioMemberPermission from registrar.views.utility.permission_views import ( - PortfolioDomainRequestsPermissionView, PortfolioDomainsPermissionView, PortfolioBasePermissionView, NoPortfolioDomainsPermissionView, @@ -58,7 +58,8 @@ class PortfolioDomainsView(PortfolioDomainsPermissionView, View): return render(request, "portfolio_domains.html", context) -class PortfolioDomainRequestsView(PortfolioDomainRequestsPermissionView, View): +@grant_access(HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM) +class PortfolioDomainRequestsView(View): template_name = "portfolio_requests.html" diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py index 6798eb4ee..4a33c9944 100644 --- a/src/registrar/views/utility/__init__.py +++ b/src/registrar/views/utility/__init__.py @@ -3,11 +3,7 @@ from .always_404 import always_404 from .permission_views import ( DomainPermissionView, - DomainRequestPermissionView, - DomainRequestPermissionWithdrawView, - DomainRequestWizardPermissionView, PortfolioMembersPermission, - DomainRequestPortfolioViewonlyView, DomainInvitationPermissionCancelView, ) from .api_views import get_senior_official_from_federal_agency_json diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 23bcff162..682d9ffcb 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -286,51 +286,6 @@ class DomainPermission(PermissionsLoginMixin): return True -class DomainRequestPermission(PermissionsLoginMixin): - """Permission mixin that redirects to domain request if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to this domain request. - - The user is in self.request.user and the domain needs to be looked - up from the domain's primary key in self.kwargs["pk"] - """ - if not self.request.user.is_authenticated: - return False - - # user needs to be the creator of the domain request - # this query is empty if there isn't a domain request with this - # id and this user as creator - if not DomainRequest.objects.filter(creator=self.request.user, id=self.kwargs["pk"]).exists(): - return False - - return True - - -class DomainRequestPortfolioViewonlyPermission(PermissionsLoginMixin): - """Permission mixin that redirects to domain request if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to this domain request. - - The user is in self.request.user and the domain needs to be looked - up from the domain's primary key in self.kwargs["pk"] - """ - if not self.request.user.is_authenticated: - return False - - if not self.request.user.is_org_user(self.request): - return False - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_view_all_requests_portfolio_permission(portfolio): - return False - - return True - - class UserDeleteDomainRolePermission(PermissionsLoginMixin): """Permission mixin for UserDomainRole if user has access, otherwise 403""" @@ -365,67 +320,6 @@ class UserDeleteDomainRolePermission(PermissionsLoginMixin): return True -class DomainRequestPermissionWithdraw(PermissionsLoginMixin): - """Permission mixin that redirects to withdraw action on domain request - if user has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to withdraw this domain request.""" - if not self.request.user.is_authenticated: - return False - - # user needs to be the creator of the domain request - # this query is empty if there isn't a domain request with this - # id and this user as creator - if not DomainRequest.objects.filter(creator=self.request.user, id=self.kwargs["pk"]).exists(): - return False - - # Restricted users should not be able to withdraw domain requests - if self.request.user.is_restricted(): - return False - - return True - - -class DomainRequestWizardPermission(PermissionsLoginMixin): - """Permission mixin that redirects to start or edit domain request if - user has access, otherwise 403""" - - def has_permission(self): - """Check if this user has permission to start or edit a domain request. - - The user is in self.request.user - """ - - if not self.request.user.is_authenticated: - return False - - # The user has an ineligible flag - if self.request.user.is_restricted(): - return False - - # If the user is an org user and doesn't have add/edit perms, forbid this - if self.request.user.is_org_user(self.request): - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_edit_request_portfolio_permission(portfolio): - return False - - # user needs to be the creator of the domain request to edit it. - id = self.kwargs.get("id") if hasattr(self, "kwargs") else None - if not id: - domain_request_wizard = self.request.session.get("wizard_domain_request") - if domain_request_wizard: - id = domain_request_wizard.get("domain_request_id") - - # If no id is provided, we can assume that the user is starting a new request. - # If one IS provided, check that they are the original creator of it. - if id: - if not DomainRequest.objects.filter(creator=self.request.user, id=id).exists(): - return False - - return True - - class DomainInvitationPermission(PermissionsLoginMixin): """Permission mixin that redirects to domain invitation if user has access, otherwise 403" @@ -496,23 +390,6 @@ class PortfolioDomainsPermission(PortfolioBasePermission): return super().has_permission() -class PortfolioDomainRequestsPermission(PortfolioBasePermission): - """Permission mixin that allows access to portfolio domain request pages if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to domain requests for this portfolio. - - The user is in self.request.user and the portfolio can be looked - up from the portfolio's primary key in self.kwargs["pk"]""" - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_any_requests_portfolio_permission(portfolio): - return False - - return super().has_permission() - - class PortfolioMembersPermission(PortfolioBasePermission): """Permission mixin that allows access to portfolio members pages if user has access, otherwise 403""" diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index b430845f4..41d620d42 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -2,18 +2,14 @@ import abc # abstract base class -from django.views.generic import DetailView, DeleteView, TemplateView, UpdateView -from registrar.models import Domain, DomainRequest, DomainInvitation, Portfolio +from django.views.generic import DetailView, DeleteView, UpdateView +from registrar.models import Domain, DomainInvitation, Portfolio from registrar.models.user import User from registrar.models.user_domain_role import UserDomainRole from .mixins import ( DomainPermission, - DomainRequestPermission, - DomainRequestPermissionWithdraw, DomainInvitationPermission, - DomainRequestWizardPermission, - PortfolioDomainRequestsPermission, PortfolioDomainsPermission, PortfolioMemberDomainsPermission, PortfolioMemberDomainsEditPermission, @@ -23,7 +19,6 @@ from .mixins import ( PortfolioBasePermission, PortfolioMembersPermission, PortfolioMemberPermission, - DomainRequestPortfolioViewonlyPermission, ) import logging @@ -86,77 +81,6 @@ class DomainPermissionView(DomainPermission, DetailView, abc.ABC): raise NotImplementedError -class DomainRequestPermissionView(DomainRequestPermission, DetailView, abc.ABC): - """Abstract base view for domain requests that enforces permissions - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # DetailView property for what model this is viewing - model = DomainRequest - # variable name in template context for the model object - context_object_name = "DomainRequest" - - # Abstract property enforces NotImplementedError on an attribute. - @property - @abc.abstractmethod - def template_name(self): - raise NotImplementedError - - -class DomainRequestPortfolioViewonlyView(DomainRequestPortfolioViewonlyPermission, DetailView, abc.ABC): - """Abstract base view for domain requests that enforces permissions - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # DetailView property for what model this is viewing - model = DomainRequest - # variable name in template context for the model object - context_object_name = "DomainRequest" - - # Abstract property enforces NotImplementedError on an attribute. - @property - @abc.abstractmethod - def template_name(self): - raise NotImplementedError - - -class DomainRequestPermissionWithdrawView(DomainRequestPermissionWithdraw, DetailView, abc.ABC): - """Abstract base view for domain request withdraw function - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # DetailView property for what model this is viewing - model = DomainRequest - # variable name in template context for the model object - context_object_name = "DomainRequest" - - # Abstract property enforces NotImplementedError on an attribute. - @property - @abc.abstractmethod - def template_name(self): - raise NotImplementedError - - -class DomainRequestWizardPermissionView(DomainRequestWizardPermission, TemplateView, abc.ABC): - """Abstract base view for the domain request form that enforces permissions - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # Abstract property enforces NotImplementedError on an attribute. - @property - @abc.abstractmethod - def template_name(self): - raise NotImplementedError - - class DomainInvitationPermissionCancelView(DomainInvitationPermission, UpdateView, abc.ABC): """Abstract view for cancelling a DomainInvitation.""" @@ -164,13 +88,6 @@ class DomainInvitationPermissionCancelView(DomainInvitationPermission, UpdateVie object: DomainInvitation -class DomainRequestPermissionDeleteView(DomainRequestPermission, DeleteView, abc.ABC): - """Abstract view for deleting a DomainRequest.""" - - model = DomainRequest - object: DomainRequest - - class UserDomainRolePermissionDeleteView(UserDeleteDomainRolePermission, DeleteView, abc.ABC): """Abstract base view for deleting a UserDomainRole. @@ -242,14 +159,6 @@ class NoPortfolioDomainsPermissionView(PortfolioBasePermissionView, abc.ABC): """ -class PortfolioDomainRequestsPermissionView(PortfolioDomainRequestsPermission, PortfolioBasePermissionView, abc.ABC): - """Abstract base view for portfolio domain request views that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - class PortfolioMembersPermissionView(PortfolioMembersPermission, PortfolioBasePermissionView, abc.ABC): """Abstract base view for portfolio members views that enforces permissions. From 40737cbcf7beb4e0d1af167e0e2898c0410eb7cb Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 11 Feb 2025 23:47:55 -0500 Subject: [PATCH 090/226] wip --- src/registrar/decorators.py | 14 ++++++++++- .../includes/domain_sidenav_item.html | 2 +- src/registrar/views/domain.py | 7 ++++-- src/registrar/views/domain_request.py | 2 +- src/registrar/views/domain_requests_json.py | 20 +++++++-------- src/registrar/views/portfolios.py | 18 ++++++++----- src/registrar/views/utility/mixins.py | 17 ------------- .../views/utility/permission_views.py | 25 ++++++------------- 8 files changed, 49 insertions(+), 56 deletions(-) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index cd19bcc95..fe71342cc 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -10,8 +10,10 @@ IS_STAFF = "is_staff" IS_DOMAIN_MANAGER = "is_domain_manager" IS_DOMAIN_REQUEST_CREATOR = "is_domain_request_creator" IS_STAFF_MANAGING_DOMAIN = "is_staff_managing_domain" +IS_PORTFOLIO_MEMBER = "is_portfolio_member" IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER = "is_portfolio_member_and_domain_manager" IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER = "is_domain_manager_and_not_portfolio_member" +HAS_PORTFOLIO_DOMAINS_ANY_PERM = "has_portfolio_domains_any_perm" HAS_PORTFOLIO_DOMAINS_VIEW_ALL = "has_portfolio_domains_view_all" HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM = "has_portfolio_domain_requests_any_perm" HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL = "has_portfolio_domain_requests_view_all" @@ -97,11 +99,21 @@ def _user_has_permission(user, request, rules, **kwargs): has_permission = _can_access_other_user_domains(request, domain_id) conditions_met.append(has_permission) + if not any(conditions_met) and IS_PORTFOLIO_MEMBER in rules: + has_permission = user.is_org_user(request) + conditions_met.append(has_permission) + if not any(conditions_met) and HAS_PORTFOLIO_DOMAINS_VIEW_ALL in rules: domain_id = kwargs.get("domain_pk") has_permission = _can_access_domain_via_portfolio_view_all_domains(request, domain_id) conditions_met.append(has_permission) + if not any(conditions_met) and HAS_PORTFOLIO_DOMAINS_ANY_PERM in rules: + has_permission = user.is_org_user(request) and user.has_any_domains_portfolio_permission( + request.session.get("portfolio") + ) + conditions_met.append(has_permission) + if not any(conditions_met) and IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER in rules: domain_id = kwargs.get("domain_pk") has_permission = _is_domain_manager(user, domain_id) and _is_portfolio_member(request) @@ -142,7 +154,7 @@ def _has_portfolio_domain_requests_edit(user, request, domain_request_id): if domain_request_id and not _is_domain_request_creator(user, domain_request_id): return False return user.is_org_user(request) and user.has_edit_request_portfolio_permission(request.session.get("portfolio")) - + def _is_domain_manager(user, domain_pk): """Checks to see if the user is a domain manager of the diff --git a/src/registrar/templates/includes/domain_sidenav_item.html b/src/registrar/templates/includes/domain_sidenav_item.html index 206e49c05..715f76865 100644 --- a/src/registrar/templates/includes/domain_sidenav_item.html +++ b/src/registrar/templates/includes/domain_sidenav_item.html @@ -1,6 +1,6 @@
  • {% if url_name %} - {% url url_name pk=domain.id as url %} + {% url url_name domain_pk=domain.id as url %} {% endif %} Date: Wed, 12 Feb 2025 08:16:16 -0500 Subject: [PATCH 091/226] additional cleanup of permission views and mixins, handling of domain invitations and user domain roles in decorators --- src/registrar/config/urls.py | 2 +- src/registrar/decorators.py | 72 +++++--- src/registrar/templates/domain_users.html | 2 +- src/registrar/views/domain.py | 154 ++++++++++++++++-- src/registrar/views/utility/__init__.py | 2 - src/registrar/views/utility/mixins.py | 153 ----------------- .../views/utility/permission_views.py | 94 +---------- 7 files changed, 196 insertions(+), 283 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 665f2cd82..12efe5a9f 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -364,7 +364,7 @@ urlpatterns = [ name="user-profile", ), path( - "invitation//cancel", + "invitation//cancel", views.DomainInvitationCancelView.as_view(http_method_names=["post"]), name="invitation-cancel", ), diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index fe71342cc..237985f02 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -1,7 +1,7 @@ import functools from django.core.exceptions import PermissionDenied from django.utils.decorators import method_decorator -from registrar.models import Domain, DomainInformation, DomainRequest, UserDomainRole +from registrar.models import Domain, DomainInformation, DomainInvitation, DomainRequest, UserDomainRole # Constants for clarity ALL = "all" @@ -18,7 +18,6 @@ HAS_PORTFOLIO_DOMAINS_VIEW_ALL = "has_portfolio_domains_view_all" HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM = "has_portfolio_domain_requests_any_perm" HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL = "has_portfolio_domain_requests_view_all" HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT = "has_portfolio_domain_requests_edit" -# HAS_PORTFOLIO_DOMAINS_VIEW_MANAGED = "has_portfolio_domains_view_managed" def grant_access(*rules): @@ -90,13 +89,11 @@ def _user_has_permission(user, request, rules, **kwargs): conditions_met.append(user.is_superuser) if not any(conditions_met) and IS_DOMAIN_MANAGER in rules: - domain_id = kwargs.get("domain_pk") - has_permission = _is_domain_manager(user, domain_id) + has_permission = _is_domain_manager(user, **kwargs) conditions_met.append(has_permission) if not any(conditions_met) and IS_STAFF_MANAGING_DOMAIN in rules: - domain_id = kwargs.get("domain_pk") - has_permission = _can_access_other_user_domains(request, domain_id) + has_permission = _is_staff_managing_domain(request, **kwargs) conditions_met.append(has_permission) if not any(conditions_met) and IS_PORTFOLIO_MEMBER in rules: @@ -115,13 +112,11 @@ def _user_has_permission(user, request, rules, **kwargs): conditions_met.append(has_permission) if not any(conditions_met) and IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER in rules: - domain_id = kwargs.get("domain_pk") - has_permission = _is_domain_manager(user, domain_id) and _is_portfolio_member(request) + has_permission = _is_domain_manager(user, **kwargs) and _is_portfolio_member(request) conditions_met.append(has_permission) if not any(conditions_met) and IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER in rules: - domain_id = kwargs.get("domain_pk") - has_permission = _is_domain_manager(user, domain_id) and not _is_portfolio_member(request) + has_permission = _is_domain_manager(user, **kwargs) and not _is_portfolio_member(request) conditions_met.append(has_permission) if not any(conditions_met) and IS_DOMAIN_REQUEST_CREATOR in rules: @@ -156,10 +151,24 @@ def _has_portfolio_domain_requests_edit(user, request, domain_request_id): return user.is_org_user(request) and user.has_edit_request_portfolio_permission(request.session.get("portfolio")) -def _is_domain_manager(user, domain_pk): - """Checks to see if the user is a domain manager of the - domain with domain_pk.""" - return UserDomainRole.objects.filter(user=user, domain_id=domain_pk).exists() +def _is_domain_manager(user, **kwargs): + """ + Determines if the given user is a domain manager for a specified domain. + + - First, it checks if 'domain_pk' is present in the URL parameters. + - If 'domain_pk' exists, it verifies if the user has a domain role for that domain. + - If 'domain_pk' is absent, it checks for 'domain_invitation_pk' to determine if the user has domain permissions through an invitation. + + Returns: + bool: True if the user is a domain manager, False otherwise. + """ + domain_id = kwargs.get("domain_pk") + if domain_id: + return UserDomainRole.objects.filter(user=user, domain_id=domain_id).exists() + domain_invitation_id = kwargs.get("domain_invitation_pk") + if domain_invitation_id: + return DomainInvitation.objects.filter(id=domain_invitation_id, domain__permissions__user=user).exists() + return False def _is_domain_request_creator(user, domain_request_pk): @@ -176,10 +185,35 @@ def _is_portfolio_member(request): return request.user.is_org_user(request) -def _can_access_other_user_domains(request, domain_pk): - """Checks to see if an authorized user (staff or superuser) - can access a domain that they did not create or were invited to. +def _is_staff_managing_domain(request, **kwargs): """ + Determines whether a staff user (analyst or superuser) has permission to manage a domain + that they did not create or were not invited to. + + The function enforces: + 1. **User Authorization** - The user must have `analyst_access_permission` or `full_access_permission`. + 2. **Valid Session Context** - The user must have explicitly selected the domain for management + via an 'analyst action' (e.g., by clicking 'Manage Domain' in the admin interface). + 3. **Domain Status Check** - Only domains in specific statuses (e.g., APPROVED, IN_REVIEW, etc.) + can be managed, except in cases where the domain lacks a status due to errors. + + Process: + - First, the function retrieves the `domain_pk` from the URL parameters. + - If `domain_pk` is not provided, it attempts to resolve the domain via `domain_invitation_pk`. + - It checks if the user has the required permissions. + - It verifies that the user has an active 'analyst action' session for the domain. + - Finally, it ensures that the domain is in a status that allows management. + + Returns: + bool: True if the user is allowed to manage the domain, False otherwise. + """ + + domain_id = kwargs.get("domain_pk") + if not domain_id: + domain_invitation_id = kwargs.get("domain_invitation_pk") + domain_invitation = DomainInvitation.objects.filter(id=domain_invitation_id).first() + if domain_invitation: + domain_id = domain_invitation.domain_id # Check if the request user is permissioned... user_is_analyst_or_superuser = request.user.has_perm( @@ -197,7 +231,7 @@ def _can_access_other_user_domains(request, domain_pk): can_do_action = ( "analyst_action" in session and "analyst_action_location" in session - and session["analyst_action_location"] == domain_pk + and session["analyst_action_location"] == domain_id ) if not can_do_action: @@ -215,7 +249,7 @@ def _can_access_other_user_domains(request, domain_pk): None, ] - requested_domain = DomainInformation.objects.filter(domain_id=domain_pk).first() + requested_domain = DomainInformation.objects.filter(domain_id=domain_id).first() # if no domain information or domain request exist, the user # should be able to manage the domain; however, if domain information diff --git a/src/registrar/templates/domain_users.html b/src/registrar/templates/domain_users.html index 984b4160b..4c5dbc728 100644 --- a/src/registrar/templates/domain_users.html +++ b/src/registrar/templates/domain_users.html @@ -154,7 +154,7 @@ {% if not portfolio %}
  • {% endif %} -
    TypeStart date {{ data.start_date }}End date {{ data.end_date }} TypeStart date {{ data.start_date }}End date {{ data.end_date }}
    ${domainsHTML} ${permissionsHTML}
    ${domainsHTML} ${permissionsHTML}
    ${domainsHTML} ${permissionsHTML}
    ${domainsHTML} ${permissionsHTML}
    {{ invitation.domain_invitation.status|title }} {% if invitation.domain_invitation.status == invitation.domain_invitation.DomainInvitationStatus.INVITED %} -
    + {% csrf_token %}
    {% endif %} diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 31f715d18..7a76284a4 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -1,10 +1,3 @@ -"""Views for a single Domain. - -Authorization is handled by the `DomainPermissionView`. To ensure that only -authorized users can see information on a domain, every view here should -inherit from `DomainPermissionView` (or DomainInvitationPermissionCancelView). -""" - from datetime import date import logging import requests @@ -13,7 +6,7 @@ from django.contrib.messages.views import SuccessMessageMixin from django.http import HttpResponseRedirect from django.shortcuts import redirect, render, get_object_or_404 from django.urls import reverse -from django.views.generic import DeleteView +from django.views.generic import DeleteView, DetailView, UpdateView from django.views.generic.edit import FormMixin from django.conf import settings from registrar.decorators import ( @@ -49,7 +42,6 @@ from registrar.utility.errors import ( SecurityEmailErrorCodes, ) from registrar.models.utility.contact_error import ContactError -from registrar.views.utility.permission_views import UserDomainRolePermissionDeleteView from registrar.utility.waffle import flag_is_active_for_user from registrar.views.utility.invitation_helper import ( get_org_membership, @@ -76,19 +68,22 @@ from epplibwrapper import ( from ..utility.email import send_templated_email, EmailSendingError from ..utility.email_invitations import send_domain_invitation_email, send_portfolio_invitation_email -from .utility import DomainPermissionView, DomainInvitationPermissionCancelView from django import forms logger = logging.getLogger(__name__) -class DomainBaseView(DomainPermissionView): +class DomainBaseView(DetailView): """ Base View for the Domain. Handles getting and setting the domain in session cache on GETs. Also provides methods for getting and setting the domain in cache """ + model = Domain + pk_url_kwarg = "domain_pk" + context_object_name = "domain" + def get(self, request, *args, **kwargs): self._get_domain(request) context = self.get_context_data(object=self.object) @@ -120,6 +115,134 @@ class DomainBaseView(DomainPermissionView): domain_pk = "domain:" + str(self.kwargs.get("domain_pk")) self.session[domain_pk] = self.object + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + user = self.request.user + context["is_analyst_or_superuser"] = user.has_perm("registrar.analyst_access_permission") or user.has_perm( + "registrar.full_access_permission" + ) + context["is_domain_manager"] = UserDomainRole.objects.filter(user=user, domain=self.object).exists() + context["is_portfolio_user"] = self.can_access_domain_via_portfolio(self.object.pk) + context["is_editable"] = self.is_editable() + # Stored in a variable for the linter + action = "analyst_action" + action_location = "analyst_action_location" + # Flag to see if an analyst is attempting to make edits + if action in self.request.session: + context[action] = self.request.session[action] + if action_location in self.request.session: + context[action_location] = self.request.session[action_location] + + return context + + def is_editable(self): + """Returns whether domain is editable in the context of the view""" + domain_editable = self.object.is_editable() + if not domain_editable: + return False + + # if user is domain manager or analyst or admin, return True + if ( + self.can_access_other_user_domains(self.object.id) + or UserDomainRole.objects.filter(user=self.request.user, domain=self.object).exists() + ): + return True + + return False + + def can_access_domain_via_portfolio(self, pk): + """Most views should not allow permission to portfolio users. + If particular views allow access to the domain pages, they will need to override + this function. + """ + return False + + def has_permission(self): + """Check if this user has access to this domain. + + The user is in self.request.user and the domain needs to be looked + up from the domain's primary key in self.kwargs["domain_pk"] + """ + pk = self.kwargs["domain_pk"] + + # test if domain in editable state + if not self.in_editable_state(pk): + return False + + # if we need to check more about the nature of role, do it here. + return True + + def in_editable_state(self, pk): + """Is the domain in an editable state""" + + requested_domain = None + if Domain.objects.filter(id=pk).exists(): + requested_domain = Domain.objects.get(id=pk) + + # if domain is editable return true + if requested_domain and requested_domain.is_editable(): + return True + return False + + def can_access_other_user_domains(self, pk): + """Checks to see if an authorized user (staff or superuser) + can access a domain that they did not create or was invited to. + """ + + # Check if the user is permissioned... + user_is_analyst_or_superuser = self.request.user.has_perm( + "registrar.analyst_access_permission" + ) or self.request.user.has_perm("registrar.full_access_permission") + + if not user_is_analyst_or_superuser: + return False + + # Check if the user is attempting a valid edit action. + # In other words, if the analyst/admin did not click + # the 'Manage Domain' button in /admin, + # then they cannot access this page. + session = self.request.session + can_do_action = ( + "analyst_action" in session + and "analyst_action_location" in session + and session["analyst_action_location"] == pk + ) + + if not can_do_action: + return False + + # Analysts may manage domains, when they are in these statuses: + valid_domain_statuses = [ + DomainRequest.DomainRequestStatus.APPROVED, + DomainRequest.DomainRequestStatus.IN_REVIEW, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.DomainRequestStatus.ACTION_NEEDED, + # Edge case - some domains do not have + # a status or DomainInformation... aka a status of 'None'. + # It is necessary to access those to correct errors. + None, + ] + + requested_domain = None + if DomainInformation.objects.filter(id=pk).exists(): + requested_domain = DomainInformation.objects.get(id=pk) + + # if no domain information or domain request exist, the user + # should be able to manage the domain; however, if domain information + # and domain request exist, and domain request is not in valid status, + # user should not be able to manage domain + if ( + requested_domain + and requested_domain.domain_request + and requested_domain.domain_request.status not in valid_domain_statuses + ): + return False + + # Valid session keys exist, + # the user is permissioned, + # and it is in a valid status + return True + class DomainFormBaseView(DomainBaseView, FormMixin): """ @@ -433,7 +556,7 @@ class DomainOrgNameAddressView(DomainFormBaseView): return super().has_permission() -@grant_access(IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER) +@grant_access(IS_PORTFOLIO_MEMBER_AND_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN) class DomainSubOrganizationView(DomainFormBaseView): """Suborganization view""" @@ -480,7 +603,7 @@ class DomainSubOrganizationView(DomainFormBaseView): return super().form_valid(form) -@grant_access(IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER) +@grant_access(IS_DOMAIN_MANAGER_AND_NOT_PORTFOLIO_MEMBER, IS_STAFF_MANAGING_DOMAIN) class DomainSeniorOfficialView(DomainFormBaseView): """Domain senior official editing view.""" @@ -1307,8 +1430,9 @@ class DomainAddUserView(DomainFormBaseView): @grant_access(IS_DOMAIN_MANAGER, IS_STAFF_MANAGING_DOMAIN) -class DomainInvitationCancelView(SuccessMessageMixin, DomainInvitationPermissionCancelView): - object: DomainInvitation +class DomainInvitationCancelView(SuccessMessageMixin, UpdateView): + model = DomainInvitation + pk_url_kwarg = "domain_invitation_pk" fields = [] def post(self, request, *args, **kwargs): diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py index 4a33c9944..f80774ef3 100644 --- a/src/registrar/views/utility/__init__.py +++ b/src/registrar/views/utility/__init__.py @@ -2,8 +2,6 @@ from .steps_helper import StepsHelper from .always_404 import always_404 from .permission_views import ( - DomainPermissionView, PortfolioMembersPermission, - DomainInvitationPermissionCancelView, ) from .api_views import get_senior_official_from_federal_agency_json diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index ad0794edd..23895cb5c 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -1,14 +1,6 @@ """Permissions-related mixin classes.""" from django.contrib.auth.mixins import PermissionRequiredMixin - -from registrar.models import ( - Domain, - DomainRequest, - DomainInvitation, - DomainInformation, - UserDomainRole, -) import logging @@ -195,151 +187,6 @@ class PortfolioReportsPermission(PermissionsLoginMixin): return self.request.user.is_org_user(self.request) -class DomainPermission(PermissionsLoginMixin): - """Permission mixin that redirects to domain if user has access, - otherwise 403""" - - def has_permission(self): - """Check if this user has access to this domain. - - The user is in self.request.user and the domain needs to be looked - up from the domain's primary key in self.kwargs["domain_pk"] - """ - pk = self.kwargs["domain_pk"] - - # test if domain in editable state - if not self.in_editable_state(pk): - return False - - # if we need to check more about the nature of role, do it here. - return True - - def in_editable_state(self, pk): - """Is the domain in an editable state""" - - requested_domain = None - if Domain.objects.filter(id=pk).exists(): - requested_domain = Domain.objects.get(id=pk) - - # if domain is editable return true - if requested_domain and requested_domain.is_editable(): - return True - return False - - def can_access_other_user_domains(self, pk): - """Checks to see if an authorized user (staff or superuser) - can access a domain that they did not create or was invited to. - """ - - # Check if the user is permissioned... - user_is_analyst_or_superuser = self.request.user.has_perm( - "registrar.analyst_access_permission" - ) or self.request.user.has_perm("registrar.full_access_permission") - - if not user_is_analyst_or_superuser: - return False - - # Check if the user is attempting a valid edit action. - # In other words, if the analyst/admin did not click - # the 'Manage Domain' button in /admin, - # then they cannot access this page. - session = self.request.session - can_do_action = ( - "analyst_action" in session - and "analyst_action_location" in session - and session["analyst_action_location"] == pk - ) - - if not can_do_action: - return False - - # Analysts may manage domains, when they are in these statuses: - valid_domain_statuses = [ - DomainRequest.DomainRequestStatus.APPROVED, - DomainRequest.DomainRequestStatus.IN_REVIEW, - DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.DomainRequestStatus.ACTION_NEEDED, - # Edge case - some domains do not have - # a status or DomainInformation... aka a status of 'None'. - # It is necessary to access those to correct errors. - None, - ] - - requested_domain = None - if DomainInformation.objects.filter(id=pk).exists(): - requested_domain = DomainInformation.objects.get(id=pk) - - # if no domain information or domain request exist, the user - # should be able to manage the domain; however, if domain information - # and domain request exist, and domain request is not in valid status, - # user should not be able to manage domain - if ( - requested_domain - and requested_domain.domain_request - and requested_domain.domain_request.status not in valid_domain_statuses - ): - return False - - # Valid session keys exist, - # the user is permissioned, - # and it is in a valid status - return True - - -class UserDeleteDomainRolePermission(PermissionsLoginMixin): - """Permission mixin for UserDomainRole if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to this domain request. - - The user is in self.request.user and the domain needs to be looked - up from the domain's primary key in self.kwargs["pk"] - """ - domain_pk = self.kwargs["pk"] - user_pk = self.kwargs["user_pk"] - - if not self.request.user.is_authenticated: - return False - - # Check if the UserDomainRole object exists, then check - # if the user requesting the delete has permissions to do so - has_delete_permission = UserDomainRole.objects.filter( - user=user_pk, - domain=domain_pk, - domain__permissions__user=self.request.user, - ).exists() - - user_is_analyst_or_superuser = self.request.user.has_perm( - "registrar.analyst_access_permission" - ) or self.request.user.has_perm("registrar.full_access_permission") - - if not (has_delete_permission or user_is_analyst_or_superuser): - return False - - return True - - -class DomainInvitationPermission(PermissionsLoginMixin): - """Permission mixin that redirects to domain invitation if user has - access, otherwise 403" - - A user has access to a domain invitation if they have a role on the - associated domain. - """ - - def has_permission(self): - """Check if this user has a role on the domain of this invitation.""" - if not self.request.user.is_authenticated: - return False - - if not DomainInvitation.objects.filter( - id=self.kwargs["pk"], domain__permissions__user=self.request.user - ).exists(): - return False - return True - - class UserProfilePermission(PermissionsLoginMixin): """Permission mixin that redirects to user profile if user has access, otherwise 403""" diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index b2b56666f..02a2b029b 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -2,18 +2,14 @@ import abc # abstract base class -from django.views.generic import DetailView, DeleteView, UpdateView -from registrar.models import Domain, DomainInvitation, Portfolio +from django.views.generic import DetailView +from registrar.models import Portfolio from registrar.models.user import User -from registrar.models.user_domain_role import UserDomainRole from .mixins import ( - DomainPermission, - DomainInvitationPermission, PortfolioMemberDomainsPermission, PortfolioMemberDomainsEditPermission, PortfolioMemberEditPermission, - UserDeleteDomainRolePermission, UserProfilePermission, PortfolioBasePermission, PortfolioMembersPermission, @@ -24,92 +20,6 @@ import logging logger = logging.getLogger(__name__) -class DomainPermissionView(DomainPermission, DetailView, abc.ABC): - """Abstract base view for domains that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # DetailView property for what model this is viewing - model = Domain - pk_url_kwarg = "domain_pk" - # variable name in template context for the model object - context_object_name = "domain" - - # Adds context information for user permissions - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) - user = self.request.user - context["is_analyst_or_superuser"] = user.has_perm("registrar.analyst_access_permission") or user.has_perm( - "registrar.full_access_permission" - ) - context["is_domain_manager"] = UserDomainRole.objects.filter(user=user, domain=self.object).exists() - context["is_portfolio_user"] = self.can_access_domain_via_portfolio(self.object.pk) - context["is_editable"] = self.is_editable() - # Stored in a variable for the linter - action = "analyst_action" - action_location = "analyst_action_location" - # Flag to see if an analyst is attempting to make edits - if action in self.request.session: - context[action] = self.request.session[action] - if action_location in self.request.session: - context[action_location] = self.request.session[action_location] - - return context - - def is_editable(self): - """Returns whether domain is editable in the context of the view""" - domain_editable = self.object.is_editable() - if not domain_editable: - return False - - # if user is domain manager or analyst or admin, return True - if ( - self.can_access_other_user_domains(self.object.id) - or UserDomainRole.objects.filter(user=self.request.user, domain=self.object).exists() - ): - return True - - return False - - def can_access_domain_via_portfolio(self, pk): - """Most views should not allow permission to portfolio users. - If particular views allow access to the domain pages, they will need to override - this function. - """ - return False - - # Abstract property enforces NotImplementedError on an attribute. - @property - @abc.abstractmethod - def template_name(self): - raise NotImplementedError - - -class DomainInvitationPermissionCancelView(DomainInvitationPermission, UpdateView, abc.ABC): - """Abstract view for cancelling a DomainInvitation.""" - - model = DomainInvitation - object: DomainInvitation - - -class UserDomainRolePermissionDeleteView(UserDeleteDomainRolePermission, DeleteView, abc.ABC): - """Abstract base view for deleting a UserDomainRole. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # DetailView property for what model this is viewing - model = UserDomainRole - # workaround for type mismatch in DeleteView - object: UserDomainRole - - # variable name in template context for the model object - context_object_name = "userdomainrole" - - class UserProfilePermissionView(UserProfilePermission, DetailView, abc.ABC): """Abstract base view for user profile view that enforces permissions. From 898a66ccc90fcb1058afe5e493da36e43736cfe7 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 12 Feb 2025 09:44:39 -0500 Subject: [PATCH 092/226] removed domain renewal feature --- src/registrar/context_processors.py | 3 -- src/registrar/models/domain.py | 2 +- src/registrar/models/user.py | 3 -- src/registrar/templates/domain_detail.html | 10 ++--- src/registrar/templates/domain_sidebar.html | 2 +- .../templates/includes/domains_table.html | 6 +-- src/registrar/tests/test_views_domain.py | 39 +++---------------- src/registrar/views/domain.py | 3 +- 8 files changed, 16 insertions(+), 52 deletions(-) diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py index a078c81ac..061c0ab4f 100644 --- a/src/registrar/context_processors.py +++ b/src/registrar/context_processors.py @@ -68,7 +68,6 @@ def portfolio_permissions(request): "has_organization_requests_flag": False, "has_organization_members_flag": False, "is_portfolio_admin": False, - "has_domain_renewal_flag": False, } try: portfolio = request.session.get("portfolio") @@ -77,7 +76,6 @@ def portfolio_permissions(request): portfolio_context.update( { "has_organization_feature_flag": True, - "has_domain_renewal_flag": request.user.has_domain_renewal_flag(), } ) @@ -95,7 +93,6 @@ def portfolio_permissions(request): "has_organization_requests_flag": request.user.has_organization_requests_flag(), "has_organization_members_flag": request.user.has_organization_members_flag(), "is_portfolio_admin": request.user.is_portfolio_admin(portfolio), - "has_domain_renewal_flag": request.user.has_domain_renewal_flag(), } return portfolio_context diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 0f0b3f112..cd768f76c 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1583,7 +1583,7 @@ class Domain(TimeStampedModel, DomainHelper): # Given expired is not a physical state, but it is displayed as such, # We need custom logic to determine this message. help_text = "This domain has expired. Complete the online renewal process to maintain access." - elif flag_is_active(request, "domain_renewal") and self.is_expiring(): + elif self.is_expiring(): help_text = "This domain is expiring soon. Complete the online renewal process to maintain access." else: help_text = Domain.State.get_help_text(self.state) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 6f8ee499b..82a0465c5 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -271,9 +271,6 @@ class User(AbstractUser): def is_portfolio_admin(self, portfolio): return "Admin" in self.portfolio_role_summary(portfolio) - def has_domain_renewal_flag(self): - return flag_is_active_for_user(self, "domain_renewal") - def get_first_portfolio(self): permission = self.portfolio_permissions.first() if permission: diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 758c43366..57749f038 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -35,7 +35,7 @@ {# UNKNOWN domains would not have an expiration date and thus would show 'Expired' #} {% if domain.is_expired and domain.state != domain.State.UNKNOWN %} Expired - {% elif has_domain_renewal_flag and domain.is_expiring %} + {% elif domain.is_expiring %} Expiring soon {% elif domain.state == domain.State.UNKNOWN or domain.state == domain.State.DNS_NEEDED %} DNS needed @@ -46,17 +46,17 @@ {% if domain.get_state_help_text %}

    - {% if has_domain_renewal_flag and domain.is_expired and is_domain_manager %} + {% if domain.is_expired and is_domain_manager %} This domain has expired, but it is still online. {% url 'domain-renewal' pk=domain.id as url %} Renew to maintain access. - {% elif has_domain_renewal_flag and domain.is_expiring and is_domain_manager %} + {% elif domain.is_expiring and is_domain_manager %} This domain will expire soon. {% url 'domain-renewal' pk=domain.id as url %} Renew to maintain access. - {% elif has_domain_renewal_flag and domain.is_expiring and is_portfolio_user %} + {% elif domain.is_expiring and is_portfolio_user %} This domain will expire soon. Contact one of the listed domain managers to renew the domain. - {% elif has_domain_renewal_flag and domain.is_expired and is_portfolio_user %} + {% elif domain.is_expired and is_portfolio_user %} This domain has expired, but it is still online. Contact one of the listed domain managers to renew the domain. {% else %} {{ domain.get_state_help_text }} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 5946b6859..3302a6a79 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -81,7 +81,7 @@ {% endwith %} - {% if has_domain_renewal_flag and is_domain_manager%} + {% if is_domain_manager%} {% if domain.is_expiring or domain.is_expired %} {% with url_name="domain-renewal" %} {% include "includes/domain_sidenav_item.html" with item_text="Renewal form" %} diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 94cb4ea6d..3cf04a830 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -9,7 +9,7 @@ -{% if has_domain_renewal_flag and num_expiring_domains > 0 and has_any_domains_portfolio_permission %} +{% if num_expiring_domains > 0 and has_any_domains_portfolio_permission %}

    @@ -75,7 +75,7 @@
    - {% if has_domain_renewal_flag and num_expiring_domains > 0 and not portfolio %} + {% if num_expiring_domains > 0 and not portfolio %}
    @@ -173,7 +173,6 @@ >Deleted
    - {% if has_domain_renewal_flag %}
    Expiring soon
    - {% endif %}
    diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index dc5bff27a..2f1bcf5e3 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -477,7 +477,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.domain_with_ip.expiration_date = self.expiration_date_one_year_out() self.domain_with_ip.save() - @override_flag("domain_renewal", active=True) def test_expiring_domain_on_detail_page_as_domain_manager(self): """If a user is a domain manager and their domain is expiring soon, user should be able to see the "Renew to maintain access" link domain overview detail box.""" @@ -496,7 +495,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertNotContains(detail_page, "DNS needed") self.assertNotContains(detail_page, "Expired") - @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) def test_expiring_domain_on_detail_page_in_org_model_as_a_non_domain_manager(self): """In org model: If a user is NOT a domain manager and their domain is expiring soon, @@ -534,7 +532,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): ) self.assertContains(detail_page, "Contact one of the listed domain managers to renew the domain.") - @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) def test_expiring_domain_on_detail_page_in_org_model_as_a_domain_manager(self): """Inorg model: If a user is a domain manager and their domain is expiring soon, @@ -555,7 +552,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): ) self.assertContains(detail_page, "Renew to maintain access") - @override_flag("domain_renewal", active=True) def test_domain_renewal_form_and_sidebar_expiring(self): """If a user is a domain manager and their domain is expiring soon, user should be able to see Renewal Form on the sidebar.""" @@ -584,7 +580,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertEqual(response.status_code, 200) self.assertContains(response, f"Renew {self.domain_to_renew.name}") - @override_flag("domain_renewal", active=True) def test_domain_renewal_form_and_sidebar_expired(self): """If a user is a domain manager and their domain is expired, user should be able to see Renewal Form on the sidebar.""" @@ -614,7 +609,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertEqual(response.status_code, 200) self.assertContains(response, f"Renew {self.domain_to_renew.name}") - @override_flag("domain_renewal", active=True) def test_domain_renewal_form_your_contact_info_edit(self): """Checking that if a user is a domain manager they can edit the Your Profile portion of the Renewal Form.""" @@ -634,7 +628,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertEqual(edit_page.status_code, 200) self.assertContains(edit_page, "Review the details below and update any required information") - @override_flag("domain_renewal", active=True) def test_domain_renewal_form_security_email_edit(self): """Checking that if a user is a domain manager they can edit the Security Email portion of the Renewal Form.""" @@ -657,7 +650,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertEqual(edit_page.status_code, 200) self.assertContains(edit_page, "A security contact should be capable of evaluating") - @override_flag("domain_renewal", active=True) def test_domain_renewal_form_domain_manager_edit(self): """Checking that if a user is a domain manager they can edit the Domain Manager portion of the Renewal Form.""" @@ -677,7 +669,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): self.assertEqual(edit_page.status_code, 200) self.assertContains(edit_page, "Domain managers can update all information related to a domain") - @override_flag("domain_renewal", active=True) def test_domain_renewal_form_not_expired_or_expiring(self): """Checking that if the user's domain is not expired or expiring that user should not be able to access /renewal and that it should receive a 403.""" @@ -686,7 +677,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): renewal_page = self.client.get(reverse("domain-renewal", kwargs={"pk": self.domain_not_expiring.id})) self.assertEqual(renewal_page.status_code, 403) - @override_flag("domain_renewal", active=True) def test_domain_renewal_form_does_not_appear_if_not_domain_manager(self): """If user is not a domain manager and tries to access /renewal, user should receive a 403.""" with patch.object(Domain, "is_expired", self.custom_is_expired_true), patch.object( @@ -695,7 +685,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): renewal_page = self.client.get(reverse("domain-renewal", kwargs={"pk": self.domain_no_domain_manager.id})) self.assertEqual(renewal_page.status_code, 403) - @override_flag("domain_renewal", active=True) def test_ack_checkbox_not_checked(self): """If user don't check the checkbox, user should receive an error message.""" # Grab the renewal URL @@ -707,7 +696,6 @@ class TestDomainDetailDomainRenewal(TestDomainOverview): error_message = "Check the box if you read and agree to the requirements for operating a .gov domain." self.assertContains(response, error_message) - @override_flag("domain_renewal", active=True) def test_ack_checkbox_checked(self): """If user check the checkbox and submits the form, user should be redirected Domain Over page with an updated by 1 year expiration date""" @@ -2966,26 +2954,15 @@ class TestDomainRenewal(TestWithUser): pass super().tearDown() - # Remove test_without_domain_renewal_flag when domain renewal is released as a feature @less_console_noise_decorator - @override_flag("domain_renewal", active=False) - def test_without_domain_renewal_flag(self): - self.client.force_login(self.user) - domains_page = self.client.get("/") - self.assertNotContains(domains_page, "will expire soon") - self.assertNotContains(domains_page, "Expiring soon") - - @less_console_noise_decorator - @override_flag("domain_renewal", active=True) - def test_domain_renewal_flag_single_domain(self): + def test_domain_with_single_domain(self): self.client.force_login(self.user) domains_page = self.client.get("/") self.assertContains(domains_page, "One domain will expire soon") self.assertContains(domains_page, "Expiring soon") @less_console_noise_decorator - @override_flag("domain_renewal", active=True) - def test_with_domain_renewal_flag_mulitple_domains(self): + def test_with_mulitple_domains(self): today = datetime.now() expiring_date = (today + timedelta(days=30)).strftime("%Y-%m-%d") self.domain_with_another_expiring, _ = Domain.objects.get_or_create( @@ -3001,8 +2978,7 @@ class TestDomainRenewal(TestWithUser): self.assertContains(domains_page, "Expiring soon") @less_console_noise_decorator - @override_flag("domain_renewal", active=True) - def test_with_domain_renewal_flag_no_expiring_domains(self): + def test_with_no_expiring_domains(self): UserDomainRole.objects.filter(user=self.user, domain=self.domain_with_expired_date).delete() UserDomainRole.objects.filter(user=self.user, domain=self.domain_with_expiring_soon_date).delete() self.client.force_login(self.user) @@ -3010,18 +2986,16 @@ class TestDomainRenewal(TestWithUser): self.assertNotContains(domains_page, "will expire soon") @less_console_noise_decorator - @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) - def test_domain_renewal_flag_single_domain_w_org_feature_flag(self): + def test_single_domain_w_org_feature_flag(self): self.client.force_login(self.user) domains_page = self.client.get("/") self.assertContains(domains_page, "One domain will expire soon") self.assertContains(domains_page, "Expiring soon") @less_console_noise_decorator - @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) - def test_with_domain_renewal_flag_mulitple_domains_w_org_feature_flag(self): + def test_with_mulitple_domains_w_org_feature_flag(self): today = datetime.now() expiring_date = (today + timedelta(days=31)).strftime("%Y-%m-%d") self.domain_with_another_expiring_org_model, _ = Domain.objects.get_or_create( @@ -3037,9 +3011,8 @@ class TestDomainRenewal(TestWithUser): self.assertContains(domains_page, "Expiring soon") @less_console_noise_decorator - @override_flag("domain_renewal", active=True) @override_flag("organization_feature", active=True) - def test_with_domain_renewal_flag_no_expiring_domains_w_org_feature_flag(self): + def test_no_expiring_domains_w_org_feature_flag(self): UserDomainRole.objects.filter(user=self.user, domain=self.domain_with_expired_date).delete() UserDomainRole.objects.filter(user=self.user, domain=self.domain_with_expiring_soon_date).delete() self.client.force_login(self.user) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 297cb689a..27ee44068 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -366,7 +366,7 @@ class DomainRenewalView(DomainBaseView): return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk})) # if not valid, render the template with error messages - # passing editable, has_domain_renewal_flag, and is_editable for re-render + # passing editable,å and is_editable for re-render return render( request, "domain_renewal.html", @@ -374,7 +374,6 @@ class DomainRenewalView(DomainBaseView): "domain": domain, "form": form, "is_editable": True, - "has_domain_renewal_flag": True, "is_domain_manager": True, }, ) From 908e71e3ae1d6de82cb42ce6e2282c9d44937178 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 12 Feb 2025 09:53:55 -0500 Subject: [PATCH 093/226] portfolio member permissions --- src/registrar/decorators.py | 18 +++ src/registrar/views/member_domains_json.py | 27 ++--- src/registrar/views/portfolio_members_json.py | 7 +- src/registrar/views/portfolios.py | 89 +++++++++------ src/registrar/views/utility/__init__.py | 3 - src/registrar/views/utility/mixins.py | 107 ------------------ .../views/utility/permission_views.py | 67 ----------- 7 files changed, 90 insertions(+), 228 deletions(-) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 237985f02..98a37c62c 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -18,6 +18,8 @@ HAS_PORTFOLIO_DOMAINS_VIEW_ALL = "has_portfolio_domains_view_all" HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM = "has_portfolio_domain_requests_any_perm" HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL = "has_portfolio_domain_requests_view_all" HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT = "has_portfolio_domain_requests_edit" +HAS_PORTFOLIO_MEMBERS_ANY_PERM = "has_portfolio_members_any_perm" +HAS_PORTFOLIO_MEMBERS_EDIT = "has_portfolio_members_edit" def grant_access(*rules): @@ -142,6 +144,22 @@ def _user_has_permission(user, request, rules, **kwargs): print(has_permission) conditions_met.append(has_permission) + if not any(conditions_met) and HAS_PORTFOLIO_MEMBERS_ANY_PERM in rules: + portfolio = request.session.get("portfolio") + has_permission = user.is_org_user(request) and ( + user.has_view_members_portfolio_permission(portfolio) or + user.has_edit_members_portfolio_permission(portfolio) + ) + conditions_met.append(has_permission) + + if not any(conditions_met) and HAS_PORTFOLIO_MEMBERS_EDIT in rules: + portfolio = request.session.get("portfolio") + has_permission = ( + user.is_org_user(request) and + user.has_edit_members_portfolio_permission(portfolio) + ) + conditions_met.append(has_permission) + return any(conditions_met) diff --git a/src/registrar/views/member_domains_json.py b/src/registrar/views/member_domains_json.py index 3d24336bb..40b7bea74 100644 --- a/src/registrar/views/member_domains_json.py +++ b/src/registrar/views/member_domains_json.py @@ -4,37 +4,38 @@ from django.http import JsonResponse from django.core.paginator import Paginator from django.shortcuts import get_object_or_404 from django.views import View +from registrar.decorators import HAS_PORTFOLIO_MEMBERS_ANY_PERM, grant_access from registrar.models import UserDomainRole, Domain, DomainInformation, User from django.urls import reverse from django.db.models import Q from registrar.models.domain_invitation import DomainInvitation -from registrar.views.utility.mixins import PortfolioMemberDomainsPermission logger = logging.getLogger(__name__) -class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioMemberDomainsJson(View): def get(self, request): """Given the current request, get all domains that are associated with the portfolio, or associated with the member/invited member""" - domain_ids = self.get_domain_ids_from_request(request) + domain_ids = self._get_domain_ids_from_request(request) objects = Domain.objects.filter(id__in=domain_ids).select_related("domain_info__sub_organization") unfiltered_total = objects.count() - objects = self.apply_search(objects, request) - objects = self.apply_sorting(objects, request) + objects = self._apply_search(objects, request) + objects = self._apply_sorting(objects, request) - paginator = Paginator(objects, self.get_page_size(request)) + paginator = Paginator(objects, self._get_page_size(request)) page_number = request.GET.get("page") page_obj = paginator.get_page(page_number) member_id = request.GET.get("member_id") - domains = [self.serialize_domain(domain, member_id, request.user) for domain in page_obj.object_list] + domains = [self._serialize_domain(domain, member_id, request.user) for domain in page_obj.object_list] return JsonResponse( { @@ -48,7 +49,7 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): } ) - def get_page_size(self, request): + def _get_page_size(self, request): """Gets the page size. If member_only, need to return the entire result set every time, so need @@ -65,7 +66,7 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): # later return 1000 - def get_domain_ids_from_request(self, request): + def _get_domain_ids_from_request(self, request): """Get domain ids from request. request.get.email - email address of invited member @@ -100,13 +101,13 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): logger.warning("Invalid search criteria, returning empty results list") return [] - def apply_search(self, queryset, request): + def _apply_search(self, queryset, request): search_term = request.GET.get("search_term") if search_term: queryset = queryset.filter(Q(name__icontains=search_term)) return queryset - def apply_sorting(self, queryset, request): + def _apply_sorting(self, queryset, request): # Get the sorting parameters from the request sort_by = request.GET.get("sort_by", "name") order = request.GET.get("order", "asc") @@ -141,7 +142,7 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): return queryset - def serialize_domain(self, domain, member_id, user): + def _serialize_domain(self, domain, member_id, user): suborganization_name = None try: domain_info = domain.domain_info @@ -176,7 +177,7 @@ class PortfolioMemberDomainsJson(PortfolioMemberDomainsPermission, View): "state": domain.state, "state_display": domain.state_display(), "get_state_help_text": domain.get_state_help_text(), - "action_url": reverse("domain", kwargs={"pk": domain.id}), + "action_url": reverse("domain", kwargs={"domain_pk": domain.id}), "action_label": ("View" if view_only else "Manage"), "svg_icon": ("visibility" if view_only else "settings"), "domain_info__sub_organization": suborganization_name, diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 29dc6a71c..ecdd24441 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -6,16 +6,17 @@ from django.contrib.postgres.aggregates import ArrayAgg from django.urls import reverse from django.views import View +from registrar.decorators import HAS_PORTFOLIO_MEMBERS_ANY_PERM, grant_access from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user_portfolio_permission import UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.views.utility.mixins import PortfolioMembersPermission from registrar.models.utility.orm_helper import ArrayRemoveNull from django.contrib.postgres.aggregates import StringAgg -class PortfolioMembersJson(PortfolioMembersPermission, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioMembersJson(View): def get(self, request): """Fetch members (permissions and invitations) for the given portfolio.""" @@ -236,7 +237,7 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): ), # split domain_info array values into ids to form urls, and names "domain_urls": [ - reverse("domain", kwargs={"pk": domain_info.split(":")[0]}) for domain_info in domain_info_list + reverse("domain", kwargs={"domain_pk": domain_info.split(":")[0]}) for domain_info in domain_info_list ], "domain_names": [domain_info.split(":")[1] for domain_info in domain_info_list], "is_admin": is_admin, diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index dc1357585..f0c4841f1 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -5,20 +5,26 @@ from django.http import Http404, JsonResponse from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils.safestring import mark_safe +from django.views.generic import DetailView from django.contrib import messages from registrar.decorators import ( HAS_PORTFOLIO_DOMAIN_REQUESTS_ANY_PERM, HAS_PORTFOLIO_DOMAINS_ANY_PERM, + HAS_PORTFOLIO_MEMBERS_ANY_PERM, + HAS_PORTFOLIO_MEMBERS_EDIT, IS_PORTFOLIO_MEMBER, grant_access, ) from registrar.forms import portfolio as portfolioForms -from registrar.models import Portfolio, User -from registrar.models.domain import Domain -from registrar.models.domain_invitation import DomainInvitation -from registrar.models.portfolio_invitation import PortfolioInvitation -from registrar.models.user_domain_role import UserDomainRole -from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models import ( + Domain, + DomainInvitation, + Portfolio, + PortfolioInvitation, + User, + UserDomainRole, + UserPortfolioPermission +) from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.utility.email import EmailSendingError from registrar.utility.email_invitations import ( @@ -29,15 +35,6 @@ from registrar.utility.email_invitations import ( ) from registrar.utility.errors import MissingEmailError from registrar.utility.enums import DefaultUserValues -from registrar.views.utility.mixins import PortfolioMemberPermission -from registrar.views.utility.permission_views import ( - PortfolioBasePermissionView, - PortfolioMemberDomainsPermissionView, - PortfolioMemberDomainsEditPermissionView, - PortfolioMemberEditPermissionView, - PortfolioMemberPermissionView, - PortfolioMembersPermissionView, -) from django.views.generic import View from django.views.generic.edit import FormMixin from django.db import IntegrityError @@ -71,8 +68,10 @@ class PortfolioDomainRequestsView(View): return render(request, "portfolio_requests.html") -class PortfolioMemberView(PortfolioMemberPermissionView, View): - +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioMemberView(DetailView, View): + model = Portfolio + context_object_name = "portfolio" template_name = "portfolio_member.html" def get(self, request, pk): @@ -113,7 +112,8 @@ class PortfolioMemberView(PortfolioMemberPermissionView, View): ) -class PortfolioMemberDeleteView(PortfolioMemberPermission, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioMemberDeleteView(View): def post(self, request, pk): """ @@ -190,8 +190,10 @@ class PortfolioMemberDeleteView(PortfolioMemberPermission, View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): - +@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +class PortfolioMemberEditView(DetailView, View): + model = Portfolio + context_object_name = "portfolio" template_name = "portfolio_member_permissions.html" form_class = portfolioForms.PortfolioMemberForm @@ -265,7 +267,8 @@ class PortfolioMemberEditView(PortfolioMemberEditPermissionView, View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -class PortfolioMemberDomainsView(PortfolioMemberDomainsPermissionView, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioMemberDomainsView(View): template_name = "portfolio_member_domains.html" @@ -283,8 +286,10 @@ class PortfolioMemberDomainsView(PortfolioMemberDomainsPermissionView, View): ) -class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, View): - +@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +class PortfolioMemberDomainsEditView(DetailView, View): + model = Portfolio + context_object_name = "portfolio" template_name = "portfolio_member_domains_edit.html" def get(self, request, pk): @@ -393,8 +398,10 @@ class PortfolioMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, V UserDomainRole.objects.filter(domain_id__in=removed_domain_ids, user=member).delete() -class PortfolioInvitedMemberView(PortfolioMemberPermissionView, View): - +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioInvitedMemberView(DetailView, View): + model = Portfolio + context_object_name = "portfolio" template_name = "portfolio_member.html" # form_class = PortfolioInvitedMemberForm @@ -435,7 +442,8 @@ class PortfolioInvitedMemberView(PortfolioMemberPermissionView, View): ) -class PortfolioInvitedMemberDeleteView(PortfolioMemberPermission, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioInvitedMemberDeleteView(View): def post(self, request, pk): """ @@ -478,8 +486,10 @@ class PortfolioInvitedMemberDeleteView(PortfolioMemberPermission, View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): - +@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +class PortfolioInvitedMemberEditView(DetailView, View): + model = Portfolio + context_object_name = "portfolio" template_name = "portfolio_member_permissions.html" form_class = portfolioForms.PortfolioInvitedMemberForm @@ -547,7 +557,8 @@ class PortfolioInvitedMemberEditView(PortfolioMemberEditPermissionView, View): messages.warning(self.request, "Could not send email notification to existing organization admins.") -class PortfolioInvitedMemberDomainsView(PortfolioMemberDomainsPermissionView, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioInvitedMemberDomainsView(View): template_name = "portfolio_member_domains.html" @@ -562,9 +573,11 @@ class PortfolioInvitedMemberDomainsView(PortfolioMemberDomainsPermissionView, Vi }, ) +@grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) +class PortfolioInvitedMemberDomainsEditView(DetailView, View): -class PortfolioInvitedMemberDomainsEditView(PortfolioMemberDomainsEditPermissionView, View): - + model = Portfolio + context_object_name = "portfolio" template_name = "portfolio_member_domains_edit.html" def get(self, request, pk): @@ -749,7 +762,8 @@ class PortfolioNoDomainRequestsView(View): return context -class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): +@grant_access(IS_PORTFOLIO_MEMBER) +class PortfolioOrganizationView(DetailView, FormMixin): """ View to handle displaying and updating the portfolio's organization details. """ @@ -811,7 +825,8 @@ class PortfolioOrganizationView(PortfolioBasePermissionView, FormMixin): return reverse("organization") -class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): +@grant_access(IS_PORTFOLIO_MEMBER) +class PortfolioSeniorOfficialView(DetailView, FormMixin): """ View to handle displaying and updating the portfolio's senior official details. For now, this view is readonly. @@ -842,7 +857,8 @@ class PortfolioSeniorOfficialView(PortfolioBasePermissionView, FormMixin): return self.render_to_response(self.get_context_data(form=form)) -class PortfolioMembersView(PortfolioMembersPermissionView, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioMembersView(View): template_name = "portfolio_members.html" @@ -851,10 +867,13 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View): return render(request, "portfolio_members.html") -class PortfolioAddMemberView(PortfolioMembersPermissionView, FormMixin): +@grant_access(HAS_PORTFOLIO_MEMBERS_ANY_PERM) +class PortfolioAddMemberView(DetailView, FormMixin): template_name = "portfolio_members_add_new.html" form_class = portfolioForms.PortfolioNewMemberForm + model = Portfolio + context_object_name = "portfolio" def get(self, request, *args, **kwargs): """Handle GET requests to display the form.""" diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py index f80774ef3..12fcc325d 100644 --- a/src/registrar/views/utility/__init__.py +++ b/src/registrar/views/utility/__init__.py @@ -1,7 +1,4 @@ from .steps_helper import StepsHelper from .always_404 import always_404 -from .permission_views import ( - PortfolioMembersPermission, -) from .api_views import get_senior_official_from_federal_agency_json diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 23895cb5c..1762d4900 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -202,110 +202,3 @@ class UserProfilePermission(PermissionsLoginMixin): return False return True - - -class PortfolioBasePermission(PermissionsLoginMixin): - """Permission mixin that redirects to portfolio pages if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to this portfolio. - - The user is in self.request.user and the portfolio can be looked - up from the portfolio's primary key in self.kwargs["pk"] - """ - if not self.request.user.is_authenticated: - return False - - return self.request.user.is_org_user(self.request) - - -class PortfolioMembersPermission(PortfolioBasePermission): - """Permission mixin that allows access to portfolio members pages if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to members for this portfolio. - - The user is in self.request.user and the portfolio can be looked - up from the portfolio's primary key in self.kwargs["pk"]""" - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_view_members_portfolio_permission( - portfolio - ) and not self.request.user.has_edit_members_portfolio_permission(portfolio): - return False - - return super().has_permission() - - -class PortfolioMemberPermission(PortfolioBasePermission): - """Permission mixin that allows access to portfolio member or invited member pages if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access to members or invited members for this portfolio. - - The user is in self.request.user and the portfolio can be looked - up from the portfolio's primary key in self.kwargs["pk"]""" - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_view_members_portfolio_permission( - portfolio - ) and not self.request.user.has_edit_members_portfolio_permission(portfolio): - return False - - return super().has_permission() - - -class PortfolioMemberEditPermission(PortfolioBasePermission): - """Permission mixin that allows access to portfolio member or invited member pages if user - has access to edit, otherwise 403""" - - def has_permission(self): - """Check if this user has access to members or invited members for this portfolio. - - The user is in self.request.user and the portfolio can be looked - up from the portfolio's primary key in self.kwargs["pk"]""" - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_edit_members_portfolio_permission(portfolio): - return False - - return super().has_permission() - - -class PortfolioMemberDomainsPermission(PortfolioBasePermission): - """Permission mixin that allows access to portfolio member or invited member domains pages if user - has access to edit, otherwise 403""" - - def has_permission(self): - """Check if this user has access to member or invited member domains for this portfolio. - - The user is in self.request.user and the portfolio can be looked - up from the portfolio's primary key in self.kwargs["pk"]""" - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_view_members_portfolio_permission( - portfolio - ) and not self.request.user.has_edit_members_portfolio_permission(portfolio): - return False - - return super().has_permission() - - -class PortfolioMemberDomainsEditPermission(PortfolioBasePermission): - """Permission mixin that allows access to portfolio member or invited member domains edit pages if user - has access to edit, otherwise 403""" - - def has_permission(self): - """Check if this user has access to member or invited member domains for this portfolio. - - The user is in self.request.user and the portfolio can be looked - up from the portfolio's primary key in self.kwargs["pk"]""" - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_edit_members_portfolio_permission(portfolio): - return False - - return super().has_permission() diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py index 02a2b029b..1961e1cdd 100644 --- a/src/registrar/views/utility/permission_views.py +++ b/src/registrar/views/utility/permission_views.py @@ -7,13 +7,7 @@ from registrar.models import Portfolio from registrar.models.user import User from .mixins import ( - PortfolioMemberDomainsPermission, - PortfolioMemberDomainsEditPermission, - PortfolioMemberEditPermission, UserProfilePermission, - PortfolioBasePermission, - PortfolioMembersPermission, - PortfolioMemberPermission, ) import logging @@ -37,64 +31,3 @@ class UserProfilePermissionView(UserProfilePermission, DetailView, abc.ABC): @abc.abstractmethod def template_name(self): raise NotImplementedError - - -class PortfolioBasePermissionView(PortfolioBasePermission, DetailView, abc.ABC): - """Abstract base view for portfolio views that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # DetailView property for what model this is viewing - model = Portfolio - # variable name in template context for the model object - context_object_name = "portfolio" - - # Abstract property enforces NotImplementedError on an attribute. - @property - @abc.abstractmethod - def template_name(self): - raise NotImplementedError - - -class PortfolioMembersPermissionView(PortfolioMembersPermission, PortfolioBasePermissionView, abc.ABC): - """Abstract base view for portfolio members views that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - -class PortfolioMemberPermissionView(PortfolioMemberPermission, PortfolioBasePermissionView, abc.ABC): - """Abstract base view for portfolio member views that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - -class PortfolioMemberEditPermissionView(PortfolioMemberEditPermission, PortfolioBasePermissionView, abc.ABC): - """Abstract base view for portfolio member edit views that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - -class PortfolioMemberDomainsPermissionView(PortfolioMemberDomainsPermission, PortfolioBasePermissionView, abc.ABC): - """Abstract base view for portfolio member domains views that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - -class PortfolioMemberDomainsEditPermissionView( - PortfolioMemberDomainsEditPermission, PortfolioBasePermissionView, abc.ABC -): - """Abstract base view for portfolio member domains edit views that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ From 4daac2bec03e452176c984f7ef4015609faf0a37 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 12 Feb 2025 10:07:51 -0500 Subject: [PATCH 094/226] user profile permissions --- src/registrar/views/user_profile.py | 7 ++-- src/registrar/views/utility/mixins.py | 17 ---------- .../views/utility/permission_views.py | 33 ------------------- 3 files changed, 5 insertions(+), 52 deletions(-) delete mode 100644 src/registrar/views/utility/permission_views.py diff --git a/src/registrar/views/user_profile.py b/src/registrar/views/user_profile.py index 2012d12ab..3434eedb3 100644 --- a/src/registrar/views/user_profile.py +++ b/src/registrar/views/user_profile.py @@ -5,22 +5,25 @@ import logging from django.contrib import messages from django.http import QueryDict +from django.views.generic import DetailView from django.views.generic.edit import FormMixin +from registrar.decorators import ALL, grant_access from registrar.forms.user_profile import UserProfileForm, FinishSetupProfileForm from django.urls import NoReverseMatch, reverse from registrar.models.user import User from registrar.models.utility.generic_helper import replace_url_queryparams -from registrar.views.utility.permission_views import UserProfilePermissionView logger = logging.getLogger(__name__) -class UserProfileView(UserProfilePermissionView, FormMixin): +@grant_access(ALL) +class UserProfileView(DetailView, FormMixin): """ Base View for the User Profile. Handles getting and setting the User Profile """ model = User + context_object_name = "user" template_name = "profile.html" form_class = UserProfileForm base_view_name = "user-profile" diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 1762d4900..d3ec95af5 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -185,20 +185,3 @@ class PortfolioReportsPermission(PermissionsLoginMixin): return False return self.request.user.is_org_user(self.request) - - -class UserProfilePermission(PermissionsLoginMixin): - """Permission mixin that redirects to user profile if user - has access, otherwise 403""" - - def has_permission(self): - """Check if this user has access. - - If the user is authenticated, they have access - """ - - # Check if the user is authenticated - if not self.request.user.is_authenticated: - return False - - return True diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py deleted file mode 100644 index 1961e1cdd..000000000 --- a/src/registrar/views/utility/permission_views.py +++ /dev/null @@ -1,33 +0,0 @@ -"""View classes that enforce authorization.""" - -import abc # abstract base class - -from django.views.generic import DetailView -from registrar.models import Portfolio -from registrar.models.user import User - -from .mixins import ( - UserProfilePermission, -) -import logging - -logger = logging.getLogger(__name__) - - -class UserProfilePermissionView(UserProfilePermission, DetailView, abc.ABC): - """Abstract base view for user profile view that enforces permissions. - - This abstract view cannot be instantiated. Actual views must specify - `template_name`. - """ - - # DetailView property for what model this is viewing - model = User - # variable name in template context for the model object - context_object_name = "user" - - # Abstract property enforces NotImplementedError on an attribute. - @property - @abc.abstractmethod - def template_name(self): - raise NotImplementedError From 9ad7b7523b242cd982a973da5567807aa02e88c1 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 12 Feb 2025 10:46:06 -0500 Subject: [PATCH 095/226] analytics, reports and transfer user --- src/registrar/decorators.py | 9 +++++ src/registrar/views/report_views.py | 28 +++++++------- src/registrar/views/transfer_user.py | 2 + src/registrar/views/utility/mixins.py | 54 +-------------------------- 4 files changed, 26 insertions(+), 67 deletions(-) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 98a37c62c..0f2c948b3 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -20,6 +20,7 @@ HAS_PORTFOLIO_DOMAIN_REQUESTS_VIEW_ALL = "has_portfolio_domain_requests_view_all HAS_PORTFOLIO_DOMAIN_REQUESTS_EDIT = "has_portfolio_domain_requests_edit" HAS_PORTFOLIO_MEMBERS_ANY_PERM = "has_portfolio_members_any_perm" HAS_PORTFOLIO_MEMBERS_EDIT = "has_portfolio_members_edit" +HAS_PORTFOLIO_MEMBERS_VIEW = "has_portfolio_members_view" def grant_access(*rules): @@ -160,6 +161,14 @@ def _user_has_permission(user, request, rules, **kwargs): ) conditions_met.append(has_permission) + if not any(conditions_met) and HAS_PORTFOLIO_MEMBERS_VIEW in rules: + portfolio = request.session.get("portfolio") + has_permission = ( + user.is_org_user(request) and + user.has_view_members_portfolio_permission(portfolio) + ) + conditions_met.append(has_permission) + return any(conditions_met) diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index 1b9198c79..d64586a2b 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -6,19 +6,17 @@ from django.shortcuts import render from django.contrib import admin from django.db.models import Avg, F -from registrar.views.utility.mixins import DomainAndRequestsReportsPermission, PortfolioReportsPermission +from registrar.decorators import ALL, HAS_PORTFOLIO_MEMBERS_VIEW, IS_STAFF, grant_access from .. import models import datetime from django.utils import timezone -from django.contrib.admin.views.decorators import staff_member_required -from django.utils.decorators import method_decorator from registrar.utility import csv_export import logging logger = logging.getLogger(__name__) -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class AnalyticsView(View): def get(self, request): thirty_days_ago = datetime.datetime.today() - datetime.timedelta(days=30) @@ -152,7 +150,7 @@ class AnalyticsView(View): return render(request, "admin/analytics.html", context) -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDataType(View): def get(self, request, *args, **kwargs): # match the CSV example with all the fields @@ -162,7 +160,8 @@ class ExportDataType(View): return response -class ExportDataTypeUser(DomainAndRequestsReportsPermission, View): +@grant_access(ALL) +class ExportDataTypeUser(View): """Returns a domain report for a given user on the request""" def get(self, request, *args, **kwargs): @@ -173,7 +172,8 @@ class ExportDataTypeUser(DomainAndRequestsReportsPermission, View): return response -class ExportMembersPortfolio(PortfolioReportsPermission, View): +@grant_access(HAS_PORTFOLIO_MEMBERS_VIEW) +class ExportMembersPortfolio(View): """Returns a members report for a given portfolio""" def get(self, request, *args, **kwargs): @@ -201,7 +201,7 @@ class ExportMembersPortfolio(PortfolioReportsPermission, View): return response -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDataFull(View): def get(self, request, *args, **kwargs): # Smaller export based on 1 @@ -211,7 +211,7 @@ class ExportDataFull(View): return response -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDataFederal(View): def get(self, request, *args, **kwargs): # Federal only @@ -221,7 +221,7 @@ class ExportDataFederal(View): return response -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDomainRequestDataFull(View): """Generates a downloaded report containing all Domain Requests (except started)""" @@ -233,7 +233,7 @@ class ExportDomainRequestDataFull(View): return response -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDataDomainsGrowth(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -246,7 +246,7 @@ class ExportDataDomainsGrowth(View): return response -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDataRequestsGrowth(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -259,7 +259,7 @@ class ExportDataRequestsGrowth(View): return response -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDataManagedDomains(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") @@ -271,7 +271,7 @@ class ExportDataManagedDomains(View): return response -@method_decorator(staff_member_required, name="dispatch") +@grant_access(IS_STAFF) class ExportDataUnmanagedDomains(View): def get(self, request, *args, **kwargs): start_date = request.GET.get("start_date", "") diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py index f574b76d9..62cd0a9d2 100644 --- a/src/registrar/views/transfer_user.py +++ b/src/registrar/views/transfer_user.py @@ -4,6 +4,7 @@ from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToO from django.shortcuts import render, get_object_or_404, redirect from django.views import View +from registrar.decorators import IS_STAFF, grant_access from registrar.models.domain import Domain from registrar.models.domain_request import DomainRequest from registrar.models.user import User @@ -18,6 +19,7 @@ from registrar.utility.db_helpers import ignore_unique_violation logger = logging.getLogger(__name__) +@grant_access(IS_STAFF) class TransferUserView(View): """Transfer user methods that set up the transfer_user template and handle the forms on it.""" diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index d3ec95af5..eb58a5125 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -1,6 +1,4 @@ -"""Permissions-related mixin classes.""" - -from django.contrib.auth.mixins import PermissionRequiredMixin +"""Mixin classes.""" import logging @@ -135,53 +133,3 @@ class OrderableFieldsMixin: # Infer the column name in a similar manner to how Django does method.short_description = field.replace("_", " ") return method - - -class PermissionsLoginMixin(PermissionRequiredMixin): - """Mixin that redirects to login page if not logged in, otherwise 403.""" - - def handle_no_permission(self): - self.raise_exception = self.request.user.is_authenticated - return super().handle_no_permission() - - -class DomainAndRequestsReportsPermission(PermissionsLoginMixin): - """Permission mixin for domain and requests csv downloads""" - - def has_permission(self): - """Check if this user has access to this domain. - - The user is in self.request.user and the domain needs to be looked - up from the domain's primary key in self.kwargs["pk"] - """ - - if not self.request.user.is_authenticated: - return False - - if self.request.user.is_restricted(): - return False - - return True - - -class PortfolioReportsPermission(PermissionsLoginMixin): - """Permission mixin for portfolio csv downloads""" - - def has_permission(self): - """Check if this user has access to this domain. - - The user is in self.request.user and the domain needs to be looked - up from the domain's primary key in self.kwargs["pk"] - """ - - if not self.request.user.is_authenticated: - return False - - if self.request.user.is_restricted(): - return False - - portfolio = self.request.session.get("portfolio") - if not self.request.user.has_view_members_portfolio_permission(portfolio): - return False - - return self.request.user.is_org_user(self.request) From 7a1348258d14d443b8f44392652ece8ea95e6169 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 12 Feb 2025 10:50:42 -0500 Subject: [PATCH 096/226] remove superuser, and assign perm to transfer user --- src/registrar/decorators.py | 18 ++++-------------- src/registrar/views/portfolios.py | 3 ++- src/registrar/views/utility/mixins.py | 1 + 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py index 0f2c948b3..517985b6d 100644 --- a/src/registrar/decorators.py +++ b/src/registrar/decorators.py @@ -5,7 +5,6 @@ from registrar.models import Domain, DomainInformation, DomainInvitation, Domain # Constants for clarity ALL = "all" -IS_SUPERUSER = "is_superuser" IS_STAFF = "is_staff" IS_DOMAIN_MANAGER = "is_domain_manager" IS_DOMAIN_REQUEST_CREATOR = "is_domain_request_creator" @@ -88,9 +87,6 @@ def _user_has_permission(user, request, rules, **kwargs): if IS_STAFF in rules: conditions_met.append(user.is_staff) - if not any(conditions_met) and IS_SUPERUSER in rules: - conditions_met.append(user.is_superuser) - if not any(conditions_met) and IS_DOMAIN_MANAGER in rules: has_permission = _is_domain_manager(user, **kwargs) conditions_met.append(has_permission) @@ -148,25 +144,19 @@ def _user_has_permission(user, request, rules, **kwargs): if not any(conditions_met) and HAS_PORTFOLIO_MEMBERS_ANY_PERM in rules: portfolio = request.session.get("portfolio") has_permission = user.is_org_user(request) and ( - user.has_view_members_portfolio_permission(portfolio) or - user.has_edit_members_portfolio_permission(portfolio) + user.has_view_members_portfolio_permission(portfolio) + or user.has_edit_members_portfolio_permission(portfolio) ) conditions_met.append(has_permission) if not any(conditions_met) and HAS_PORTFOLIO_MEMBERS_EDIT in rules: portfolio = request.session.get("portfolio") - has_permission = ( - user.is_org_user(request) and - user.has_edit_members_portfolio_permission(portfolio) - ) + has_permission = user.is_org_user(request) and user.has_edit_members_portfolio_permission(portfolio) conditions_met.append(has_permission) if not any(conditions_met) and HAS_PORTFOLIO_MEMBERS_VIEW in rules: portfolio = request.session.get("portfolio") - has_permission = ( - user.is_org_user(request) and - user.has_view_members_portfolio_permission(portfolio) - ) + has_permission = user.is_org_user(request) and user.has_view_members_portfolio_permission(portfolio) conditions_met.append(has_permission) return any(conditions_met) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index f0c4841f1..41038443b 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -23,7 +23,7 @@ from registrar.models import ( PortfolioInvitation, User, UserDomainRole, - UserPortfolioPermission + UserPortfolioPermission, ) from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from registrar.utility.email import EmailSendingError @@ -573,6 +573,7 @@ class PortfolioInvitedMemberDomainsView(View): }, ) + @grant_access(HAS_PORTFOLIO_MEMBERS_EDIT) class PortfolioInvitedMemberDomainsEditView(DetailView, View): diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index eb58a5125..e228fbcd8 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -1,4 +1,5 @@ """Mixin classes.""" + import logging From 2a6d401e9b23c2c8d22bedf8282903b55f3ea6ee Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:00:32 -0700 Subject: [PATCH 097/226] Move get-gov-reports to src/ @rachidatecs see above ^ --- src/registrar/assets/js/get-gov-reports.js | 203 ------------------ .../assets/src/js/getgov-admin/analytics.js | 196 +++++++++++++++++ .../src/js/getgov-admin/helpers-admin.js | 10 + .../assets/src/js/getgov-admin/main.js | 4 + src/registrar/templates/admin/analytics.html | 14 +- src/registrar/templates/admin/base_site.html | 1 - 6 files changed, 217 insertions(+), 211 deletions(-) delete mode 100644 src/registrar/assets/js/get-gov-reports.js create mode 100644 src/registrar/assets/src/js/getgov-admin/analytics.js diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js deleted file mode 100644 index 54cdc789a..000000000 --- a/src/registrar/assets/js/get-gov-reports.js +++ /dev/null @@ -1,203 +0,0 @@ - -/** An IIFE for admin in DjangoAdmin to listen to clicks on the growth report export button, - * attach the seleted start and end dates to a url that'll trigger the view, and finally - * redirect to that url. - * - * This function also sets the start and end dates to match the url params if they exist -*/ -(function () { - // Function to get URL parameter value by name - function getParameterByName(name, url) { - if (!url) url = window.location.href; - name = name.replace(/[\[\]]/g, '\\$&'); - var regex = new RegExp('[?&]' + name + '(=([^&#]*)|&|#|$)'), - results = regex.exec(url); - if (!results) return null; - if (!results[2]) return ''; - return decodeURIComponent(results[2].replace(/\+/g, ' ')); - } - - // Get the current date in the format YYYY-MM-DD - let currentDate = new Date().toISOString().split('T')[0]; - - // Default the value of the start date input field to the current date - let startDateInput = document.getElementById('start'); - - // Default the value of the end date input field to the current date - let endDateInput = document.getElementById('end'); - - let exportButtons = document.querySelectorAll('.exportLink'); - - if (exportButtons.length > 0) { - // Check if start and end dates are present in the URL - let urlStartDate = getParameterByName('start_date'); - let urlEndDate = getParameterByName('end_date'); - - // Set input values based on URL parameters or current date - startDateInput.value = urlStartDate || currentDate; - endDateInput.value = urlEndDate || currentDate; - - exportButtons.forEach((btn) => { - btn.addEventListener('click', function () { - // Get the selected start and end dates - let startDate = startDateInput.value; - let endDate = endDateInput.value; - let exportUrl = btn.dataset.exportUrl; - - // Build the URL with parameters - exportUrl += "?start_date=" + startDate + "&end_date=" + endDate; - - // Redirect to the export URL - window.location.href = exportUrl; - }); - }); - } - -})(); - - -/** An IIFE to initialize the analytics page -*/ -(function () { - const chartInstances = new Map(); - - /** - * Creates a diagonal stripe pattern for chart.js - * Inspired by https://stackoverflow.com/questions/28569667/fill-chart-js-bar-chart-with-diagonal-stripes-or-other-patterns - * and https://github.com/ashiguruma/patternomaly - * @param {string} backgroundColor - Background color of the pattern - * @param {string} [lineColor="white"] - Color of the diagonal lines - * @param {boolean} [rightToLeft=false] - Direction of the diagonal lines - * @param {number} [lineGap=1] - Gap between lines - * @returns {CanvasPattern} A canvas pattern object for use with backgroundColor - */ - function createDiagonalPattern(backgroundColor, lineColor, rightToLeft=false, lineGap=1) { - // Define the canvas and the 2d context so we can draw on it - let shape = document.createElement("canvas"); - shape.width = 20; - shape.height = 20; - let context = shape.getContext("2d"); - - // Fill with specified background color - context.fillStyle = backgroundColor; - context.fillRect(0, 0, shape.width, shape.height); - - // Set stroke properties - context.strokeStyle = lineColor; - context.lineWidth = 2; - - // Rotate canvas for a right-to-left pattern - if (rightToLeft) { - context.translate(shape.width, 0); - context.rotate(90 * Math.PI / 180); - }; - - // First diagonal line - let halfSize = shape.width / 2; - context.moveTo(halfSize - lineGap, -lineGap); - context.lineTo(shape.width + lineGap, halfSize + lineGap); - - // Second diagonal line (x,y are swapped) - context.moveTo(-lineGap, halfSize - lineGap); - context.lineTo(halfSize + lineGap, shape.width + lineGap); - - context.stroke(); - return context.createPattern(shape, "repeat"); - } - - function createComparativeColumnChart(canvasId, title, labelOne, labelTwo) { - var canvas = document.getElementById(canvasId); - if (!canvas) { - return - } - - var ctx = canvas.getContext("2d"); - - var listOne = JSON.parse(canvas.getAttribute('data-list-one')); - var listTwo = JSON.parse(canvas.getAttribute('data-list-two')); - - var data = { - labels: ["Total", "Federal", "Interstate", "State/Territory", "Tribal", "County", "City", "Special District", "School District", "Election Board"], - datasets: [ - { - label: labelOne, - backgroundColor: "rgba(255, 99, 132, 0.3)", - borderColor: "rgba(255, 99, 132, 1)", - borderWidth: 1, - data: listOne, - // Set this line style to be rightToLeft for visual distinction - backgroundColor: createDiagonalPattern('rgba(255, 99, 132, 0.3)', 'white', true) - }, - { - label: labelTwo, - backgroundColor: "rgba(75, 192, 192, 0.3)", - borderColor: "rgba(75, 192, 192, 1)", - borderWidth: 1, - data: listTwo, - backgroundColor: createDiagonalPattern('rgba(75, 192, 192, 0.3)', 'white') - }, - ], - }; - - var options = { - responsive: true, - maintainAspectRatio: false, - plugins: { - legend: { - position: 'top', - }, - title: { - display: true, - text: title - } - }, - scales: { - y: { - beginAtZero: true, - }, - }, - }; - - if (chartInstances.has(canvasId)) { - chartInstances.get(canvasId).destroy(); - } - - const chart = new Chart(ctx, { - type: "bar", - data: data, - options: options, - }); - - chartInstances.set(canvasId, chart); - } - - function handleResize() { - // Debounce the resize handler - if (handleResize.timeout) { - clearTimeout(handleResize.timeout); - } - - handleResize.timeout = setTimeout(() => { - chartInstances.forEach((chart, canvasId) => { - if (chart && chart.canvas) { - chart.resize(); - } - }); - }, 100); - } - - function initComparativeColumnCharts() { - document.addEventListener("DOMContentLoaded", function () { - createComparativeColumnChart("myChart1", "Managed domains", "Start Date", "End Date"); - createComparativeColumnChart("myChart2", "Unmanaged domains", "Start Date", "End Date"); - createComparativeColumnChart("myChart3", "Deleted domains", "Start Date", "End Date"); - createComparativeColumnChart("myChart4", "Ready domains", "Start Date", "End Date"); - createComparativeColumnChart("myChart5", "Submitted requests", "Start Date", "End Date"); - createComparativeColumnChart("myChart6", "All requests", "Start Date", "End Date"); - - window.addEventListener("resize", handleResize); - }); - }; - - initComparativeColumnCharts(); -})(); diff --git a/src/registrar/assets/src/js/getgov-admin/analytics.js b/src/registrar/assets/src/js/getgov-admin/analytics.js new file mode 100644 index 000000000..d2808f623 --- /dev/null +++ b/src/registrar/assets/src/js/getgov-admin/analytics.js @@ -0,0 +1,196 @@ + +import { debounce } from '../getgov/helpers.js'; +import { getParameterByName } from './helpers-admin.js'; + + +/** This function also sets the start and end dates to match the url params if they exist +*/ +function initAnalyticsExportButtons() { + // Get the current date in the format YYYY-MM-DD + let currentDate = new Date().toISOString().split('T')[0]; + + // Default the value of the start date input field to the current date + let startDateInput = document.getElementById('start'); + + // Default the value of the end date input field to the current date + let endDateInput = document.getElementById('end'); + + let exportButtons = document.querySelectorAll('.exportLink'); + + if (exportButtons.length > 0) { + // Check if start and end dates are present in the URL + let urlStartDate = getParameterByName('start_date'); + let urlEndDate = getParameterByName('end_date'); + + // Set input values based on URL parameters or current date + startDateInput.value = urlStartDate || currentDate; + endDateInput.value = urlEndDate || currentDate; + + exportButtons.forEach((btn) => { + btn.addEventListener('click', function () { + // Get the selected start and end dates + let startDate = startDateInput.value; + let endDate = endDateInput.value; + let exportUrl = btn.dataset.exportUrl; + + // Build the URL with parameters + exportUrl += "?start_date=" + startDate + "&end_date=" + endDate; + + // Redirect to the export URL + window.location.href = exportUrl; + }); + }); + } + +}; + +/** + * Creates a diagonal stripe pattern for chart.js + * Inspired by https://stackoverflow.com/questions/28569667/fill-chart-js-bar-chart-with-diagonal-stripes-or-other-patterns + * and https://github.com/ashiguruma/patternomaly + * @param {string} backgroundColor - Background color of the pattern + * @param {string} [lineColor="white"] - Color of the diagonal lines + * @param {boolean} [rightToLeft=false] - Direction of the diagonal lines + * @param {number} [lineGap=1] - Gap between lines + * @returns {CanvasPattern} A canvas pattern object for use with backgroundColor + */ +function createDiagonalPattern(backgroundColor, lineColor, rightToLeft=false, lineGap=1) { + // Define the canvas and the 2d context so we can draw on it + let shape = document.createElement("canvas"); + shape.width = 20; + shape.height = 20; + let context = shape.getContext("2d"); + + // Fill with specified background color + context.fillStyle = backgroundColor; + context.fillRect(0, 0, shape.width, shape.height); + + // Set stroke properties + context.strokeStyle = lineColor; + context.lineWidth = 2; + + // Rotate canvas for a right-to-left pattern + if (rightToLeft) { + context.translate(shape.width, 0); + context.rotate(90 * Math.PI / 180); + }; + + // First diagonal line + let halfSize = shape.width / 2; + context.moveTo(halfSize - lineGap, -lineGap); + context.lineTo(shape.width + lineGap, halfSize + lineGap); + + // Second diagonal line (x,y are swapped) + context.moveTo(-lineGap, halfSize - lineGap); + context.lineTo(halfSize + lineGap, shape.width + lineGap); + + context.stroke(); + return context.createPattern(shape, "repeat"); +} + +function createComparativeColumnChart(canvasId, title, labelOne, labelTwo, chartInstances) { + var canvas = document.getElementById(canvasId); + if (!canvas) { + return + } + + var ctx = canvas.getContext("2d"); + + var listOne = JSON.parse(canvas.getAttribute('data-list-one')); + var listTwo = JSON.parse(canvas.getAttribute('data-list-two')); + + var data = { + labels: ["Total", "Federal", "Interstate", "State/Territory", "Tribal", "County", "City", "Special District", "School District", "Election Board"], + datasets: [ + { + label: labelOne, + backgroundColor: "rgba(255, 99, 132, 0.3)", + borderColor: "rgba(255, 99, 132, 1)", + borderWidth: 1, + data: listOne, + // Set this line style to be rightToLeft for visual distinction + backgroundColor: createDiagonalPattern('rgba(255, 99, 132, 0.3)', 'white', true) + }, + { + label: labelTwo, + backgroundColor: "rgba(75, 192, 192, 0.3)", + borderColor: "rgba(75, 192, 192, 1)", + borderWidth: 1, + data: listTwo, + backgroundColor: createDiagonalPattern('rgba(75, 192, 192, 0.3)', 'white') + }, + ], + }; + + var options = { + responsive: true, + maintainAspectRatio: false, + plugins: { + legend: { + position: 'top', + }, + title: { + display: true, + text: title + } + }, + scales: { + y: { + beginAtZero: true, + }, + }, + }; + + if (chartInstances.has(canvasId)) { + chartInstances.get(canvasId).destroy(); + } + + const chart = new Chart(ctx, { + type: "bar", + data: data, + options: options, + }); + + chartInstances.set(canvasId, chart); +} + +function initComparativeColumnCharts(chartInstances) { + // Create charts + const charts = [ + { id: "managed-domains-chart", title: "Managed domains" }, + { id: "unmanaged-domains-chart", title: "Unmanaged domains" }, + { id: "deleted-domains-chart", title: "Deleted domains" }, + { id: "ready-domains-chart", title: "Ready domains" }, + { id: "submitted-requests-chart", title: "Submitted requests" }, + { id: "all-requests-chart", title: "All requests" } + ]; + charts.forEach(chart => { + createComparativeColumnChart( + chart.id, + chart.title, + "Start Date", + "End Date", + chartInstances + ); + }); + + // Add resize listener to each chart + window.addEventListener("resize", debounce(() => { + chartInstances.forEach((chart) => { + if (chart?.canvas) chart.resize(); + }); + }, 200)); +}; + +/** An IIFE to initialize the analytics page +*/ +export function initAnalyticsDashboard() { + const chartInstances = new Map(); + const analyticsPageContainer = document.querySelector('.analytics-dashboard .analytics-dashboard-charts'); + if (analyticsPageContainer) { + document.addEventListener("DOMContentLoaded", function () { + initAnalyticsExportButtons(); + initComparativeColumnCharts(chartInstances); + }); + } +}; diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-admin.js b/src/registrar/assets/src/js/getgov-admin/helpers-admin.js index ff618a67d..8055e29d3 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-admin.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-admin.js @@ -22,3 +22,13 @@ export function addOrRemoveSessionBoolean(name, add){ sessionStorage.removeItem(name); } } + +export function getParameterByName(name, url) { + if (!url) url = window.location.href; + name = name.replace(/[\[\]]/g, '\\$&'); + var regex = new RegExp('[?&]' + name + '(=([^&#]*)|&|#|$)'), + results = regex.exec(url); + if (!results) return null; + if (!results[2]) return ''; + return decodeURIComponent(results[2].replace(/\+/g, ' ')); +} diff --git a/src/registrar/assets/src/js/getgov-admin/main.js b/src/registrar/assets/src/js/getgov-admin/main.js index 64be572b2..5c6de20ab 100644 --- a/src/registrar/assets/src/js/getgov-admin/main.js +++ b/src/registrar/assets/src/js/getgov-admin/main.js @@ -15,6 +15,7 @@ import { initDomainFormTargetBlankButtons } from './domain-form.js'; import { initDynamicPortfolioFields } from './portfolio-form.js'; import { initDynamicDomainInformationFields } from './domain-information-form.js'; import { initDynamicDomainFields } from './domain-form.js'; +import { initAnalyticsDashboard } from './analytics.js'; // General initModals(); @@ -41,3 +42,6 @@ initDynamicPortfolioFields(); // Domain information initDynamicDomainInformationFields(); + +// Analytics dashboard +initAnalyticsDashboard(); diff --git a/src/registrar/templates/admin/analytics.html b/src/registrar/templates/admin/analytics.html index ca3501eec..d345aeb14 100644 --- a/src/registrar/templates/admin/analytics.html +++ b/src/registrar/templates/admin/analytics.html @@ -18,7 +18,7 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/ {% block content %} -
    +
    @@ -136,7 +136,7 @@ https://github.com/django/django/blob/main/django/contrib/admin/templates/admin/
    {% comment %} Managed/Unmanaged domains {% endcomment %}
    -
    - -
    - -
    - - {% endblock %} From b3a3dcad6dcfe15331f3da2dba69af58c450dce0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:31:01 -0700 Subject: [PATCH 098/226] Cleanup + code simplification --- .../assets/src/js/getgov-admin/analytics.js | 67 +++++++------------ .../admin/analytics_graph_table.html | 2 +- 2 files changed, 27 insertions(+), 42 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/analytics.js b/src/registrar/assets/src/js/getgov-admin/analytics.js index d2808f623..e2de7b247 100644 --- a/src/registrar/assets/src/js/getgov-admin/analytics.js +++ b/src/registrar/assets/src/js/getgov-admin/analytics.js @@ -41,7 +41,6 @@ function initAnalyticsExportButtons() { }); }); } - }; /** @@ -88,8 +87,8 @@ function createDiagonalPattern(backgroundColor, lineColor, rightToLeft=false, li return context.createPattern(shape, "repeat"); } -function createComparativeColumnChart(canvasId, title, labelOne, labelTwo, chartInstances) { - var canvas = document.getElementById(canvasId); +function createComparativeColumnChart(id, title, labelOne, labelTwo) { + var canvas = document.getElementById(id); if (!canvas) { return } @@ -140,57 +139,43 @@ function createComparativeColumnChart(canvasId, title, labelOne, labelTwo, chart }, }, }; - - if (chartInstances.has(canvasId)) { - chartInstances.get(canvasId).destroy(); - } - - const chart = new Chart(ctx, { + return new Chart(ctx, { type: "bar", data: data, options: options, }); - - chartInstances.set(canvasId, chart); } -function initComparativeColumnCharts(chartInstances) { - // Create charts - const charts = [ - { id: "managed-domains-chart", title: "Managed domains" }, - { id: "unmanaged-domains-chart", title: "Unmanaged domains" }, - { id: "deleted-domains-chart", title: "Deleted domains" }, - { id: "ready-domains-chart", title: "Ready domains" }, - { id: "submitted-requests-chart", title: "Submitted requests" }, - { id: "all-requests-chart", title: "All requests" } - ]; - charts.forEach(chart => { - createComparativeColumnChart( - chart.id, - chart.title, - "Start Date", - "End Date", - chartInstances - ); - }); - - // Add resize listener to each chart - window.addEventListener("resize", debounce(() => { - chartInstances.forEach((chart) => { - if (chart?.canvas) chart.resize(); - }); - }, 200)); -}; - /** An IIFE to initialize the analytics page */ export function initAnalyticsDashboard() { - const chartInstances = new Map(); const analyticsPageContainer = document.querySelector('.analytics-dashboard .analytics-dashboard-charts'); if (analyticsPageContainer) { document.addEventListener("DOMContentLoaded", function () { initAnalyticsExportButtons(); - initComparativeColumnCharts(chartInstances); + + // Create charts and store each instance of it + const chartInstances = new Map(); + const charts = [ + { id: "managed-domains-chart", title: "Managed domains" }, + { id: "unmanaged-domains-chart", title: "Unmanaged domains" }, + { id: "deleted-domains-chart", title: "Deleted domains" }, + { id: "ready-domains-chart", title: "Ready domains" }, + { id: "submitted-requests-chart", title: "Submitted requests" }, + { id: "all-requests-chart", title: "All requests" } + ]; + charts.forEach(chart => { + if (chartInstances.has(chart.id)) chartInstances.get(chart.id).destroy(); + let chart = createComparativeColumnChart(...chart, "Start Date", "End Date"); + chartInstances.set(chart.id, chart); + }); + + // Add resize listener to each chart + window.addEventListener("resize", debounce(() => { + chartInstances.forEach((chart) => { + if (chart?.canvas) chart.resize(); + }); + }, 200)); }); } }; diff --git a/src/registrar/templates/admin/analytics_graph_table.html b/src/registrar/templates/admin/analytics_graph_table.html index 88b538745..5f10da93a 100644 --- a/src/registrar/templates/admin/analytics_graph_table.html +++ b/src/registrar/templates/admin/analytics_graph_table.html @@ -23,4 +23,4 @@ {% endfor %} {% endwith %}
    \ No newline at end of file + From 62a8a0c2a1e5fbd6cc72de262d0267c566a89681 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:34:38 -0700 Subject: [PATCH 099/226] Update analytics.js --- src/registrar/assets/src/js/getgov-admin/analytics.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/analytics.js b/src/registrar/assets/src/js/getgov-admin/analytics.js index e2de7b247..7bf6a05b8 100644 --- a/src/registrar/assets/src/js/getgov-admin/analytics.js +++ b/src/registrar/assets/src/js/getgov-admin/analytics.js @@ -1,8 +1,6 @@ - import { debounce } from '../getgov/helpers.js'; import { getParameterByName } from './helpers-admin.js'; - /** This function also sets the start and end dates to match the url params if they exist */ function initAnalyticsExportButtons() { @@ -94,7 +92,6 @@ function createComparativeColumnChart(id, title, labelOne, labelTwo) { } var ctx = canvas.getContext("2d"); - var listOne = JSON.parse(canvas.getAttribute('data-list-one')); var listTwo = JSON.parse(canvas.getAttribute('data-list-two')); From 8a0fdc7ea637053c532d387d58c0afef0698551b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:44:22 -0700 Subject: [PATCH 100/226] Update test_reports.py --- src/registrar/tests/test_reports.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index bab4f327b..18c98807d 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -1,4 +1,5 @@ import io +from unittest import skip from django.test import Client, RequestFactory from io import StringIO from registrar.models import ( @@ -819,6 +820,7 @@ class MemberExportTest(MockDbForIndividualTests, MockEppLib): super().setUp() self.factory = RequestFactory() + @skip("flaky test that needs to be refactored") @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @less_console_noise_decorator From 8acb36ef9ad15989608ab98dc15ba355f2f53761 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:46:14 -0700 Subject: [PATCH 101/226] remove unrelated changes --- src/.pa11yci | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/.pa11yci b/src/.pa11yci index ed382a38a..571d0b1c8 100644 --- a/src/.pa11yci +++ b/src/.pa11yci @@ -8,20 +8,20 @@ "http://localhost:8080/health/", "http://localhost:8080/request/", "http://localhost:8080/request/start", - "http://localhost:8080/request/1/organization/", - "http://localhost:8080/request/1/org_federal/", - "http://localhost:8080/request/1/org_election/", - "http://localhost:8080/request/1/org_contact/", - "http://localhost:8080/request/1/senior_official/", - "http://localhost:8080/request/1/current_sites/", - "http://localhost:8080/request/1/dotgov_domain/", - "http://localhost:8080/request/1/purpose/", - "http://localhost:8080/request/1/your_contact/", - "http://localhost:8080/request/1/other_contacts/", - "http://localhost:8080/request/1/anything_else/", - "http://localhost:8080/request/1/requirements/", - "http://localhost:8080/request/1/finished/", - "http://localhost:8080/request/1/requesting_entity/", + "http://localhost:8080/request/organization/", + "http://localhost:8080/request/org_federal/", + "http://localhost:8080/request/org_election/", + "http://localhost:8080/request/org_contact/", + "http://localhost:8080/request/senior_official/", + "http://localhost:8080/request/current_sites/", + "http://localhost:8080/request/dotgov_domain/", + "http://localhost:8080/request/purpose/", + "http://localhost:8080/request/your_contact/", + "http://localhost:8080/request/other_contacts/", + "http://localhost:8080/request/anything_else/", + "http://localhost:8080/request/requirements/", + "http://localhost:8080/request/finished/", + "http://localhost:8080/request/requesting_entity/", "http://localhost:8080/user-profile/", "http://localhost:8080/members/", "http://localhost:8080/members/new-member" From c540a324c7df33c7011fcef4bebe158a43bf417a Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 12 Feb 2025 10:11:25 -0800 Subject: [PATCH 102/226] 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 0454e7295136eedc10a3190f09bc90b95ca899f2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 12 Feb 2025 11:23:37 -0700 Subject: [PATCH 103/226] fix typo --- src/registrar/assets/src/js/getgov-admin/analytics.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/assets/src/js/getgov-admin/analytics.js b/src/registrar/assets/src/js/getgov-admin/analytics.js index 7bf6a05b8..a5488fa3d 100644 --- a/src/registrar/assets/src/js/getgov-admin/analytics.js +++ b/src/registrar/assets/src/js/getgov-admin/analytics.js @@ -163,8 +163,7 @@ export function initAnalyticsDashboard() { ]; charts.forEach(chart => { if (chartInstances.has(chart.id)) chartInstances.get(chart.id).destroy(); - let chart = createComparativeColumnChart(...chart, "Start Date", "End Date"); - chartInstances.set(chart.id, chart); + chartInstances.set(chart.id, createComparativeColumnChart(...chart, "Start Date", "End Date")); }); // Add resize listener to each chart From 0725ab8e59e59ac63267fe1d008998d7fd7ba26c Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 12 Feb 2025 10:31:53 -0800 Subject: [PATCH 104/226] 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 105/226] 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 106/226] 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 76b72d19627af77fe21e27c7c34597d82aaf0250 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 12 Feb 2025 15:49:12 -0500 Subject: [PATCH 107/226] kebob --- .../assets/src/js/getgov/portfolio-member-page.js | 2 +- src/registrar/assets/src/js/getgov/table-base.js | 6 +++--- .../assets/src/sass/_theme/_accordions.scss | 12 +++++++++++- src/registrar/templates/portfolio_member.html | 10 ++++++++++ 4 files changed, 25 insertions(+), 5 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 95723fc7e..602fbc65e 100644 --- a/src/registrar/assets/src/js/getgov/portfolio-member-page.js +++ b/src/registrar/assets/src/js/getgov/portfolio-member-page.js @@ -18,7 +18,7 @@ export function initPortfolioNewMemberPageToggle() { const unique_id = `${member_type}-${member_id}`; let cancelInvitationButton = member_type === "invitedmember" ? "Cancel invitation" : "Remove member"; - wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `More Options for ${member_name}`); + wrapperDeleteAction.innerHTML = generateKebabHTML('remove-member', unique_id, cancelInvitationButton, `More Options for ${member_name}`, "usa-icon--large"); // This easter egg is only for fixtures that dont have names as we are displaying their emails // All prod users will have emails linked to their account diff --git a/src/registrar/assets/src/js/getgov/table-base.js b/src/registrar/assets/src/js/getgov/table-base.js index ce4397887..f36fbc7b1 100644 --- a/src/registrar/assets/src/js/getgov/table-base.js +++ b/src/registrar/assets/src/js/getgov/table-base.js @@ -79,7 +79,7 @@ export function addModal(id, ariaLabelledby, ariaDescribedby, modalHeading, moda * @param {string} modal_button_text - The action button's text * @param {string} screen_reader_text - A screen reader helper */ -export function generateKebabHTML(action, unique_id, modal_button_text, screen_reader_text) { +export function generateKebabHTML(action, unique_id, modal_button_text, screen_reader_text, icon_class) { const generateModalButton = (mobileOnly = false) => ` -