From aea8539d66559f2b636c995a27421bdb33e7d692 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 14 Aug 2024 11:18:47 -0600 Subject: [PATCH 01/78] Initial draft logic for updating action needed email behavior --- src/registrar/admin.py | 2 +- src/registrar/assets/js/get-gov-admin.js | 60 +++++++++++++------ .../admin/includes/detail_table_fieldset.html | 29 ++++++--- 3 files changed, 63 insertions(+), 28 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ca4038d51..2cd391435 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -223,7 +223,7 @@ class DomainRequestAdminForm(forms.ModelForm): "other_contacts": NoAutocompleteFilteredSelectMultiple("other_contacts", False), } labels = { - "action_needed_reason_email": "Auto-generated email", + "action_needed_reason_email": "E-mail", } def __init__(self, *args, **kwargs): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 04f5417b0..c8fd45bd0 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -353,7 +353,7 @@ function initializeWidgetOnList(list, parentId) { let rejectionReasonFormGroup = document.querySelector('.field-rejection_reason') // This is the "action needed reason" field let actionNeededReasonFormGroup = document.querySelector('.field-action_needed_reason'); - // This is the "auto-generated email" field + // This is the "Email" field let actionNeededReasonEmailFormGroup = document.querySelector('.field-action_needed_reason_email') if (rejectionReasonFormGroup && actionNeededReasonFormGroup && actionNeededReasonEmailFormGroup) { @@ -510,7 +510,10 @@ function initializeWidgetOnList(list, parentId) { // Since this is an iife, these vars will be removed from memory afterwards var actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); var actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); - var readonlyView = document.querySelector("#action-needed-reason-email-readonly"); + var helptext = document.querySelector("#action-needed-reason-email-placeholder-text") + var actionNeededEmailReadonly = document.querySelector("#action-needed-reason-email-readonly"); + var actionNeededEmailReadonlyTextarea = document.querySelector("#action-needed-reason-email-readonly-textarea") + var editEmailButton = document.querySelector("#action-needed-reason-email-edit-button") let emailWasSent = document.getElementById("action-needed-email-sent"); let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; @@ -536,6 +539,21 @@ function initializeWidgetOnList(list, parentId) { updateActionNeededEmailDisplay(reason) }); + editEmailButton.addEventListener("click", function() { + let emailHasBeenSentBefore = sessionStorage.getItem(emailSentSessionVariableName) !== null; + + if (emailHasBeenSentBefore) { + // Show warning Modal + + } + else { + // Show editable view + showElement(actionNeededEmail.parentElement) + hideElement(actionNeededEmailReadonly) + hideElement(helptext) + } + }); + // Add a change listener to the action needed reason dropdown actionNeededReasonDropdown.addEventListener("change", function() { let reason = actionNeededReasonDropdown.value; @@ -543,6 +561,7 @@ function initializeWidgetOnList(list, parentId) { if (reason && emailBody) { // Replace the email content actionNeededEmail.value = emailBody; + actionNeededEmailReadonlyTextarea.value = emailBody; // Reset the session object on change since change refreshes the email content. if (oldDropdownValue !== actionNeededReasonDropdown.value || oldEmailValue !== actionNeededEmail.value) { @@ -562,27 +581,30 @@ function initializeWidgetOnList(list, parentId) { // If the email doesn't exist or if we're of reason "other", display that no email was sent. // Likewise, if we've sent this email before, we should just display the content. function updateActionNeededEmailDisplay(reason) { - let emailHasBeenSentBefore = sessionStorage.getItem(emailSentSessionVariableName) !== null; - let collapseableDiv = readonlyView.querySelector(".collapse--dgsimple"); - let showMoreButton = document.querySelector("#action_needed_reason_email__show_details"); - if ((reason && reason != "other") && !emailHasBeenSentBefore) { - showElement(actionNeededEmail.parentElement) - hideElement(readonlyView) - hideElement(showMoreButton) - } else { - if (!reason || reason === "other") { - collapseableDiv.innerHTML = reason ? "No email will be sent." : "-"; - hideElement(showMoreButton) - if (collapseableDiv.classList.contains("collapsed")) { - showMoreButton.click() - } - }else { - showElement(showMoreButton) + + console.info("REASON: "+reason) + + if (reason) { + if (reason === "other") { + helptext.innerHTML = "No email will be sent."; + hideElement(actionNeededEmail.parentElement) + hideElement(actionNeededEmailReadonly) + showElement(helptext) } + else { + // Always show readonly view to start + hideElement(actionNeededEmail.parentElement) + showElement(actionNeededEmailReadonly) + hideElement(helptext) + } + } else { + helptext.innerHTML = "Select an action needed reason to see email"; hideElement(actionNeededEmail.parentElement) - showElement(readonlyView) + hideElement(actionNeededEmailReadonly) + showElement(helptext) } } + })(); diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 067b69c07..bcf5fd770 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -145,16 +145,29 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block field_other %} {% if field.field.name == "action_needed_reason_email" %} -
- {% else %} {{ field.field }} From 76fef5303b1e24460fb1a282fe7662b4c38f5df6 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 21 Aug 2024 18:41:14 -0400 Subject: [PATCH 46/78] removed pseudo apostrophe --- src/registrar/tests/test_views_portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 01cd84743..42cf82fa4 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -504,7 +504,7 @@ class TestPortfolio(WebTest): self.assertFalse(self.user.has_domains_portfolio_permission(response.wsgi_request.session.get("portfolio"))) self.assertEqual(response.status_code, 200) - self.assertContains(response, "You aren't managing any domains.") + self.assertContains(response, "You aren") # Test the domains page - this user should not have access response = self.client.get(reverse("domains")) From bef49a85c16de5fc9f2640a9c2ee3f1c37e25478 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 22 Aug 2024 08:33:03 -0600 Subject: [PATCH 47/78] Cleanup --- .../models/user_portfolio_permission.py | 10 +++++----- src/registrar/tests/test_models.py | 16 ---------------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index abbd20afb..c2cdc61ad 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -100,12 +100,12 @@ class UserPortfolioPermission(TimeStampedModel): # Check if a user is set without accessing the related object. has_user = bool(self.user_id) - # Have to create a bogus request to set the user and pass to flag_is_active - request = HttpRequest() - request.user = self.user - if not flag_is_active(request, "multiple_portfolios") and self.pk is None and has_user: + if self.pk is None and has_user: + # Have to create a bogus request to set the user and pass to flag_is_active + request = HttpRequest() + request.user = self.user existing_permissions = UserPortfolioPermission.objects.filter(user=self.user) - if existing_permissions.exists(): + if not flag_is_active(request, "multiple_portfolios") and existing_permissions.exists(): raise ValidationError( "Only one portfolio permission is allowed per user when multiple portfolios are disabled." ) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 227548b61..9f55fced1 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1483,10 +1483,6 @@ class TestUser(TestCase): portfolio, _ = Portfolio.objects.get_or_create(creator=self.user, organization_name="Hotel California") - # TODO - uncomment this when we just pass request to these functions - # request = get_wsgi_request_object(self.client, self.user) - # user_can_view_all_domains = self.user.has_domains_portfolio_permission(request) - # user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(request) user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(portfolio) @@ -1499,10 +1495,6 @@ class TestUser(TestCase): additional_permissions=[UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS], ) - # TODO - uncomment this when we just pass request to these functions - # request = get_wsgi_request_object(self.client, self.user) - # user_can_view_all_domains = self.user.has_domains_portfolio_permission(request) - # user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(request) user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(portfolio) @@ -1513,10 +1505,6 @@ class TestUser(TestCase): portfolio_permission.save() portfolio_permission.refresh_from_db() - # TODO - uncomment this when we just pass request to these functions - # request = get_wsgi_request_object(self.client, self.user) - # user_can_view_all_domains = self.user.has_domains_portfolio_permission(request) - # user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(request) user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(portfolio) @@ -1525,10 +1513,6 @@ class TestUser(TestCase): UserDomainRole.objects.get_or_create(user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER) - # TODO - uncomment this when we just pass request to these functions - # request = get_wsgi_request_object(self.client, self.user) - # user_can_view_all_domains = self.user.has_domains_portfolio_permission(request) - # user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(request) user_can_view_all_domains = self.user.has_domains_portfolio_permission(portfolio) user_can_view_all_requests = self.user.has_domain_requests_portfolio_permission(portfolio) From e59d4738e388ea2bad4a73ab99cdee68bbad475c Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 22 Aug 2024 16:44:25 -0400 Subject: [PATCH 48/78] filtering by portfolios --- src/registrar/admin.py | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 3ad5e3ea0..4be9c375d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -34,6 +34,7 @@ from django_fsm import TransitionNotAllowed # type: ignore from django.utils.safestring import mark_safe from django.utils.html import escape from django.contrib.auth.forms import UserChangeForm, UsernameField +from django.contrib.admin.views.main import ChangeList, IGNORED_PARAMS from django_admin_multiple_choice_list_filter.list_filters import MultipleChoiceListFilter from import_export import resources from import_export.admin import ImportExportModelAdmin @@ -45,6 +46,27 @@ from django.utils.translation import gettext_lazy as _ logger = logging.getLogger(__name__) +class CustomChangeListForPortfolioFiltering(ChangeList): + """CustomChangeList so that portfolio can be passed in a url, but not appear + in the list of filters on the right side of the page.""" + + def get_filters_params(self, params=None): + """ + Return all params except IGNORED_PARAMS. + """ + params = params or self.params + lookup_params = params.copy() # a dictionary of the query string + # Remove all the parameters that are globally and systematically + # ignored. + # Remove portfolio so that it does not error as an invalid + # filter parameter. + ignored_params = list(IGNORED_PARAMS) + ['portfolio'] + for ignored in ignored_params: + if ignored in lookup_params: + del lookup_params[ignored] + return lookup_params + + class FsmModelResource(resources.ModelResource): """ModelResource is extended to support importing of tables which have FSMFields. ModelResource is extended with the following changes @@ -644,6 +666,19 @@ class ListHeaderAdmin(AuditedAdmin, OrderableFieldsMixin): ) except models.User.DoesNotExist: pass + elif parameter_name == "portfolio": + # Retrieves the corresponding portfolio from Portfolio + id_value = request.GET.get(param) + try: + portfolio = models.Portfolio.objects.get(id=id_value) + filters.append( + { + "parameter_name": "portfolio", + "parameter_value": portfolio.organization_name, + } + ) + except models.Portfolio.DoesNotExist: + pass else: # For other parameter names, append a dictionary with the original # parameter_name and the corresponding parameter_value @@ -2235,6 +2270,23 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # objects rather than Contact objects. use_sort = db_field.name != "senior_official" return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) + + def get_queryset(self, request): + """Custom get_queryset to filter by portfolio if portfolio is in the + request params.""" + qs = super().get_queryset(request) + # Check if a 'portfolio' parameter is passed in the request + portfolio_id = request.GET.get('portfolio') + if portfolio_id: + # Further filter the queryset by the portfolio + qs = qs.filter(DomainRequest_info__portfolio=portfolio_id) + return qs + + def get_changelist(self, request, **kwargs): + """ + Return the ChangeList class for use on the changelist page. + """ + return CustomChangeListForPortfolioFiltering class TransitionDomainAdmin(ListHeaderAdmin): @@ -2688,6 +2740,23 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): ): return True return super().has_change_permission(request, obj) + + def get_queryset(self, request): + """Custom get_queryset to filter by portfolio if portfolio is in the + request params.""" + qs = super().get_queryset(request) + # Check if a 'portfolio' parameter is passed in the request + portfolio_id = request.GET.get('portfolio') + if portfolio_id: + # Further filter the queryset by the portfolio + qs = qs.filter(domain_info__portfolio=portfolio_id) + return qs + + def get_changelist(self, request, **kwargs): + """ + Return the ChangeList class for use on the changelist page. + """ + return CustomChangeListForPortfolioFiltering class DraftDomainResource(resources.ModelResource): From 6286ae96be749deffd0176eaa180f9f61a2365c6 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 22 Aug 2024 17:24:33 -0400 Subject: [PATCH 49/78] updated portfolio changeform view in admin --- src/registrar/admin.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 4be9c375d..1e20523cc 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2279,7 +2279,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): portfolio_id = request.GET.get('portfolio') if portfolio_id: # Further filter the queryset by the portfolio - qs = qs.filter(DomainRequest_info__portfolio=portfolio_id) + qs = qs.filter(portfolio=portfolio_id) return qs def get_changelist(self, request, **kwargs): @@ -2942,7 +2942,7 @@ class PortfolioAdmin(ListHeaderAdmin): # "classes": ("collapse", "closed"), # "fields": ["administrators", "members"]} # ), - ("Portfolio domains", {"classes": ("collapse", "closed"), "fields": ["domains", "domain_requests"]}), + ("Portfolio domains", {"fields": ["domains", "domain_requests"]}), ("Type of organization", {"fields": ["organization_type", "federal_type"]}), ( "Organization name and mailing address", @@ -3022,21 +3022,30 @@ class PortfolioAdmin(ListHeaderAdmin): return self.get_field_links_as_list(queryset, "suborganization") suborganizations.short_description = "Suborganizations" # type: ignore - + def domains(self, obj: models.Portfolio): - """Returns a list of links for each related domain""" - queryset = obj.get_domains() - return self.get_field_links_as_list( - queryset, "domaininformation", link_info_attribute="get_state_display_of_domain" - ) + """Returns the count of domains with a link to view them in the admin.""" + domain_count = obj.get_domains().count() # Count the related domains + if domain_count > 0: + # Construct the URL to the admin page, filtered by portfolio + url = reverse("admin:registrar_domain_changelist") + f"?portfolio={obj.id}" + label = "Domain" if domain_count == 1 else "Domains" + # Create a clickable link with the domain count + return format_html('{} {}', url, domain_count, label) + return "No Domains" domains.short_description = "Domains" # type: ignore def domain_requests(self, obj: models.Portfolio): - """Returns a list of links for each related domain request""" - queryset = obj.get_domain_requests() - return self.get_field_links_as_list(queryset, "domainrequest", link_info_attribute="get_status_display") - + """Returns the count of domain requests with a link to view them in the admin.""" + domain_request_count = obj.get_domain_requests().count() # Count the related domain requests + if domain_request_count > 0: + # Construct the URL to the admin page, filtered by portfolio + url = reverse("admin:registrar_domainrequest_changelist") + f"?portfolio={obj.id}" + # Create a clickable link with the domain request count + return format_html('{} Domain Requests', url, domain_request_count) + return "No Domain Requests" + domain_requests.short_description = "Domain requests" # type: ignore # Creates select2 fields (with search bars) From 62ac4757ec1b0f41b05d87d4d5cd2e3baa503f40 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 22 Aug 2024 16:25:24 -0500 Subject: [PATCH 50/78] review changes --- src/registrar/assets/js/get-gov.js | 4 +-- .../commands/populate_domain_request_dates.py | 36 +++++++++---------- ...0119_add_domainrequest_submission_dates.py | 2 +- src/registrar/models/domain_request.py | 8 ++--- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index f3b41eb51..70659b009 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1599,7 +1599,7 @@ document.addEventListener('DOMContentLoaded', function() { const domainName = request.requested_domain ? request.requested_domain : `New domain request
(${utcDateString(request.created_at)})`; const actionUrl = request.action_url; const actionLabel = request.action_label; - const submissionDate = request.submission_date ? new Date(request.submission_date).toLocaleDateString('en-US', options) : `Not submitted`; + const submissionDate = request.last_submitted_date ? new Date(request.last_submitted_date).toLocaleDateString('en-US', options) : `Not submitted`; // Even if the request is not deletable, we may need this empty string for the td if the deletable column is displayed let modalTrigger = ''; @@ -1699,7 +1699,7 @@ document.addEventListener('DOMContentLoaded', function() { ${domainName} - + ${submissionDate} diff --git a/src/registrar/management/commands/populate_domain_request_dates.py b/src/registrar/management/commands/populate_domain_request_dates.py index 90fc06dcf..19d8e4f00 100644 --- a/src/registrar/management/commands/populate_domain_request_dates.py +++ b/src/registrar/management/commands/populate_domain_request_dates.py @@ -17,26 +17,26 @@ class Command(BaseCommand, PopulateScriptTemplate): def update_record(self, record: DomainRequest): """Defines how we update the first_submitted_date and last_status_update fields""" - try: - # Retrieve and order audit log entries by timestamp in descending order - audit_log_entries = LogEntry.objects.filter(object_pk=record.pk).order_by("-timestamp") + + # Retrieve and order audit log entries by timestamp in descending order + audit_log_entries = LogEntry.objects.filter(object_pk=record.pk).order_by("-timestamp") - # Loop through logs in descending order to find most recent status change - for log_entry in audit_log_entries: - if "status" in log_entry.changes: - record.last_status_update = log_entry.timestamp.date() - break - - # Loop through logs in ascending order to find first submission - for log_entry in audit_log_entries.reverse(): - if log_entry.changes_dict['status'](1) == 'Submitted': - record.first_submitted_date = log_entry.timestamp.date() + # Loop through logs in descending order to find most recent status change + for log_entry in audit_log_entries: + if 'status' in LogEntry.changes_dict: + record.last_status_update = log_entry.timestamp.date() + break - except ObjectDoesNotExist as e: - logger.error(f"Object with object_pk {record.pk} does not exist: {e}") - except Exception as e: - logger.error(f"An error occurred during update_record: {e}") + # Loop through logs in ascending order to find first submission + for log_entry in audit_log_entries.reverse(): + if log_entry.changes_dict['status'](1) == 'Submitted': + record.first_submitted_date = log_entry.timestamp.date() + break logger.info( - f"{TerminalColors.OKCYAN}Updating {record} => first submitted date: " f"{record.first_submitted_date}{TerminalColors.OKCYAN}, last status update:" f"{record.last_status_update}{TerminalColors.OKCYAN}" + f"{TerminalColors.OKCYAN}Updating {record} => first submitted date: " f"{record.first_submitted_date}{TerminalColors.OKCYAN}, last status update:" f"{record.last_status_update}{TerminalColors.ENDC}" ) + + def should_skip_record(self, record) -> bool: + # make sure the record had some kind of history + return LogEntry.objects.filter(object_pk=record.pk).exists() diff --git a/src/registrar/migrations/0119_add_domainrequest_submission_dates.py b/src/registrar/migrations/0119_add_domainrequest_submission_dates.py index 0b94e1257..ea209626e 100644 --- a/src/registrar/migrations/0119_add_domainrequest_submission_dates.py +++ b/src/registrar/migrations/0119_add_domainrequest_submission_dates.py @@ -35,7 +35,7 @@ class Migration(migrations.Migration): field=models.DateField( blank=True, default=None, - help_text="Date of last status updated", + help_text="Date of the last status update", null=True, verbose_name="last updated on", ), diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 74d275d95..09f200793 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -563,7 +563,7 @@ class DomainRequest(TimeStampedModel): help_text="Acknowledged .gov acceptable use policy", ) - # initial submission date records when domain request was first submitted + # Records when the domain request was first submitted first_submitted_date = models.DateField( null=True, blank=True, @@ -572,7 +572,7 @@ class DomainRequest(TimeStampedModel): help_text="Date initially submitted", ) - # last submission date records when domain request was last submitted + # Records when domain request was last submitted last_submitted_date = models.DateField( null=True, blank=True, @@ -581,13 +581,13 @@ class DomainRequest(TimeStampedModel): help_text="Date last submitted", ) - # last status update records when domain request status was last updated by an admin or analyst + # Records when domain request status was last updated by an admin or analyst last_status_update = models.DateField( null=True, blank=True, default=None, verbose_name="last updated on", - help_text="Date of last status updated", + help_text="Date of the last status update", ) notes = models.TextField( null=True, From 4a104b6ff6afbbe1870f630205af5fb66f557611 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 22 Aug 2024 17:25:52 -0400 Subject: [PATCH 51/78] formatted for linter --- src/registrar/admin.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1e20523cc..a49536102 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -60,7 +60,7 @@ class CustomChangeListForPortfolioFiltering(ChangeList): # ignored. # Remove portfolio so that it does not error as an invalid # filter parameter. - ignored_params = list(IGNORED_PARAMS) + ['portfolio'] + ignored_params = list(IGNORED_PARAMS) + ["portfolio"] for ignored in ignored_params: if ignored in lookup_params: del lookup_params[ignored] @@ -2270,18 +2270,18 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): # objects rather than Contact objects. use_sort = db_field.name != "senior_official" return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) - + def get_queryset(self, request): """Custom get_queryset to filter by portfolio if portfolio is in the request params.""" qs = super().get_queryset(request) # Check if a 'portfolio' parameter is passed in the request - portfolio_id = request.GET.get('portfolio') + portfolio_id = request.GET.get("portfolio") if portfolio_id: # Further filter the queryset by the portfolio qs = qs.filter(portfolio=portfolio_id) return qs - + def get_changelist(self, request, **kwargs): """ Return the ChangeList class for use on the changelist page. @@ -2740,18 +2740,18 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): ): return True return super().has_change_permission(request, obj) - + def get_queryset(self, request): """Custom get_queryset to filter by portfolio if portfolio is in the request params.""" qs = super().get_queryset(request) # Check if a 'portfolio' parameter is passed in the request - portfolio_id = request.GET.get('portfolio') + portfolio_id = request.GET.get("portfolio") if portfolio_id: # Further filter the queryset by the portfolio qs = qs.filter(domain_info__portfolio=portfolio_id) return qs - + def get_changelist(self, request, **kwargs): """ Return the ChangeList class for use on the changelist page. @@ -3022,7 +3022,7 @@ class PortfolioAdmin(ListHeaderAdmin): return self.get_field_links_as_list(queryset, "suborganization") suborganizations.short_description = "Suborganizations" # type: ignore - + def domains(self, obj: models.Portfolio): """Returns the count of domains with a link to view them in the admin.""" domain_count = obj.get_domains().count() # Count the related domains @@ -3045,7 +3045,7 @@ class PortfolioAdmin(ListHeaderAdmin): # Create a clickable link with the domain request count return format_html('{} Domain Requests', url, domain_request_count) return "No Domain Requests" - + domain_requests.short_description = "Domain requests" # type: ignore # Creates select2 fields (with search bars) From 5da8119f86be3a8654cfabf915a4433556ad7326 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 22 Aug 2024 19:02:55 -0400 Subject: [PATCH 52/78] adjusted code for correct version of django --- src/registrar/admin.py | 51 +++++++++++-------------------- src/registrar/tests/test_admin.py | 8 ++--- src/registrar/tests/test_api.py | 6 ++++ 3 files changed, 25 insertions(+), 40 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a49536102..65974b42b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -34,7 +34,7 @@ from django_fsm import TransitionNotAllowed # type: ignore from django.utils.safestring import mark_safe from django.utils.html import escape from django.contrib.auth.forms import UserChangeForm, UsernameField -from django.contrib.admin.views.main import ChangeList, IGNORED_PARAMS +from django.contrib.admin.views.main import IGNORED_PARAMS from django_admin_multiple_choice_list_filter.list_filters import MultipleChoiceListFilter from import_export import resources from import_export.admin import ImportExportModelAdmin @@ -46,27 +46,6 @@ from django.utils.translation import gettext_lazy as _ logger = logging.getLogger(__name__) -class CustomChangeListForPortfolioFiltering(ChangeList): - """CustomChangeList so that portfolio can be passed in a url, but not appear - in the list of filters on the right side of the page.""" - - def get_filters_params(self, params=None): - """ - Return all params except IGNORED_PARAMS. - """ - params = params or self.params - lookup_params = params.copy() # a dictionary of the query string - # Remove all the parameters that are globally and systematically - # ignored. - # Remove portfolio so that it does not error as an invalid - # filter parameter. - ignored_params = list(IGNORED_PARAMS) + ["portfolio"] - for ignored in ignored_params: - if ignored in lookup_params: - del lookup_params[ignored] - return lookup_params - - class FsmModelResource(resources.ModelResource): """ModelResource is extended to support importing of tables which have FSMFields. ModelResource is extended with the following changes @@ -450,6 +429,22 @@ class MultiFieldSortableChangeList(admin.views.main.ChangeList): return ordering + def get_filters_params(self, params=None): + """ + Return all params except IGNORED_PARAMS. + """ + params = params or self.params + lookup_params = params.copy() # a dictionary of the query string + # Remove all the parameters that are globally and systematically + # ignored. + # Remove portfolio so that it does not error as an invalid + # filter parameter. + ignored_params = list(IGNORED_PARAMS) + ["portfolio"] + for ignored in ignored_params: + if ignored in lookup_params: + del lookup_params[ignored] + return lookup_params + class CustomLogEntryAdmin(LogEntryAdmin): """Overwrite the generated LogEntry admin class""" @@ -2282,12 +2277,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): qs = qs.filter(portfolio=portfolio_id) return qs - def get_changelist(self, request, **kwargs): - """ - Return the ChangeList class for use on the changelist page. - """ - return CustomChangeListForPortfolioFiltering - class TransitionDomainAdmin(ListHeaderAdmin): """Custom transition domain admin class.""" @@ -2752,12 +2741,6 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): qs = qs.filter(domain_info__portfolio=portfolio_id) return qs - def get_changelist(self, request, **kwargs): - """ - Return the ChangeList class for use on the changelist page. - """ - return CustomChangeListForPortfolioFiltering - class DraftDomainResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 827742ef1..d4404b564 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2107,9 +2107,7 @@ class TestPortfolioAdmin(TestCase): domain_2.save() domains = self.admin.domains(self.portfolio) - self.assertIn("domain1.gov", domains) - self.assertIn("domain2.gov", domains) - self.assertIn('
    ', domains) + self.assertIn("2 Domains", domains) @less_console_noise_decorator def test_domain_requests_display(self): @@ -2118,6 +2116,4 @@ class TestPortfolioAdmin(TestCase): completed_domain_request(name="request2.gov", portfolio=self.portfolio) domain_requests = self.admin.domain_requests(self.portfolio) - self.assertIn("request1.gov", domain_requests) - self.assertIn("request2.gov", domain_requests) - self.assertIn('
      ', domain_requests) + self.assertIn("2 Domain Requests", domain_requests) diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index 0025bc902..de52ad3d6 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -4,6 +4,8 @@ from registrar.models import FederalAgency, SeniorOfficial, User from django.contrib.auth import get_user_model from registrar.tests.common import create_superuser, create_user +from api.tests.common import less_console_noise_decorator + class GetSeniorOfficialJsonTest(TestCase): def setUp(self): @@ -26,6 +28,7 @@ class GetSeniorOfficialJsonTest(TestCase): SeniorOfficial.objects.all().delete() FederalAgency.objects.all().delete() + @less_console_noise_decorator def test_get_senior_official_json_authenticated_superuser(self): """Test that a superuser can fetch the senior official information.""" p = "adminpass" @@ -38,6 +41,7 @@ class GetSeniorOfficialJsonTest(TestCase): self.assertEqual(data["last_name"], "Doe") self.assertEqual(data["title"], "Director") + @less_console_noise_decorator def test_get_senior_official_json_authenticated_analyst(self): """Test that an analyst user can fetch the senior official's information.""" p = "userpass" @@ -50,6 +54,7 @@ class GetSeniorOfficialJsonTest(TestCase): self.assertEqual(data["last_name"], "Doe") self.assertEqual(data["title"], "Director") + @less_console_noise_decorator def test_get_senior_official_json_unauthenticated(self): """Test that an unauthenticated user receives a 403 with an error message.""" p = "password" @@ -57,6 +62,7 @@ class GetSeniorOfficialJsonTest(TestCase): response = self.client.get(self.api_url, {"agency_name": "Test Agency"}) self.assertEqual(response.status_code, 302) + @less_console_noise_decorator def test_get_senior_official_json_not_found(self): """Test that a request for a non-existent agency returns a 404 with an error message.""" p = "adminpass" From 851e176f83209dd8e902ac79d1848650d37e2268 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 22 Aug 2024 21:45:00 -0400 Subject: [PATCH 53/78] added tests and comments --- src/registrar/admin.py | 10 ++++++-- src/registrar/tests/test_admin_domain.py | 28 +++++++++++++++++++++++ src/registrar/tests/test_admin_request.py | 25 ++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 65974b42b..385c1e60e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -367,7 +367,9 @@ class DomainRequestAdminForm(forms.ModelForm): class MultiFieldSortableChangeList(admin.views.main.ChangeList): """ This class overrides the behavior of column sorting in django admin tables in order - to allow for multi field sorting on admin_order_field + to allow for multi field sorting on admin_order_field. It also overrides behavior + of getting the filter params to allow portfolio filters to be executed without + displaying on the right side of the ChangeList view. Usage: @@ -431,7 +433,11 @@ class MultiFieldSortableChangeList(admin.views.main.ChangeList): def get_filters_params(self, params=None): """ - Return all params except IGNORED_PARAMS. + Overrides the default behavior which gets filter_params, except + those in IGNORED_PARAMS. The override is to also include + portfolio in the overrides. This allows the portfolio filter + not to throw an error as a valid filter while not listing the + portfolio filter on the right side of the Change List view. """ params = params or self.params lookup_params = params.copy() # a dictionary of the query string diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index e156dd377..64d4ee77d 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -14,6 +14,7 @@ from registrar.models import ( DomainInformation, User, Host, + Portfolio, ) from .common import ( MockSESClient, @@ -359,6 +360,7 @@ class TestDomainAdminWithClient(TestCase): Domain.objects.all().delete() DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() + Portfolio.objects.all().delete() @classmethod def tearDownClass(self): @@ -452,6 +454,32 @@ class TestDomainAdminWithClient(TestCase): domain_request.delete() _creator.delete() + @less_console_noise_decorator + def test_domains_by_portfolio(self): + """ + Tests that domains display for a portfolio. + """ + + portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test Portfolio", creator=self.superuser) + # Create a fake domain request and domain + _domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.IN_REVIEW, portfolio=portfolio + ) + _domain_request.approve() + + domain = _domain_request.approved_domain + + self.client.force_login(self.superuser) + response = self.client.get( + "/admin/registrar/domain/?portfolio={}".format(portfolio.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain.name) + self.assertContains(response, portfolio.organization_name) + @less_console_noise_decorator def test_helper_text(self): """ diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index c4fc8bcee..04faf0504 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -22,6 +22,7 @@ from registrar.models import ( Contact, Website, SeniorOfficial, + Portfolio, ) from .common import ( MockSESClient, @@ -78,6 +79,7 @@ class TestDomainRequestAdmin(MockEppLib): Contact.objects.all().delete() Website.objects.all().delete() SeniorOfficial.objects.all().delete() + Portfolio.objects.all().delete() self.mock_client.EMAILS_SENT.clear() @classmethod @@ -263,6 +265,29 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, domain_request.requested_domain.name) self.assertContains(response, "Show details") + @less_console_noise_decorator + def test_domain_requests_by_portfolio(self): + """ + Tests that domain_requests display for a portfolio. + """ + + portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test Portfolio", creator=self.superuser) + # Create a fake domain request and domain + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.IN_REVIEW, portfolio=portfolio + ) + + self.client.force_login(self.superuser) + response = self.client.get( + "/admin/registrar/domainrequest/?portfolio={}".format(portfolio.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain_request.requested_domain.name) + self.assertContains(response, portfolio.organization_name) + @less_console_noise_decorator def test_analyst_can_see_and_edit_alternative_domain(self): """Tests if an analyst can still see and edit the alternative domain field""" From 8a9971ac79bb08a934e269674a28993d5470b608 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 23 Aug 2024 17:50:24 -0400 Subject: [PATCH 54/78] initial implementation --- src/registrar/assets/js/get-gov-admin.js | 51 +++++++++++++++++++ src/registrar/config/urls.py | 10 +++- src/registrar/models/portfolio.py | 16 ++++-- .../django/admin/portfolio_change_form.html | 2 + src/registrar/views/utility/api_views.py | 33 ++++++++++++ 5 files changed, 107 insertions(+), 5 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 01c93abf6..11e3cae69 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -837,6 +837,27 @@ function initializeWidgetOnList(list, parentId) { // If we can update the contact information, it'll be shown again. hideElement(contactList.parentElement); + let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; + fetch(`${federalPortfolioApi}?organization_type=${organizationType.value}&agency_name=${selectedText}`) + .then(response => { + const statusCode = response.status; + return response.json().then(data => ({ statusCode, data })); + }) + .then(({ statusCode, data }) => { + if (data.error) { + console.error("Error in AJAX call: " + data.error); + return; + } + + let federal_type = data.federal_type; + let portfolio_type = data.portfolio_type; + console.log("portfolio type: " + portfolio_type); + console.log("federal type: " + federal_type); + updateFederalType(data.federal_type); + updatePortfolioType(data.portfolio_type); + }) + .catch(error => console.error("Error fetching federal and portfolio types: ", error)); + let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) .then(response => { @@ -879,6 +900,7 @@ function initializeWidgetOnList(list, parentId) { } }) .catch(error => console.error("Error fetching senior official: ", error)); + } function handleStateTerritoryChange(stateTerritory, urbanizationField) { @@ -890,6 +912,35 @@ function initializeWidgetOnList(list, parentId) { } } + function updatePortfolioType(portfolioType) { + console.log("attempting to update portfolioType"); + // Find the div with class 'field-portfolio_type' + const portfolioTypeDiv = document.querySelector('.field-portfolio_type'); + if (portfolioTypeDiv) { + console.log("found portfoliotype"); + // Find the nested div with class 'readonly' inside 'field-portfolio_type' + const readonlyDiv = portfolioTypeDiv.querySelector('.readonly'); + if (readonlyDiv) { + console.log("found readonly div"); + // Update the text content of the readonly div + readonlyDiv.textContent = portfolioType !== null ? portfolioType : '-'; + } + } + } + + function updateFederalType(federalType) { + // Find the div with class 'field-federal_type' + const federalTypeDiv = document.querySelector('.field-federal_type'); + if (federalTypeDiv) { + // Find the nested div with class 'readonly' inside 'field-federal_type' + const readonlyDiv = federalTypeDiv.querySelector('.readonly'); + if (readonlyDiv) { + // Update the text content of the readonly div + readonlyDiv.textContent = federalType !== null ? federalType : '-'; + } + } + } + function updateContactInfo(data) { if (!contactList) return; diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 413449896..5e58cf740 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -24,7 +24,10 @@ from registrar.views.report_views import ( from registrar.views.domain_request import Step from registrar.views.domain_requests_json import get_domain_requests_json -from registrar.views.utility.api_views import get_senior_official_from_federal_agency_json +from registrar.views.utility.api_views import ( + get_senior_official_from_federal_agency_json, + get_federal_and_portfolio_types_from_federal_agency_json +) from registrar.views.domains_json import get_domains_json from registrar.views.utility import always_404 from api.views import available, get_current_federal, get_current_full @@ -139,6 +142,11 @@ urlpatterns = [ get_senior_official_from_federal_agency_json, name="get-senior-official-from-federal-agency-json", ), + path( + "admin/api/get-federal-and-portfolio-types-from-federal-agency-json/", + get_federal_and_portfolio_types_from_federal_agency_json, + name="get-federal-and-portfolio-types-from-federal-agency-json", + ), path("admin/", admin.site.urls), path( "reports/export_data_type_user/", diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 0f9904c31..b9b0d4a22 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -131,9 +131,13 @@ class Portfolio(TimeStampedModel): Returns a combination of organization_type / federal_type, seperated by ' - '. If no federal_type is found, we just return the org type. """ - org_type_label = self.OrganizationChoices.get_org_label(self.organization_type) - agency_type_label = BranchChoices.get_branch_label(self.federal_type) - if self.organization_type == self.OrganizationChoices.FEDERAL and agency_type_label: + return self.get_portfolio_type(self.organization_type, self.federal_type) + + @classmethod + def get_portfolio_type(cls, organization_type, federal_type): + org_type_label = cls.OrganizationChoices.get_org_label(organization_type) + agency_type_label = BranchChoices.get_branch_label(federal_type) + if organization_type == cls.OrganizationChoices.FEDERAL and agency_type_label: return " - ".join([org_type_label, agency_type_label]) else: return org_type_label @@ -141,8 +145,12 @@ class Portfolio(TimeStampedModel): @property def federal_type(self): """Returns the federal_type value on the underlying federal_agency field""" - return self.federal_agency.federal_type if self.federal_agency else None + return self.get_federal_type(self.federal_agency) + @classmethod + def get_federal_type(cls, federal_agency): + return federal_agency.federal_type if federal_agency else None + # == Getters for domains == # def get_domains(self): """Returns all DomainInformations associated with this portfolio""" diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 9d59aae42..4eb941340 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -5,6 +5,8 @@ {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} {% url 'get-senior-official-from-federal-agency-json' as url %} + {% url 'get-federal-and-portfolio-types-from-federal-agency-json' as url %} + {{ block.super }} {% endblock content %} diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 2c9414d1d..e67a1974c 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -5,6 +5,9 @@ from registrar.models import FederalAgency, SeniorOfficial from django.contrib.admin.views.decorators import staff_member_required from django.contrib.auth.decorators import login_required +from registrar.models.portfolio import Portfolio +from registrar.utility.constants import BranchChoices + logger = logging.getLogger(__name__) @@ -34,3 +37,33 @@ def get_senior_official_from_federal_agency_json(request): return JsonResponse(so_dict) else: return JsonResponse({"error": "Senior Official not found"}, status=404) + +@login_required +@staff_member_required +def get_federal_and_portfolio_types_from_federal_agency_json(request): + """Returns specific portfolio information as a JSON. Request must have + both agency_name and organization_type.""" + + # This API is only accessible to admins and analysts + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + if not request.user.is_authenticated or not any([analyst_perm, superuser_perm]): + return JsonResponse({"error": "You do not have access to this resource"}, status=403) + + federal_type = None + portfolio_type = None + + agency_name = request.GET.get("agency_name") + organization_type = request.GET.get("organization_type") + agency = FederalAgency.objects.filter(agency=agency_name).first() + if agency: + federal_type = Portfolio.get_federal_type(agency) + portfolio_type = Portfolio.get_portfolio_type(organization_type, federal_type) + federal_type = BranchChoices.get_branch_label(federal_type) if federal_type else "-" + + response_data = { + 'portfolio_type': portfolio_type, + 'federal_type': federal_type, + } + + return JsonResponse(response_data) \ No newline at end of file From ef7739dc556b9e9c7b5c046934e4ee44a5bf733e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 23 Aug 2024 17:52:57 -0400 Subject: [PATCH 55/78] lint --- src/registrar/config/urls.py | 2 +- src/registrar/models/portfolio.py | 2 +- src/registrar/views/utility/api_views.py | 11 ++++++----- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 5e58cf740..19fa99809 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -26,7 +26,7 @@ from registrar.views.domain_request import Step from registrar.views.domain_requests_json import get_domain_requests_json from registrar.views.utility.api_views import ( get_senior_official_from_federal_agency_json, - get_federal_and_portfolio_types_from_federal_agency_json + get_federal_and_portfolio_types_from_federal_agency_json, ) from registrar.views.domains_json import get_domains_json from registrar.views.utility import always_404 diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index b9b0d4a22..fadcf8cac 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -150,7 +150,7 @@ class Portfolio(TimeStampedModel): @classmethod def get_federal_type(cls, federal_agency): return federal_agency.federal_type if federal_agency else None - + # == Getters for domains == # def get_domains(self): """Returns all DomainInformations associated with this portfolio""" diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index e67a1974c..97eb7e86c 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -37,7 +37,8 @@ def get_senior_official_from_federal_agency_json(request): return JsonResponse(so_dict) else: return JsonResponse({"error": "Senior Official not found"}, status=404) - + + @login_required @staff_member_required def get_federal_and_portfolio_types_from_federal_agency_json(request): @@ -62,8 +63,8 @@ def get_federal_and_portfolio_types_from_federal_agency_json(request): federal_type = BranchChoices.get_branch_label(federal_type) if federal_type else "-" response_data = { - 'portfolio_type': portfolio_type, - 'federal_type': federal_type, + "portfolio_type": portfolio_type, + "federal_type": federal_type, } - - return JsonResponse(response_data) \ No newline at end of file + + return JsonResponse(response_data) From 31285da3956043a0a924ef4ea2a30d785a6a1c1a Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 23 Aug 2024 18:16:01 -0400 Subject: [PATCH 56/78] added test code --- src/registrar/tests/test_api.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index 0025bc902..bd96f3e24 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -4,6 +4,8 @@ from registrar.models import FederalAgency, SeniorOfficial, User from django.contrib.auth import get_user_model from registrar.tests.common import create_superuser, create_user +from api.tests.common import less_console_noise_decorator + class GetSeniorOfficialJsonTest(TestCase): def setUp(self): @@ -65,3 +67,32 @@ class GetSeniorOfficialJsonTest(TestCase): self.assertEqual(response.status_code, 404) data = response.json() self.assertEqual(data["error"], "Senior Official not found") + + +class GetFederalPortfolioTypeJsonTest(TestCase): + def setUp(self): + self.client = Client() + p = "password" + self.user = get_user_model().objects.create_user(username="testuser", password=p) + + self.superuser = create_superuser() + self.analyst_user = create_user() + + self.agency = FederalAgency.objects.create(agency="Test Agency", federal_type="judicial") + + self.api_url = reverse("get-federal-and-portfolio-types-from-federal-agency-json") + + def tearDown(self): + User.objects.all().delete() + FederalAgency.objects.all().delete() + + @less_console_noise_decorator + def test_get_federal_and_portfolio_types_json_authenticated_superuser(self): + """Test that a superuser can fetch the federal and portfolio types.""" + p = "adminpass" + self.client.login(username="superuser", password=p) + response = self.client.get(self.api_url, {"agency_name": "Test Agency", "organization_type": "federal"}) + self.assertEqual(response.status_code, 200) + data = response.json() + self.assertEqual(data["federal_type"], "Judicial") + self.assertEqual(data["portfolio_type"], "Federal - Judicial") From 4cee98edee40a611dd8d770c60fe22011115bfbd Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 23 Aug 2024 22:39:49 -0400 Subject: [PATCH 57/78] A bit of cleanup --- src/registrar/assets/js/get-gov-admin.js | 29 +---------- .../admin/includes/detail_table_fieldset.html | 51 ++++++++++--------- 2 files changed, 27 insertions(+), 53 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index d58bced42..c05ef090c 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -518,9 +518,6 @@ function initializeWidgetOnList(list, parentId) { var actionNeededEmailReadonly = document.querySelector("#action-needed-reason-email-readonly") var actionNeededEmailReadonlyTextarea = document.querySelector("#action-needed-reason-email-readonly-textarea") - // Edit e-mail button (appears only in readonly view for e-mail text) - var editEmailButton = document.querySelector("#action-needed-reason-email-edit-button") - // Edit e-mail modal (and its confirmation button) var actionNeededEmailAlreadySentModal = document.querySelector("#email-already-sent-modal") var confirmEditEmailButton = document.querySelector("#email-already-sent-modal_continue-editing-button") @@ -565,12 +562,7 @@ function initializeWidgetOnList(list, parentId) { }); editEmailButton.addEventListener("click", function() { - if (checkEmailAlreadySent()) { - // Show warning Modal - setModalVisibility(actionNeededEmailAlreadySentModal, true) - } - else { - // Show editable view + if (!checkEmailAlreadySent()) { showEmail(canEdit=true) } }); @@ -580,13 +572,6 @@ function initializeWidgetOnList(list, parentId) { showEmail(canEdit=true) }); - // Event delegation for data-close-modal buttons - // (since manually opening the modal interferes with how this normally works) - document.addEventListener('click', function(event) { - if (event.target.matches('[data-close-modal]')) { - setModalVisibility(actionNeededEmailAlreadySentModal, false) - } - }); // Add a change listener to the action needed reason dropdown actionNeededReasonDropdown.addEventListener("change", function() { @@ -612,8 +597,6 @@ function initializeWidgetOnList(list, parentId) { { lastEmailSent = lastSentEmailText.value.replace(/\s+/g, '') currentEmailInTextArea = actionNeededEmail.value.replace(/\s+/g, '') - console.log("old email value = " + lastEmailSent) - console.log("current email value = " + currentEmailInTextArea) return lastEmailSent === currentEmailInTextArea } @@ -633,16 +616,6 @@ function initializeWidgetOnList(list, parentId) { actionNeededEmailFooter.innerHTML = "This email will be sent to the creator of this request after saving"; } - function setModalVisibility(modalToToggle, isVisible) { - if (!isVisible) { - modalToToggle.classList.remove('is-visible'); - modalToToggle.setAttribute('aria-hidden', 'true'); - } else { - modalToToggle.classList.add('is-visible'); - modalToToggle.setAttribute('aria-hidden', 'false'); - } - } - // Shows either a preview of the email or some text describing no email will be sent. // If the email doesn't exist or if we're of reason "other", display that no email was sent. function updateActionNeededEmailDisplay(reason) { diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 087bacda6..7819e3aff 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -151,12 +151,31 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
-
From 69a3db5f59af1a8eb83b40643d6b4ff3e985babb Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 23 Aug 2024 22:54:47 -0400 Subject: [PATCH 59/78] accessibility fix --- .../templates/django/admin/includes/detail_table_fieldset.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 8cfb31064..3b4047d39 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -232,7 +232,8 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
-
From 6091ff76c3ad71aaaa130842d5cdbdf0c24dd21c Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 26 Aug 2024 11:42:10 -0400 Subject: [PATCH 60/78] cleanup --- src/registrar/assets/js/get-gov-admin.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 11e3cae69..98a2395db 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -837,6 +837,8 @@ function initializeWidgetOnList(list, parentId) { // If we can update the contact information, it'll be shown again. hideElement(contactList.parentElement); + // Determine if any changes are necessary to the display of portfolio type or federal type + // based on changes to the Federal Agency let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; fetch(`${federalPortfolioApi}?organization_type=${organizationType.value}&agency_name=${selectedText}`) .then(response => { @@ -851,8 +853,6 @@ function initializeWidgetOnList(list, parentId) { let federal_type = data.federal_type; let portfolio_type = data.portfolio_type; - console.log("portfolio type: " + portfolio_type); - console.log("federal type: " + federal_type); updateFederalType(data.federal_type); updatePortfolioType(data.portfolio_type); }) @@ -912,22 +912,25 @@ function initializeWidgetOnList(list, parentId) { } } + /** + * Dynamically update the portfolio type text in the dom to portfolioType + */ function updatePortfolioType(portfolioType) { - console.log("attempting to update portfolioType"); // Find the div with class 'field-portfolio_type' const portfolioTypeDiv = document.querySelector('.field-portfolio_type'); if (portfolioTypeDiv) { - console.log("found portfoliotype"); // Find the nested div with class 'readonly' inside 'field-portfolio_type' const readonlyDiv = portfolioTypeDiv.querySelector('.readonly'); if (readonlyDiv) { - console.log("found readonly div"); // Update the text content of the readonly div readonlyDiv.textContent = portfolioType !== null ? portfolioType : '-'; } } } + /** + * Dynamically update the federal type text in the dom to federalType + */ function updateFederalType(federalType) { // Find the div with class 'field-federal_type' const federalTypeDiv = document.querySelector('.field-federal_type'); From 497837b09882731c121adbba88110d6155253e66 Mon Sep 17 00:00:00 2001 From: Matt-Spence Date: Mon, 26 Aug 2024 11:18:43 -0500 Subject: [PATCH 61/78] Update src/registrar/management/commands/populate_domain_request_dates.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- .../management/commands/populate_domain_request_dates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_domain_request_dates.py b/src/registrar/management/commands/populate_domain_request_dates.py index 19d8e4f00..fe471fc9d 100644 --- a/src/registrar/management/commands/populate_domain_request_dates.py +++ b/src/registrar/management/commands/populate_domain_request_dates.py @@ -34,7 +34,7 @@ class Command(BaseCommand, PopulateScriptTemplate): break logger.info( - f"{TerminalColors.OKCYAN}Updating {record} => first submitted date: " f"{record.first_submitted_date}{TerminalColors.OKCYAN}, last status update:" f"{record.last_status_update}{TerminalColors.ENDC}" + f"{TerminalColors.OKCYAN}Updating {record} => first submitted date: " f"{record.first_submitted_date}, last status update:" f"{record.last_status_update}{TerminalColors.ENDC}" ) def should_skip_record(self, record) -> bool: From f7b2e38bba21c91cc46b39da26464800cdf72ac9 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Mon, 26 Aug 2024 11:54:08 -0500 Subject: [PATCH 62/78] linter fixes --- .../commands/populate_domain_request_dates.py | 11 +++++++---- src/registrar/models/domain_request.py | 2 +- src/registrar/tests/test_views_requests_json.py | 6 ++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_request_dates.py b/src/registrar/management/commands/populate_domain_request_dates.py index fe471fc9d..e20e35909 100644 --- a/src/registrar/management/commands/populate_domain_request_dates.py +++ b/src/registrar/management/commands/populate_domain_request_dates.py @@ -3,7 +3,6 @@ from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors from registrar.models import DomainRequest from auditlog.models import LogEntry -from django.core.exceptions import ObjectDoesNotExist logger = logging.getLogger(__name__) @@ -12,12 +11,13 @@ class Command(BaseCommand, PopulateScriptTemplate): help = "Loops through each domain request object and populates the last_status_update and first_submitted_date" def handle(self, **kwargs): - """Loops through each DomainRequest object and populates its last_status_update and first_submitted_date values""" + """Loops through each DomainRequest object and populates + its last_status_update and first_submitted_date values""" self.mass_update_records(DomainRequest, None, ["last_status_update", "first_submitted_date"]) def update_record(self, record: DomainRequest): """Defines how we update the first_submitted_date and last_status_update fields""" - + # Retrieve and order audit log entries by timestamp in descending order audit_log_entries = LogEntry.objects.filter(object_pk=record.pk).order_by("-timestamp") @@ -34,7 +34,10 @@ class Command(BaseCommand, PopulateScriptTemplate): break logger.info( - f"{TerminalColors.OKCYAN}Updating {record} => first submitted date: " f"{record.first_submitted_date}, last status update:" f"{record.last_status_update}{TerminalColors.ENDC}" + f"""{TerminalColors.OKCYAN}Updating {record} => + first submitted date: {record.first_submitted_date}, + last status update: {record.last_status_update}{TerminalColors.ENDC} + """ ) def should_skip_record(self, record) -> bool: diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 09f200793..7ee80e43a 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -823,7 +823,7 @@ class DomainRequest(TimeStampedModel): if not DraftDomain.string_could_be_domain(self.requested_domain.name): raise ValueError("Requested domain is not a valid domain name.") - # if the domain has not been submitted before this must be the first time + # if the domain has not been submitted before this must be the first time if not self.first_submitted_date: self.first_submitted_date = timezone.now().date() diff --git a/src/registrar/tests/test_views_requests_json.py b/src/registrar/tests/test_views_requests_json.py index c140b7e44..fd44bdc31 100644 --- a/src/registrar/tests/test_views_requests_json.py +++ b/src/registrar/tests/test_views_requests_json.py @@ -287,7 +287,8 @@ class GetRequestsJsonTest(TestWithUser, WebTest): def test_sorting(self): """test that sorting works properly on the result set""" - response = self.app.get(reverse("get_domain_requests_json"), {"sort_by": "last_submitted_date", "order": "desc"}) + response = self.app.get(reverse("get_domain_requests_json"), + {"sort_by": "last_submitted_date", "order": "desc"}) self.assertEqual(response.status_code, 200) data = response.json @@ -306,7 +307,8 @@ class GetRequestsJsonTest(TestWithUser, WebTest): def test_filter_approved_excluded(self): """test that approved requests are excluded from result set.""" # sort in reverse chronological order of submission date, since most recent request is approved - response = self.app.get(reverse("get_domain_requests_json"), {"sort_by": "last_submitted_date", "order": "desc"}) + response = self.app.get(reverse("get_domain_requests_json"), + {"sort_by": "last_submitted_date", "order": "desc"}) self.assertEqual(response.status_code, 200) data = response.json From c2f71c984ffe13e83bb768a7bef72c167cb1fe7b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 26 Aug 2024 20:34:56 -0400 Subject: [PATCH 63/78] a few improvements to code, comments, and tests --- src/registrar/admin.py | 12 +++++------- src/registrar/tests/test_admin_domain.py | 8 +++++++- src/registrar/tests/test_admin_request.py | 6 +++++- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 385c1e60e..ca7742a89 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -433,11 +433,9 @@ class MultiFieldSortableChangeList(admin.views.main.ChangeList): def get_filters_params(self, params=None): """ - Overrides the default behavior which gets filter_params, except - those in IGNORED_PARAMS. The override is to also include - portfolio in the overrides. This allows the portfolio filter - not to throw an error as a valid filter while not listing the - portfolio filter on the right side of the Change List view. + Add portfolio to ignored params to allow the portfolio filter while not + listing it as a filter option on the right side of Change List on the + portfolio list. """ params = params or self.params lookup_params = params.copy() # a dictionary of the query string @@ -3018,7 +3016,7 @@ class PortfolioAdmin(ListHeaderAdmin): if domain_count > 0: # Construct the URL to the admin page, filtered by portfolio url = reverse("admin:registrar_domain_changelist") + f"?portfolio={obj.id}" - label = "Domain" if domain_count == 1 else "Domains" + label = "Domain" if domain_count == 1 else "No domains" # Create a clickable link with the domain count return format_html('{} {}', url, domain_count, label) return "No Domains" @@ -3033,7 +3031,7 @@ class PortfolioAdmin(ListHeaderAdmin): url = reverse("admin:registrar_domainrequest_changelist") + f"?portfolio={obj.id}" # Create a clickable link with the domain request count return format_html('{} Domain Requests', url, domain_request_count) - return "No Domain Requests" + return "No domain requests" domain_requests.short_description = "Domain requests" # type: ignore diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 64d4ee77d..385a00800 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -16,6 +16,7 @@ from registrar.models import ( Host, Portfolio, ) +from registrar.models.user_domain_role import UserDomainRole from .common import ( MockSESClient, completed_domain_request, @@ -357,6 +358,7 @@ class TestDomainAdminWithClient(TestCase): def tearDown(self): super().tearDown() Host.objects.all().delete() + UserDomainRole.objects.all().delete() Domain.objects.all().delete() DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() @@ -457,7 +459,7 @@ class TestDomainAdminWithClient(TestCase): @less_console_noise_decorator def test_domains_by_portfolio(self): """ - Tests that domains display for a portfolio. + Tests that domains display for a portfolio. And that domains outside the portfolio do not display. """ portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test Portfolio", creator=self.superuser) @@ -468,6 +470,9 @@ class TestDomainAdminWithClient(TestCase): _domain_request.approve() domain = _domain_request.approved_domain + domain2, _ = Domain.objects.get_or_create(name="fake.gov", state=Domain.State.READY) + UserDomainRole.objects.get_or_create() + UserDomainRole.objects.get_or_create(user=self.superuser, domain=domain2, role=UserDomainRole.Roles.MANAGER) self.client.force_login(self.superuser) response = self.client.get( @@ -478,6 +483,7 @@ class TestDomainAdminWithClient(TestCase): # Make sure the page loaded, and that we're on the right page self.assertEqual(response.status_code, 200) self.assertContains(response, domain.name) + self.assertNotContains(response, domain2.name) self.assertContains(response, portfolio.organization_name) @less_console_noise_decorator diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 04faf0504..b54383b09 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -268,7 +268,7 @@ class TestDomainRequestAdmin(MockEppLib): @less_console_noise_decorator def test_domain_requests_by_portfolio(self): """ - Tests that domain_requests display for a portfolio. + Tests that domain_requests display for a portfolio. And requests not in portfolio do not display. """ portfolio, _ = Portfolio.objects.get_or_create(organization_name="Test Portfolio", creator=self.superuser) @@ -276,6 +276,9 @@ class TestDomainRequestAdmin(MockEppLib): domain_request = completed_domain_request( status=DomainRequest.DomainRequestStatus.IN_REVIEW, portfolio=portfolio ) + domain_request2 = completed_domain_request( + name="testdomain2.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW + ) self.client.force_login(self.superuser) response = self.client.get( @@ -286,6 +289,7 @@ class TestDomainRequestAdmin(MockEppLib): # Make sure the page loaded, and that we're on the right page self.assertEqual(response.status_code, 200) self.assertContains(response, domain_request.requested_domain.name) + self.assertNotContains(response, domain_request2.requested_domain.name) self.assertContains(response, portfolio.organization_name) @less_console_noise_decorator From 6f4c63c9952341c21c35865909751e926237cd17 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 26 Aug 2024 20:38:29 -0400 Subject: [PATCH 64/78] lint --- src/registrar/tests/test_admin_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index b54383b09..4a828fe42 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -277,7 +277,7 @@ class TestDomainRequestAdmin(MockEppLib): status=DomainRequest.DomainRequestStatus.IN_REVIEW, portfolio=portfolio ) domain_request2 = completed_domain_request( - name="testdomain2.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW + name="testdomain2.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW ) self.client.force_login(self.superuser) From 88086a07d82bcfe931d9198068468553b0eec0e8 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 26 Aug 2024 20:43:06 -0400 Subject: [PATCH 65/78] fixed some casing and associated tests --- src/registrar/admin.py | 6 +++--- src/registrar/tests/test_admin.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ca7742a89..f0adaab76 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3016,10 +3016,10 @@ class PortfolioAdmin(ListHeaderAdmin): if domain_count > 0: # Construct the URL to the admin page, filtered by portfolio url = reverse("admin:registrar_domain_changelist") + f"?portfolio={obj.id}" - label = "Domain" if domain_count == 1 else "No domains" + label = "domain" if domain_count == 1 else "domains" # Create a clickable link with the domain count return format_html('{} {}', url, domain_count, label) - return "No Domains" + return "No domains" domains.short_description = "Domains" # type: ignore @@ -3030,7 +3030,7 @@ class PortfolioAdmin(ListHeaderAdmin): # Construct the URL to the admin page, filtered by portfolio url = reverse("admin:registrar_domainrequest_changelist") + f"?portfolio={obj.id}" # Create a clickable link with the domain request count - return format_html('{} Domain Requests', url, domain_request_count) + return format_html('{} domain requests', url, domain_request_count) return "No domain requests" domain_requests.short_description = "Domain requests" # type: ignore diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index d4404b564..a435c6a69 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2107,7 +2107,7 @@ class TestPortfolioAdmin(TestCase): domain_2.save() domains = self.admin.domains(self.portfolio) - self.assertIn("2 Domains", domains) + self.assertIn("2 domains", domains) @less_console_noise_decorator def test_domain_requests_display(self): @@ -2116,4 +2116,4 @@ class TestPortfolioAdmin(TestCase): completed_domain_request(name="request2.gov", portfolio=self.portfolio) domain_requests = self.admin.domain_requests(self.portfolio) - self.assertIn("2 Domain Requests", domain_requests) + self.assertIn("2 domain requests", domain_requests) From b2a464668b5c475c4a263a0f745dfe49679ceed3 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 27 Aug 2024 11:44:30 -0400 Subject: [PATCH 66/78] update from merge --- src/registrar/admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 57c8cdfaf..d1ffda23a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2982,6 +2982,7 @@ class PortfolioAdmin(ListHeaderAdmin): "domain_requests", "suborganizations", "portfolio_type", + "creator", ] def federal_type(self, obj: models.Portfolio): From 89c2e4543a3d37b64306b44829c13815e06f305f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 27 Aug 2024 12:27:35 -0600 Subject: [PATCH 67/78] Update src/registrar/models/user.py Co-authored-by: Matt-Spence --- src/registrar/models/user.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 03d11e5bd..a7ea1e14a 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -206,12 +206,11 @@ class User(AbstractUser): if not portfolio: return False - portfolio_perms = self.portfolio_permissions.filter(portfolio=portfolio, user=self).first() - if not portfolio_perms: + user_portfolio_perms = self.portfolio_permissions.filter(portfolio=portfolio, user=self).first() + if not user_portfolio_perms: return False - portfolio_permissions = portfolio_perms._get_portfolio_permissions() - return portfolio_permission in portfolio_permissions + return portfolio_permission in user_portfolio_perms._get_portfolio_permissions() def has_base_portfolio_permission(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_PORTFOLIO) From 207a3ac3aa2801e1d188f28349d1ea71a95b328a Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 27 Aug 2024 13:31:03 -0500 Subject: [PATCH 68/78] fix populate script --- .../management/commands/populate_domain_request_dates.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_request_dates.py b/src/registrar/management/commands/populate_domain_request_dates.py index e20e35909..f7ea5ce27 100644 --- a/src/registrar/management/commands/populate_domain_request_dates.py +++ b/src/registrar/management/commands/populate_domain_request_dates.py @@ -20,16 +20,16 @@ class Command(BaseCommand, PopulateScriptTemplate): # Retrieve and order audit log entries by timestamp in descending order audit_log_entries = LogEntry.objects.filter(object_pk=record.pk).order_by("-timestamp") - # Loop through logs in descending order to find most recent status change for log_entry in audit_log_entries: - if 'status' in LogEntry.changes_dict: + if 'status' in log_entry.changes_dict: record.last_status_update = log_entry.timestamp.date() break # Loop through logs in ascending order to find first submission for log_entry in audit_log_entries.reverse(): - if log_entry.changes_dict['status'](1) == 'Submitted': + status = log_entry.changes_dict.get('status') + if status and status[1] == 'Submitted': record.first_submitted_date = log_entry.timestamp.date() break @@ -42,4 +42,4 @@ class Command(BaseCommand, PopulateScriptTemplate): def should_skip_record(self, record) -> bool: # make sure the record had some kind of history - return LogEntry.objects.filter(object_pk=record.pk).exists() + return not LogEntry.objects.filter(object_pk=record.pk).exists() \ No newline at end of file From 990e16246d49b25173d85e00a6f88927690da071 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 27 Aug 2024 13:26:01 -0600 Subject: [PATCH 69/78] Update src/registrar/tests/test_views_portfolio.py Co-authored-by: Matt-Spence --- src/registrar/tests/test_views_portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 42cf82fa4..c5d1a9830 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -414,7 +414,7 @@ class TestPortfolio(WebTest): def test_portfolio_in_session_is_none_when_organization_feature_inactive(self): """When organization_feature flag is false and user has a portfolio, the portfolio should be set to None in session. - This test also satisfies the conidition when multiple_portfolios flag + This test also satisfies the condition when multiple_portfolios flag is false and user has a portfolio, so won't add a redundant test for that.""" self.client.force_login(self.user) portfolio_roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN] From 1fa80e8193c2650a1296ac8479cc036af633f569 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 27 Aug 2024 13:26:13 -0600 Subject: [PATCH 70/78] Update src/registrar/registrar_middleware.py Co-authored-by: Matt-Spence --- src/registrar/registrar_middleware.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index caefee650..4b590db1e 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -146,7 +146,7 @@ class CheckPortfolioMiddleware: # set the portfolio in the session if it is not set if "portfolio" not in request.session or request.session["portfolio"] is None: - # if user is a multiple portfolio + # if multiple portfolios are allowed for this user if flag_is_active(request, "multiple_portfolios"): # NOTE: we will want to change later to have a workflow for selecting # portfolio and another for switching portfolio; for now, select first From 9e592cbd753052c6f5df68d5120bf732f6c43e25 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 27 Aug 2024 15:11:43 -0500 Subject: [PATCH 71/78] fix dumb typo --- .../management/commands/populate_domain_request_dates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_domain_request_dates.py b/src/registrar/management/commands/populate_domain_request_dates.py index f7ea5ce27..cef140cdd 100644 --- a/src/registrar/management/commands/populate_domain_request_dates.py +++ b/src/registrar/management/commands/populate_domain_request_dates.py @@ -29,7 +29,7 @@ class Command(BaseCommand, PopulateScriptTemplate): # Loop through logs in ascending order to find first submission for log_entry in audit_log_entries.reverse(): status = log_entry.changes_dict.get('status') - if status and status[1] == 'Submitted': + if status and status[1] == 'submitted': record.first_submitted_date = log_entry.timestamp.date() break From 9c069a8e64e48b237caec9d41423aec264f66fb6 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 27 Aug 2024 15:12:57 -0500 Subject: [PATCH 72/78] add instructions --- docs/operations/data_migration.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index cd748b22d..ed2f643d5 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -816,3 +816,30 @@ Example: `cf ssh getgov-za` | | Parameter | Description | |:-:|:-------------------------- |:-----------------------------------------------------------------------------------| | 1 | **federal_cio_csv_path** | Specifies where the federal CIO csv is | + +## Populate Domain Request Dates +This section outlines how to run the populate_domain_request_dates script + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Running the script +```./manage.py populate_domain_request_dates --debug``` + +### Running locally +```docker-compose exec app ./manage.py populate_domain_request_dates --debug``` + +##### Optional parameters +| | Parameter | Description | +|:-:|:-------------------------- |:----------------------------------------------------------------------------| +| 1 | **debug** | Increases logging detail. Defaults to False. | From 09ba7450b39d00e6cec1a3e91085ede27715db09 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 27 Aug 2024 15:19:36 -0500 Subject: [PATCH 73/78] linter fixes --- .../management/commands/populate_domain_request_dates.py | 8 ++++---- src/registrar/tests/test_views_requests_json.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_request_dates.py b/src/registrar/management/commands/populate_domain_request_dates.py index cef140cdd..3bbe6b306 100644 --- a/src/registrar/management/commands/populate_domain_request_dates.py +++ b/src/registrar/management/commands/populate_domain_request_dates.py @@ -11,7 +11,7 @@ class Command(BaseCommand, PopulateScriptTemplate): help = "Loops through each domain request object and populates the last_status_update and first_submitted_date" def handle(self, **kwargs): - """Loops through each DomainRequest object and populates + """Loops through each DomainRequest object and populates its last_status_update and first_submitted_date values""" self.mass_update_records(DomainRequest, None, ["last_status_update", "first_submitted_date"]) @@ -34,12 +34,12 @@ class Command(BaseCommand, PopulateScriptTemplate): break logger.info( - f"""{TerminalColors.OKCYAN}Updating {record} => - first submitted date: {record.first_submitted_date}, + f"""{TerminalColors.OKCYAN}Updating {record} => + first submitted date: {record.first_submitted_date}, last status update: {record.last_status_update}{TerminalColors.ENDC} """ ) def should_skip_record(self, record) -> bool: # make sure the record had some kind of history - return not LogEntry.objects.filter(object_pk=record.pk).exists() \ No newline at end of file + return not LogEntry.objects.filter(object_pk=record.pk).exists() diff --git a/src/registrar/tests/test_views_requests_json.py b/src/registrar/tests/test_views_requests_json.py index fd44bdc31..07a65b586 100644 --- a/src/registrar/tests/test_views_requests_json.py +++ b/src/registrar/tests/test_views_requests_json.py @@ -287,7 +287,7 @@ class GetRequestsJsonTest(TestWithUser, WebTest): def test_sorting(self): """test that sorting works properly on the result set""" - response = self.app.get(reverse("get_domain_requests_json"), + response = self.app.get(reverse("get_domain_requests_json"), {"sort_by": "last_submitted_date", "order": "desc"}) self.assertEqual(response.status_code, 200) data = response.json @@ -307,7 +307,7 @@ class GetRequestsJsonTest(TestWithUser, WebTest): def test_filter_approved_excluded(self): """test that approved requests are excluded from result set.""" # sort in reverse chronological order of submission date, since most recent request is approved - response = self.app.get(reverse("get_domain_requests_json"), + response = self.app.get(reverse("get_domain_requests_json"), {"sort_by": "last_submitted_date", "order": "desc"}) self.assertEqual(response.status_code, 200) data = response.json From 635e6e67351594a9242d6d704a94bbc63a91918d Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 28 Aug 2024 09:32:12 -0500 Subject: [PATCH 74/78] linter fixes --- .../commands/populate_domain_request_dates.py | 6 +++--- .../0119_add_domainrequest_submission_dates.py | 6 +++++- src/registrar/tests/test_views_requests_json.py | 10 ++++++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_request_dates.py b/src/registrar/management/commands/populate_domain_request_dates.py index 3bbe6b306..d975a035d 100644 --- a/src/registrar/management/commands/populate_domain_request_dates.py +++ b/src/registrar/management/commands/populate_domain_request_dates.py @@ -22,14 +22,14 @@ class Command(BaseCommand, PopulateScriptTemplate): audit_log_entries = LogEntry.objects.filter(object_pk=record.pk).order_by("-timestamp") # Loop through logs in descending order to find most recent status change for log_entry in audit_log_entries: - if 'status' in log_entry.changes_dict: + if "status" in log_entry.changes_dict: record.last_status_update = log_entry.timestamp.date() break # Loop through logs in ascending order to find first submission for log_entry in audit_log_entries.reverse(): - status = log_entry.changes_dict.get('status') - if status and status[1] == 'submitted': + status = log_entry.changes_dict.get("status") + if status and status[1] == "submitted": record.first_submitted_date = log_entry.timestamp.date() break diff --git a/src/registrar/migrations/0119_add_domainrequest_submission_dates.py b/src/registrar/migrations/0119_add_domainrequest_submission_dates.py index ea209626e..35897e302 100644 --- a/src/registrar/migrations/0119_add_domainrequest_submission_dates.py +++ b/src/registrar/migrations/0119_add_domainrequest_submission_dates.py @@ -26,7 +26,11 @@ class Migration(migrations.Migration): model_name="domainrequest", name="first_submitted_date", field=models.DateField( - blank=True, default=None, help_text="Date initially submitted", null=True, verbose_name="first submitted on" + blank=True, + default=None, + help_text="Date initially submitted", + null=True, + verbose_name="first submitted on", ), ), migrations.AddField( diff --git a/src/registrar/tests/test_views_requests_json.py b/src/registrar/tests/test_views_requests_json.py index 07a65b586..20a4069f7 100644 --- a/src/registrar/tests/test_views_requests_json.py +++ b/src/registrar/tests/test_views_requests_json.py @@ -287,8 +287,9 @@ class GetRequestsJsonTest(TestWithUser, WebTest): def test_sorting(self): """test that sorting works properly on the result set""" - response = self.app.get(reverse("get_domain_requests_json"), - {"sort_by": "last_submitted_date", "order": "desc"}) + response = self.app.get( + reverse("get_domain_requests_json"), {"sort_by": "last_submitted_date", "order": "desc"} + ) self.assertEqual(response.status_code, 200) data = response.json @@ -307,8 +308,9 @@ class GetRequestsJsonTest(TestWithUser, WebTest): def test_filter_approved_excluded(self): """test that approved requests are excluded from result set.""" # sort in reverse chronological order of submission date, since most recent request is approved - response = self.app.get(reverse("get_domain_requests_json"), - {"sort_by": "last_submitted_date", "order": "desc"}) + response = self.app.get( + reverse("get_domain_requests_json"), {"sort_by": "last_submitted_date", "order": "desc"} + ) self.assertEqual(response.status_code, 200) data = response.json From 6849f13e6cba86890f6c2e620d5a2c74d12d98d9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 08:22:16 -0600 Subject: [PATCH 75/78] Update src/registrar/models/user_portfolio_permission.py Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- src/registrar/models/user_portfolio_permission.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index c2cdc61ad..bf1c3e566 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -42,7 +42,7 @@ class UserPortfolioPermission(TimeStampedModel): user = models.ForeignKey( "registrar.User", null=False, - # when a portfolio is deleted, permissions are too + # when a user is deleted, permissions are too on_delete=models.CASCADE, related_name="portfolio_permissions", ) From 06aacd91ce464fd808d792a04f285b34e2954a05 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 29 Aug 2024 10:58:19 -0400 Subject: [PATCH 76/78] minor improvements --- src/registrar/assets/js/get-gov-admin.js | 51 +++++++++--------------- src/registrar/tests/test_api.py | 11 ++++- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 25dd91e0a..24f020b75 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -908,10 +908,6 @@ function initializeWidgetOnList(list, parentId) { return; } - // Hide the contactList initially. - // If we can update the contact information, it'll be shown again. - hideElement(contactList.parentElement); - // Determine if any changes are necessary to the display of portfolio type or federal type // based on changes to the Federal Agency let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; @@ -925,14 +921,15 @@ function initializeWidgetOnList(list, parentId) { console.error("Error in AJAX call: " + data.error); return; } - - let federal_type = data.federal_type; - let portfolio_type = data.portfolio_type; - updateFederalType(data.federal_type); - updatePortfolioType(data.portfolio_type); + updateReadOnly(data.federal_type, '.field-federal_type'); + updateReadOnly(data.portfolio_type, '.field-portfolio_type'); }) .catch(error => console.error("Error fetching federal and portfolio types: ", error)); + // Hide the contactList initially. + // If we can update the contact information, it'll be shown again. + hideElement(contactList.parentElement); + let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) .then(response => { @@ -988,33 +985,21 @@ function initializeWidgetOnList(list, parentId) { } /** - * Dynamically update the portfolio type text in the dom to portfolioType + * Utility that selects a div from the DOM using selectorString, + * and updates a div within that div which has class of 'readonly' + * so that the text of the div is updated to updateText + * @param {*} updateText + * @param {*} selectorString */ - function updatePortfolioType(portfolioType) { - // Find the div with class 'field-portfolio_type' - const portfolioTypeDiv = document.querySelector('.field-portfolio_type'); - if (portfolioTypeDiv) { - // Find the nested div with class 'readonly' inside 'field-portfolio_type' - const readonlyDiv = portfolioTypeDiv.querySelector('.readonly'); + function updateReadOnly(updateText, selectorString) { + // find the div by selectorString + const selectedDiv = document.querySelector(selectorString); + if (selectedDiv) { + // find the nested div with class 'readonly' inside the selectorString div + const readonlyDiv = selectedDiv.querySelector('.readonly'); if (readonlyDiv) { // Update the text content of the readonly div - readonlyDiv.textContent = portfolioType !== null ? portfolioType : '-'; - } - } - } - - /** - * Dynamically update the federal type text in the dom to federalType - */ - function updateFederalType(federalType) { - // Find the div with class 'field-federal_type' - const federalTypeDiv = document.querySelector('.field-federal_type'); - if (federalTypeDiv) { - // Find the nested div with class 'readonly' inside 'field-federal_type' - const readonlyDiv = federalTypeDiv.querySelector('.readonly'); - if (readonlyDiv) { - // Update the text content of the readonly div - readonlyDiv.textContent = federalType !== null ? federalType : '-'; + readonlyDiv.textContent = updateText !== null ? updateText : '-'; } } } diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index 00bc9a1a2..2597e65c2 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -5,6 +5,7 @@ from django.contrib.auth import get_user_model from registrar.tests.common import create_superuser, create_user from api.tests.common import less_console_noise_decorator +from registrar.utility.constants import BranchChoices class GetSeniorOfficialJsonTest(TestCase): @@ -82,7 +83,7 @@ class GetFederalPortfolioTypeJsonTest(TestCase): self.superuser = create_superuser() self.analyst_user = create_user() - self.agency = FederalAgency.objects.create(agency="Test Agency", federal_type="judicial") + self.agency = FederalAgency.objects.create(agency="Test Agency", federal_type=BranchChoices.JUDICIAL) self.api_url = reverse("get-federal-and-portfolio-types-from-federal-agency-json") @@ -100,3 +101,11 @@ class GetFederalPortfolioTypeJsonTest(TestCase): data = response.json() self.assertEqual(data["federal_type"], "Judicial") self.assertEqual(data["portfolio_type"], "Federal - Judicial") + + @less_console_noise_decorator + def test_get_federal_and_portfolio_types_json_authenticated_regularuser(self): + """Test that a regular user receives a 403 with an error message.""" + p = "password" + self.client.login(username="testuser", password=p) + response = self.client.get(self.api_url, {"agency_name": "Test Agency", "organization_type": "federal"}) + self.assertEqual(response.status_code, 302) From ba1553a936aedf26c3d95177e1dddf73ad77a729 Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 29 Aug 2024 11:49:45 -0500 Subject: [PATCH 77/78] add new dates to csv export --- src/registrar/utility/csv_export.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 6f8510418..7ca3b7e97 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1235,7 +1235,9 @@ class DomainRequestExport(BaseExport): "State/territory": model.get("state_territory"), "Request purpose": model.get("purpose"), "CISA regional representative": model.get("cisa_representative_email"), - "Submitted at": model.get("last_submitted_date"), + "Last submitted date": model.get("last_submitted_date"), + "First submitted date": model.get("first_submitted_date"), + "Last status update": model.get("last_status_update"), } row = [FIELDS.get(column, "") for column in columns] @@ -1304,7 +1306,9 @@ class DomainRequestDataFull(DomainRequestExport): """ return [ "Domain request", - "Submitted at", + "Last submitted date", + "First submitted date", + "Last status update", "Status", "Domain type", "Federal type", From fc17854529ecdebd544ba717a67bb979948f069f Mon Sep 17 00:00:00 2001 From: Matthew Spence Date: Thu, 29 Aug 2024 11:52:04 -0500 Subject: [PATCH 78/78] correct migration doc --- docs/operations/data_migration.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index ed2f643d5..5914eb179 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -834,12 +834,7 @@ Example: `cf ssh getgov-za` ```/tmp/lifecycle/shell``` #### Step 4: Running the script -```./manage.py populate_domain_request_dates --debug``` +```./manage.py populate_domain_request_dates``` ### Running locally -```docker-compose exec app ./manage.py populate_domain_request_dates --debug``` - -##### Optional parameters -| | Parameter | Description | -|:-:|:-------------------------- |:----------------------------------------------------------------------------| -| 1 | **debug** | Increases logging detail. Defaults to False. | +```docker-compose exec app ./manage.py populate_domain_request_dates```