From 5d2fec86afed8bb4b11eff1bfac4f618b47e6190 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:32:12 -0600 Subject: [PATCH 01/42] Expose portfolios to analysts --- src/registrar/admin.py | 54 ------------------- .../migrations/0128_create_groups_v17.py | 37 +++++++++++++ src/registrar/models/user_group.py | 15 ++++++ 3 files changed, 52 insertions(+), 54 deletions(-) create mode 100644 src/registrar/migrations/0128_create_groups_v17.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1c8551d4e..5da46f110 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1543,33 +1543,6 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): change_form_template = "django/admin/domain_information_change_form.html" - superuser_only_fields = [ - "portfolio", - "sub_organization", - ] - - # DEVELOPER's NOTE: - # Normally, to exclude a field from an Admin form, we could simply utilize - # Django's "exclude" feature. However, it causes a "missing key" error if we - # go that route for this particular form. The error gets thrown by our - # custom fieldset.html code and is due to the fact that "exclude" removes - # fields from base_fields but not fieldsets. Rather than reworking our - # custom frontend, it seems more straightforward (and easier to read) to simply - # modify the fieldsets list so that it excludes any fields we want to remove - # based on permissions (eg. superuser_only_fields) or other conditions. - def get_fieldsets(self, request, obj=None): - fieldsets = self.fieldsets - - # Create a modified version of fieldsets to exclude certain fields - if not request.user.has_perm("registrar.full_access_permission"): - modified_fieldsets = [] - for name, data in fieldsets: - fields = data.get("fields", []) - fields = tuple(field for field in fields if field not in DomainInformationAdmin.superuser_only_fields) - modified_fieldsets.append((name, {**data, "fields": fields})) - return modified_fieldsets - return fieldsets - def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. We have 1 conditions that determine which fields are read-only: @@ -1865,33 +1838,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] filter_horizontal = ("current_websites", "alternative_domains", "other_contacts") - superuser_only_fields = [ - "portfolio", - "sub_organization", - ] - - # DEVELOPER's NOTE: - # Normally, to exclude a field from an Admin form, we could simply utilize - # Django's "exclude" feature. However, it causes a "missing key" error if we - # go that route for this particular form. The error gets thrown by our - # custom fieldset.html code and is due to the fact that "exclude" removes - # fields from base_fields but not fieldsets. Rather than reworking our - # custom frontend, it seems more straightforward (and easier to read) to simply - # modify the fieldsets list so that it excludes any fields we want to remove - # based on permissions (eg. superuser_only_fields) or other conditions. - def get_fieldsets(self, request, obj=None): - fieldsets = super().get_fieldsets(request, obj) - - # Create a modified version of fieldsets to exclude certain fields - if not request.user.has_perm("registrar.full_access_permission"): - modified_fieldsets = [] - for name, data in fieldsets: - fields = data.get("fields", []) - fields = tuple(field for field in fields if field not in self.superuser_only_fields) - modified_fieldsets.append((name, {**data, "fields": fields})) - return modified_fieldsets - return fieldsets - # Table ordering # NOTE: This impacts the select2 dropdowns (combobox) # Currentl, there's only one for requests on DomainInfo diff --git a/src/registrar/migrations/0128_create_groups_v17.py b/src/registrar/migrations/0128_create_groups_v17.py new file mode 100644 index 000000000..7ac392da7 --- /dev/null +++ b/src/registrar/migrations/0128_create_groups_v17.py @@ -0,0 +1,37 @@ +# This migration creates the create_full_access_group and create_cisa_analyst_group groups +# It is dependent on 0079 (which populates federal agencies) +# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS +# in the user_group model then: +# [NOT RECOMMENDED] +# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions +# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups +# step 3: fake run the latest migration in the migrations list +# [RECOMMENDED] +# Alternatively: +# step 1: duplicate the migration that loads data +# step 2: docker-compose exec app ./manage.py migrate + +from django.db import migrations +from registrar.models import UserGroup +from typing import Any + + +# For linting: RunPython expects a function reference, +# so let's give it one +def create_groups(apps, schema_editor) -> Any: + UserGroup.create_cisa_analyst_group(apps, schema_editor) + UserGroup.create_full_access_group(apps, schema_editor) + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0127_remove_domaininformation_submitter_and_more"), + ] + + operations = [ + migrations.RunPython( + create_groups, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ] diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index 76657fe29..e6bcaa4f4 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -66,6 +66,21 @@ class UserGroup(Group): "model": "federalagency", "permissions": ["add_federalagency", "change_federalagency", "delete_federalagency"], }, + { + "app_label": "registrar", + "model": "portfolio", + "permissions": ["add_portfolio", "change_portfolio", "delete_portfolio"], + }, + { + "app_label": "registrar", + "model": "suborganization", + "permissions": ["add_suborganization", "change_suborganization", "delete_suborganization"], + }, + { + "app_label": "registrar", + "model": "userportfoliopermission", + "permissions": ["add_userportfoliopermission", "change_userportfoliopermission", "delete_userportfoliopermission"], + }, ] # Avoid error: You can't execute queries until the end From 887af431b82e297f068c72ce45eb00c5d5bab6ad Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 23 Sep 2024 09:56:17 -0600 Subject: [PATCH 02/42] review changes: minor tweaks --- src/registrar/admin.py | 99 ++++++++++++++----- src/registrar/assets/js/get-gov-admin.js | 13 ++- .../migrations/0129_portfolio_federal_type.py | 24 +++++ src/registrar/models/portfolio.py | 22 +++-- .../admin/includes/detail_table_fieldset.html | 17 ---- .../admin/includes/portfolio_fieldset.html | 35 +++++++ .../django/admin/portfolio_change_form.html | 2 +- src/registrar/views/utility/api_views.py | 2 +- 8 files changed, 156 insertions(+), 58 deletions(-) create mode 100644 src/registrar/migrations/0129_portfolio_federal_type.py create mode 100644 src/registrar/templates/django/admin/includes/portfolio_fieldset.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5da46f110..74126ab46 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2894,6 +2894,15 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): class PortfolioAdmin(ListHeaderAdmin): + + class Meta: + """Contains meta information about this class""" + + model = models.Portfolio + fields = "__all__" + + _meta = Meta() + change_form_template = "django/admin/portfolio_change_form.html" fieldsets = [ # created_on is the created_at field, and portfolio_type is f"{organization_type} - {federal_type}" @@ -2940,16 +2949,12 @@ class PortfolioAdmin(ListHeaderAdmin): ("Senior official", {"fields": ["senior_official"]}), ] - list_display = ("organization_name", "federal_agency", "creator") + list_display = ("organization_name", "organization_type", "federal_type", "creator") search_fields = ["organization_name"] - search_help_text = "Search by organization name." + search_help_text = "Search by portfolio organization." readonly_fields = [ # This is the created_at field "created_on", - # Django admin doesn't allow methods to be directly listed in fieldsets. We can - # display the custom methods display_admins amd display_members in the admin form if - # they are readonly. - "federal_type", "domains", "domain_requests", "suborganizations", @@ -2959,16 +2964,47 @@ class PortfolioAdmin(ListHeaderAdmin): "creator", ] + analyst_readonly_fields = [ + "organization_name", + "organization_type", + ] + + def get_readonly_fields(self, request, obj=None): + """Set the read-only state on form elements. + We have 2 conditions that determine which fields are read-only: + admin user permissions and the creator's status, so + we'll use the baseline readonly_fields and extend it as needed. + """ + readonly_fields = list(self.readonly_fields) + + # Check if the creator is restricted + if obj and obj.creator.status == models.User.RESTRICTED: + # For fields like CharField, IntegerField, etc., the widget used is + # straightforward and the readonly_fields list can control their behavior + readonly_fields.extend([field.name for field in self.model._meta.fields]) + + if request.user.has_perm("registrar.full_access_permission"): + return readonly_fields + + # Return restrictive Read-only fields for analysts and + # users who might not belong to groups + readonly_fields.extend([field for field in self.analyst_readonly_fields]) + return readonly_fields + def get_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio - admin_permissions = UserPortfolioPermission.objects.filter( - portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) + admin_permissions = self.get_user_portfolio_permission_admins(obj) # Get the user objects associated with these permissions admin_users = User.objects.filter(portfolio_permissions__in=admin_permissions) return admin_users + + def get_user_portfolio_permission_admins(self, obj): + """Returns each admin on UserPortfolioPermission for a given portfolio.""" + return obj.portfolio_users.filter( + portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) def get_non_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio that do NOT have the "Admin" role @@ -2980,6 +3016,13 @@ class PortfolioAdmin(ListHeaderAdmin): non_admin_users = User.objects.filter(portfolio_permissions__in=non_admin_permissions) return non_admin_users + + def get_user_portfolio_permission_non_admins(self, obj): + """Returns each admin on UserPortfolioPermission for a given portfolio.""" + return obj.portfolio_users.exclude( + roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + def display_admins(self, obj): """Get joined users who are Admin, unpack and return an HTML block. @@ -2989,19 +3032,23 @@ class PortfolioAdmin(ListHeaderAdmin): data would display in a custom change form without extensive template customization. Will be used in the field_readonly block""" - admins = self.get_admin_users(obj) + admins = self.get_user_portfolio_permission_admins(obj) if not admins: return format_html("

No admins found.

") admin_details = "" - for portfolio_admin in admins: - change_url = reverse("admin:registrar_user_change", args=[portfolio_admin.pk]) - admin_details += "
" - admin_details += f'{escape(portfolio_admin)}
' - admin_details += f"{escape(portfolio_admin.title)}
" - admin_details += f"{escape(portfolio_admin.email)}" + for i, portfolio_admin in enumerate(admins): + change_url = reverse("admin:registrar_userportfoliopermission_change", args=[portfolio_admin.pk]) + + address_id = f"portfolio-administrator-{portfolio_admin.pk}" + if len(admins) > 1: + admin_details += f'' + admin_details += f'
' + admin_details += f'{escape(portfolio_admin.user)}
' + admin_details += f"{escape(portfolio_admin.user.title)}
" + admin_details += f"{escape(portfolio_admin.user.email)}" admin_details += "
" - admin_details += f"" + admin_details += f"" admin_details += ( "" admin_details += "

" - admin_details += f"{escape(portfolio_admin.phone)}" + admin_details += f"{escape(portfolio_admin.user.phone)}" admin_details += "
" return format_html(admin_details) @@ -3026,7 +3073,7 @@ class PortfolioAdmin(ListHeaderAdmin): data would display in a custom change form without extensive template customization. Will be used in the after_help_text block.""" - members = self.get_non_admin_users(obj) + members = self.get_user_portfolio_permission_non_admins(obj) if not members: return "" @@ -3035,14 +3082,14 @@ class PortfolioAdmin(ListHeaderAdmin): + "PhoneRoles" ) for member in members: - full_name = member.get_formatted_name() + full_name = member.user.get_formatted_name() member_details += "" member_details += f"{escape(full_name)}" - member_details += f"{escape(member.title)}" - member_details += f"{escape(member.email)}" - member_details += f"{escape(member.phone)}" + member_details += f"{escape(member.user.title)}" + member_details += f"{escape(member.user.email)}" + member_details += f"{escape(member.user.phone)}" member_details += "" - for role in member.portfolio_role_summary(obj): + for role in member.user.portfolio_role_summary(obj): member_details += f"{escape(role)} " member_details += "" member_details += "" @@ -3052,11 +3099,11 @@ class PortfolioAdmin(ListHeaderAdmin): def display_members_summary(self, obj): """Will be passed as context and used in the field_readonly block.""" - members = self.get_non_admin_users(obj) + members = self.get_user_portfolio_permission_non_admins(obj) if not members: return {} - return self.get_field_links_as_list(members, "user", separator=", ") + return self.get_field_links_as_list(members, "userportfoliopermission", attribute_name="user", separator=", ") def federal_type(self, obj: models.Portfolio): """Returns the federal_type field""" diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 7ff02ba1f..f621d5b07 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -858,10 +858,11 @@ function initializeWidgetOnList(list, parentId) { // $ symbolically denotes that this is using jQuery let $federalAgency = django.jQuery("#id_federal_agency"); let organizationType = document.getElementById("id_organization_type"); - if ($federalAgency && organizationType) { + let federalType = document.getElementById("id_federal_type") + if ($federalAgency && organizationType && federalType) { // Attach the change event listener $federalAgency.on("change", function() { - handleFederalAgencyChange($federalAgency, organizationType); + handleFederalAgencyChange($federalAgency, organizationType, federalType); }); } @@ -879,7 +880,7 @@ function initializeWidgetOnList(list, parentId) { } }); - function handleFederalAgencyChange(federalAgency, organizationType) { + function handleFederalAgencyChange(federalAgency, organizationType, federalType) { // Don't do anything on page load if (isInitialPageLoad) { isInitialPageLoad = false; @@ -924,7 +925,11 @@ function initializeWidgetOnList(list, parentId) { console.error("Error in AJAX call: " + data.error); return; } - updateReadOnly(data.federal_type, '.field-federal_type'); + if (data.federal_type && selectedText !== "Non-Federal Agency") { + federalType.value = data.federal_type.toLowerCase(); + }else { + federalType.value = ""; + } updateReadOnly(data.portfolio_type, '.field-portfolio_type'); }) .catch(error => console.error("Error fetching federal and portfolio types: ", error)); diff --git a/src/registrar/migrations/0129_portfolio_federal_type.py b/src/registrar/migrations/0129_portfolio_federal_type.py new file mode 100644 index 000000000..79548f314 --- /dev/null +++ b/src/registrar/migrations/0129_portfolio_federal_type.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.10 on 2024-09-23 15:29 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0128_create_groups_v17"), + ] + + operations = [ + migrations.AddField( + model_name="portfolio", + name="federal_type", + field=models.CharField( + blank=True, + choices=[("executive", "Executive"), ("judicial", "Judicial"), ("legislative", "Legislative")], + help_text="Federal agency type (executive, judicial, legislative, etc.)", + max_length=20, + null=True, + ), + ), + ] diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index fadcf8cac..2cef1446f 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -58,6 +58,14 @@ class Portfolio(TimeStampedModel): default=FederalAgency.get_non_federal_agency, ) + federal_type = models.CharField( + max_length=20, + choices=BranchChoices.choices, + null=True, + blank=True, + help_text="Federal agency type (executive, judicial, legislative, etc.)", + ) + senior_official = models.ForeignKey( "registrar.SeniorOfficial", on_delete=models.PROTECT, @@ -123,8 +131,13 @@ class Portfolio(TimeStampedModel): if self.state_territory != self.StateTerritoryChoices.PUERTO_RICO and self.urbanization: self.urbanization = None + # Set the federal type field if it doesn't exist already + if self.federal_type is None and self.federal_agency and self.federal_agency.federal_type: + self.federal_type = self.federal_agency.federal_type if self.federal_agency else None + super().save(*args, **kwargs) + @property def portfolio_type(self): """ @@ -142,15 +155,6 @@ class Portfolio(TimeStampedModel): else: return org_type_label - @property - def federal_type(self): - """Returns the federal_type value on the underlying federal_agency field""" - 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/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index e22bcb571..e03203f4b 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -137,16 +137,6 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endfor %} {% endwith %} - {% elif field.field.name == "display_admins" %} -
{{ field.contents|safe }}
- {% elif field.field.name == "display_members" %} -
- {% if display_members_summary %} - {{ display_members_summary }} - {% else %} -

No additional members found.

- {% endif %} -
{% else %}
{{ field.contents }}
{% endif %} @@ -335,13 +325,6 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endif %} {% endwith %} - {% elif field.field.name == "display_members" and field.contents %} -
- Details -
- {{ field.contents|safe }} -
-
{% elif field.field.name == "state_territory" and original_object|model_name_lowercase != 'portfolio' %}
diff --git a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html new file mode 100644 index 000000000..001052b84 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html @@ -0,0 +1,35 @@ +{% extends "django/admin/includes/detail_table_fieldset.html" %} +{% load custom_filters %} +{% load static url_helpers %} + +{% block field_readonly %} + {% if field.field.name == "display_admins" %} +
{{ field.contents|safe }}
+ {% elif field.field.name == "display_members" %} +
+ {% if display_members_summary %} + {{ display_members_summary }} + {% else %} +

No additional members found.

+ {% endif %} +
+ {% else %} +
{{ field.contents }}
+ {% endif %} +{% endblock field_readonly%} + +{% block after_help_text %} + {% if field.field.name == "senior_official" %} +
+ + {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly %} +
+ {% elif field.field.name == "display_members" and field.contents %} +
+ Details +
+ {{ field.contents|safe }} +
+
+ {% endif %} +{% endblock after_help_text %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 8dae8a080..38b155ce2 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -20,7 +20,7 @@ When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. {% endcomment %} - {% include "django/admin/includes/detail_table_fieldset.html" with original_object=original %} + {% include "django/admin/includes/portfolio_fieldset.html" with original_object=original %} {% endfor %} {% endblock %} diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 97eb7e86c..430c3416b 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -58,7 +58,7 @@ def get_federal_and_portfolio_types_from_federal_agency_json(request): organization_type = request.GET.get("organization_type") agency = FederalAgency.objects.filter(agency=agency_name).first() if agency: - federal_type = Portfolio.get_federal_type(agency) + federal_type = agency.federal_type portfolio_type = Portfolio.get_portfolio_type(organization_type, federal_type) federal_type = BranchChoices.get_branch_label(federal_type) if federal_type else "-" From 83615cf3fce300c8ebbc3aebb3158bc322c3d5ba Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 23 Sep 2024 10:16:05 -0600 Subject: [PATCH 03/42] Add bold for organization label --- src/registrar/admin.py | 2 +- src/registrar/assets/sass/_theme/_admin.scss | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 74126ab46..c25593dca 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3042,7 +3042,7 @@ class PortfolioAdmin(ListHeaderAdmin): address_id = f"portfolio-administrator-{portfolio_admin.pk}" if len(admins) > 1: - admin_details += f'' + admin_details += f'' admin_details += f'
' admin_details += f'{escape(portfolio_admin.user)}
' admin_details += f"{escape(portfolio_admin.user.title)}
" diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index ef1a810ac..84a74a853 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -930,3 +930,8 @@ ul.add-list-reset { .dl-dja dt { font-size: 14px; } + +.organization-admin-label { + font-weight: 600; + font-size: .8125rem; +} From 280bc23fa6d29cac792e65f57356ce41c2fbbb0a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 23 Sep 2024 11:29:01 -0600 Subject: [PATCH 04/42] Fix migrations + readonly views --- src/registrar/admin.py | 11 ++++++- src/registrar/assets/js/get-gov-admin.js | 29 ++++++++++++++----- .../migrations/0129_portfolio_federal_type.py | 2 +- ...roups_v17.py => 0130_create_groups_v17.py} | 2 +- src/registrar/models/user_group.py | 2 +- .../models/user_portfolio_permission.py | 19 +++++++++++- .../models/utility/portfolio_helper.py | 8 +++++ .../admin/includes/portfolio_fieldset.html | 16 ++++++++++ .../user_portfolio_permission_fieldset.html | 26 +++++++++++++++++ ...user_portfolio_permission_change_form.html | 16 ++++++++++ 10 files changed, 118 insertions(+), 13 deletions(-) rename src/registrar/migrations/{0128_create_groups_v17.py => 0130_create_groups_v17.py} (94%) create mode 100644 src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html create mode 100644 src/registrar/templates/django/admin/user_portfolio_permission_change_form.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 10e6a9176..904701be7 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1242,8 +1242,10 @@ class UserDomainRoleResource(resources.ModelResource): model = models.UserDomainRole +# Note: This is "viewonly" for analysts class UserPortfolioPermissionAdmin(ListHeaderAdmin): form = UserPortfolioPermissionsForm + change_form_template = "django/admin/user_portfolio_permission_change_form.html" class Meta: """Contains meta information about this class""" @@ -1261,6 +1263,14 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): autocomplete_fields = ["user", "portfolio"] + def change_view(self, request, object_id, form_url="", extra_context=None): + """Adds a readonly display for roles and permissions""" + obj = self.get_object(request, object_id) + extra_context = extra_context or {} + extra_context["display_roles"] = ", ".join(obj.get_readable_roles()) + extra_context["display_permissions"] = ", ".join(obj.get_readable_additional_permissions()) + return super().change_view(request, object_id, form_url, extra_context) + class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" @@ -2961,7 +2971,6 @@ class PortfolioAdmin(ListHeaderAdmin): analyst_readonly_fields = [ "organization_name", - "organization_type", ] def get_readonly_fields(self, request, obj=None): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f621d5b07..fd673878d 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -858,11 +858,13 @@ function initializeWidgetOnList(list, parentId) { // $ symbolically denotes that this is using jQuery let $federalAgency = django.jQuery("#id_federal_agency"); let organizationType = document.getElementById("id_organization_type"); + let readonlyOrganizationType = document.querySelector(".field-organization_type .readonly"); + let federalType = document.getElementById("id_federal_type") - if ($federalAgency && organizationType && federalType) { + if ($federalAgency && (organizationType || readonlyOrganizationType) && federalType) { // Attach the change event listener $federalAgency.on("change", function() { - handleFederalAgencyChange($federalAgency, organizationType, federalType); + handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType, federalType); }); } @@ -880,7 +882,7 @@ function initializeWidgetOnList(list, parentId) { } }); - function handleFederalAgencyChange(federalAgency, organizationType, federalType) { + function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType, federalType) { // Don't do anything on page load if (isInitialPageLoad) { isInitialPageLoad = false; @@ -895,13 +897,22 @@ function initializeWidgetOnList(list, parentId) { return; } + let organizationTypeValue = organizationType ? organizationType.value : readonlyOrganizationType.innerText.toLowerCase(); if (selectedText !== "Non-Federal Agency") { - if (organizationType.value !== "federal") { - organizationType.value = "federal"; + if (organizationTypeValue !== "federal") { + if (organizationType){ + organizationType.value = "federal"; + }else { + readonlyOrganizationType.innerText = "Federal" + } } }else { - if (organizationType.value === "federal") { - organizationType.value = ""; + if (organizationTypeValue === "federal") { + if (organizationType){ + organizationType.value = ""; + }else { + readonlyOrganizationType.innerText = "-" + } } } @@ -912,10 +923,12 @@ function initializeWidgetOnList(list, parentId) { return; } + organizationTypeValue = organizationType ? organizationType.value : readonlyOrganizationType.innerText.toLowerCase(); + // 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}`) + fetch(`${federalPortfolioApi}?organization_type=${organizationTypeValue}&agency_name=${selectedText}`) .then(response => { const statusCode = response.status; return response.json().then(data => ({ statusCode, data })); diff --git a/src/registrar/migrations/0129_portfolio_federal_type.py b/src/registrar/migrations/0129_portfolio_federal_type.py index 79548f314..c6d874678 100644 --- a/src/registrar/migrations/0129_portfolio_federal_type.py +++ b/src/registrar/migrations/0129_portfolio_federal_type.py @@ -6,7 +6,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ("registrar", "0128_create_groups_v17"), + ("registrar", "0128_alter_domaininformation_state_territory_and_more"), ] operations = [ diff --git a/src/registrar/migrations/0128_create_groups_v17.py b/src/registrar/migrations/0130_create_groups_v17.py similarity index 94% rename from src/registrar/migrations/0128_create_groups_v17.py rename to src/registrar/migrations/0130_create_groups_v17.py index 7ac392da7..943507267 100644 --- a/src/registrar/migrations/0128_create_groups_v17.py +++ b/src/registrar/migrations/0130_create_groups_v17.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0127_remove_domaininformation_submitter_and_more"), + ("registrar", "0129_portfolio_federal_type"), ] operations = [ diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index e6bcaa4f4..5c9bea8b4 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -79,7 +79,7 @@ class UserGroup(Group): { "app_label": "registrar", "model": "userportfoliopermission", - "permissions": ["add_userportfoliopermission", "change_userportfoliopermission", "delete_userportfoliopermission"], + "permissions": ["view_userportfoliopermission"], }, ] diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index f7bafc8c6..29e900ab6 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -75,7 +75,24 @@ class UserPortfolioPermission(TimeStampedModel): ) def __str__(self): - return f"User '{self.user}' on Portfolio '{self.portfolio}' " f"" if self.roles else "" + readable_roles = [] + if self.roles: + readable_roles = sorted([UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles]) + return f"{self.user}' " f"" if self.roles else "" + + def get_readable_roles(self): + """Returns a list of labels of each role in self.roles""" + role_labels = [] + for role in self.roles: + role_labels.append(UserPortfolioRoleChoices.get_user_portfolio_role_label(role)) + return role_labels + + def get_readable_additional_permissions(self): + """Returns a list of labels of each additional_permission in self.additional_permissions""" + perm_labels = [] + for perm in self.additional_permissions: + perm_labels.append(UserPortfolioPermissionChoices.get_user_portfolio_permission_label(perm)) + return perm_labels def _get_portfolio_permissions(self): """ diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 7f34221fd..24800fd19 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -10,6 +10,10 @@ class UserPortfolioRoleChoices(models.TextChoices): ORGANIZATION_ADMIN_READ_ONLY = "organization_admin_read_only", "Admin read only" ORGANIZATION_MEMBER = "organization_member", "Member" + @classmethod + def get_user_portfolio_role_label(cls, user_portfolio_role): + return cls(user_portfolio_role).label if user_portfolio_role else None + class UserPortfolioPermissionChoices(models.TextChoices): """ """ @@ -29,3 +33,7 @@ class UserPortfolioPermissionChoices(models.TextChoices): # Domain: field specific permissions VIEW_SUBORGANIZATION = "view_suborganization", "View suborganization" EDIT_SUBORGANIZATION = "edit_suborganization", "Edit suborganization" + + @classmethod + def get_user_portfolio_permission_label(cls, user_portfolio_permission): + return cls(user_portfolio_permission).label if user_portfolio_permission else None \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html index 001052b84..aae096a2e 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html @@ -13,6 +13,22 @@

No additional members found.

{% endif %}
+ {% elif field.field.name == "roles" %} +
+ {% if get_readable_roles %} + {{ get_readable_roles }} + {% else %} +

No roles found.

+ {% endif %} +
+ {% elif field.field.name == "additional_permissions" %} +
+ {% if display_permissions %} + {{ display_permissions }} + {% else %} +

No additional permissions found.

+ {% endif %} +
{% else %}
{{ field.contents }}
{% endif %} diff --git a/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html b/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html new file mode 100644 index 000000000..a7ffc889c --- /dev/null +++ b/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html @@ -0,0 +1,26 @@ +{% extends "django/admin/includes/detail_table_fieldset.html" %} +{% load custom_filters %} +{% load static url_helpers %} + + +{% block field_readonly %} + {% if field.field.name == "roles" %} +
+ {% if display_roles %} + {{ display_roles }} + {% else %} + No roles found. + {% endif %} +
+ {% elif field.field.name == "additional_permissions" %} +
+ {% if display_permissions %} + {{ display_permissions }} + {% else %} + No additional permissions found. + {% endif %} +
+ {% else %} +
{{ field.contents }}
+ {% endif %} +{% endblock field_readonly%} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html b/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html new file mode 100644 index 000000000..1249a486c --- /dev/null +++ b/src/registrar/templates/django/admin/user_portfolio_permission_change_form.html @@ -0,0 +1,16 @@ +{% extends 'django/admin/email_clipboard_change_form.html' %} +{% load custom_filters %} +{% load i18n static %} + +{% block field_sets %} + {% for fieldset in adminform %} + {% comment %} + This is a placeholder for now. + + Disclaimer: + When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. + detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. + {% endcomment %} + {% include "django/admin/includes/user_portfolio_permission_fieldset.html" with original_object=original %} + {% endfor %} +{% endblock %} \ No newline at end of file From 4597133186a66a98acc5953b057013df948a142b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 23 Sep 2024 11:41:37 -0600 Subject: [PATCH 05/42] Update _admin.scss --- src/registrar/assets/sass/_theme/_admin.scss | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 3e6d0d171..b83882340 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -937,4 +937,9 @@ ul.add-list-reset { overflow: visible; word-break: break-all; max-width: 100%; - } \ No newline at end of file +} + +.organization-admin-label { + font-weight: 600; + font-size: .8125rem; +} From aa926ec88baf393c473c5b11ea4ea8dee95da9a2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 23 Sep 2024 11:47:43 -0600 Subject: [PATCH 06/42] lint + tests --- src/registrar/admin.py | 22 ---------------------- src/registrar/tests/test_admin.py | 16 ++++++++-------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 904701be7..338311f67 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3227,28 +3227,6 @@ class PortfolioAdmin(ListHeaderAdmin): return self.add_fieldsets return super().get_fieldsets(request, obj) - def get_readonly_fields(self, request, obj=None): - """Set the read-only state on form elements. - We have 2 conditions that determine which fields are read-only: - admin user permissions and the creator's status, so - we'll use the baseline readonly_fields and extend it as needed. - """ - readonly_fields = list(self.readonly_fields) - - # Check if the creator is restricted - if obj and obj.creator.status == models.User.RESTRICTED: - # For fields like CharField, IntegerField, etc., the widget used is - # straightforward and the readonly_fields list can control their behavior - readonly_fields.extend([field.name for field in self.model._meta.fields]) - - if request.user.has_perm("registrar.full_access_permission"): - return readonly_fields - - # Return restrictive Read-only fields for analysts and - # users who might not belong to groups - readonly_fields.extend([field for field in self.analyst_readonly_fields]) - return readonly_fields - def change_view(self, request, object_id, form_url="", extra_context=None): """Add related suborganizations and domain groups. Add the summary for the portfolio members field (list of members that link to change_forms).""" diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 88482e4db..fd55e83f1 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2051,7 +2051,7 @@ class TestPortfolioAdmin(TestCase): email="meaoward@gov.gov", ) - UserPortfolioPermission.objects.all().create( + perm_1 = UserPortfolioPermission.objects.all().create( user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) @@ -2063,7 +2063,7 @@ class TestPortfolioAdmin(TestCase): email="poopy@gov.gov", ) - UserPortfolioPermission.objects.all().create( + perm_2 = UserPortfolioPermission.objects.all().create( user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) @@ -2075,7 +2075,7 @@ class TestPortfolioAdmin(TestCase): email="madmax@gov.gov", ) - UserPortfolioPermission.objects.all().create( + perm_3 = UserPortfolioPermission.objects.all().create( user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] ) @@ -2087,7 +2087,7 @@ class TestPortfolioAdmin(TestCase): email="thematrix@gov.gov", ) - UserPortfolioPermission.objects.all().create( + perm_4 = UserPortfolioPermission.objects.all().create( user=admin_user_4, portfolio=self.portfolio, additional_permissions=[ @@ -2099,23 +2099,23 @@ class TestPortfolioAdmin(TestCase): display_admins = self.admin.display_admins(self.portfolio) self.assertIn( - f'Gerald Meoward meaoward@gov.gov', + f'Gerald Meoward meaoward@gov.gov', display_admins, ) self.assertIn("Captain", display_admins) self.assertIn( - f'Arnold Poopy poopy@gov.gov', display_admins + f'Arnold Poopy poopy@gov.gov', display_admins ) self.assertIn("Major", display_admins) display_members_summary = self.admin.display_members_summary(self.portfolio) self.assertIn( - f'Mad Max madmax@gov.gov', + f'Mad Max madmax@gov.gov', display_members_summary, ) self.assertIn( - f'Agent Smith thematrix@gov.gov', + f'Agent Smith thematrix@gov.gov', display_members_summary, ) From d8227148f61e5d92bbdbe30ddd4b35cfe9c0dff4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 23 Sep 2024 12:20:53 -0600 Subject: [PATCH 07/42] Fix tests and lint --- src/registrar/models/portfolio.py | 1 - .../models/user_portfolio_permission.py | 6 ++++-- src/registrar/models/utility/portfolio_helper.py | 2 +- src/registrar/tests/test_admin.py | 16 ++++++++++------ src/registrar/tests/test_migrations.py | 7 +++++++ 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 7a2a4b5b7..0a60d3ec8 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -137,7 +137,6 @@ class Portfolio(TimeStampedModel): super().save(*args, **kwargs) - @property def portfolio_type(self): """ diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 29e900ab6..a55c04e5c 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -77,7 +77,9 @@ class UserPortfolioPermission(TimeStampedModel): def __str__(self): readable_roles = [] if self.roles: - readable_roles = sorted([UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles]) + readable_roles = sorted( + [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles] + ) return f"{self.user}' " f"" if self.roles else "" def get_readable_roles(self): @@ -86,7 +88,7 @@ class UserPortfolioPermission(TimeStampedModel): for role in self.roles: role_labels.append(UserPortfolioRoleChoices.get_user_portfolio_role_label(role)) return role_labels - + def get_readable_additional_permissions(self): """Returns a list of labels of each additional_permission in self.additional_permissions""" perm_labels = [] diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 24800fd19..ef9336f18 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -36,4 +36,4 @@ class UserPortfolioPermissionChoices(models.TextChoices): @classmethod def get_user_portfolio_permission_label(cls, user_portfolio_permission): - return cls(user_portfolio_permission).label if user_portfolio_permission else None \ No newline at end of file + return cls(user_portfolio_permission).label if user_portfolio_permission else None diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index fd55e83f1..daf5e7086 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2098,24 +2098,28 @@ class TestPortfolioAdmin(TestCase): display_admins = self.admin.display_admins(self.portfolio) + url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_1.pk]) self.assertIn( - f'Gerald Meoward meaoward@gov.gov', + f'Gerald Meoward meaoward@gov.gov', display_admins, ) self.assertIn("Captain", display_admins) - self.assertIn( - f'Arnold Poopy poopy@gov.gov', display_admins - ) + + url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_2.pk]) + self.assertIn(f'Arnold Poopy poopy@gov.gov', display_admins) self.assertIn("Major", display_admins) display_members_summary = self.admin.display_members_summary(self.portfolio) + url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_3.pk]) self.assertIn( - f'Mad Max madmax@gov.gov', + f'Mad Max madmax@gov.gov', display_members_summary, ) + + url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_4.pk]) self.assertIn( - f'Agent Smith thematrix@gov.gov', + f'Agent Smith thematrix@gov.gov', display_members_summary, ) diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index 6d8ff7151..5dde4831c 100644 --- a/src/registrar/tests/test_migrations.py +++ b/src/registrar/tests/test_migrations.py @@ -40,10 +40,17 @@ class TestGroups(TestCase): "add_federalagency", "change_federalagency", "delete_federalagency", + "add_portfolio", + "change_portfolio", + "delete_portfolio", + "add_suborganization", + "change_suborganization", + "delete_suborganization", "analyst_access_permission", "change_user", "delete_userdomainrole", "view_userdomainrole", + "view_userportfoliopermission", "add_verifiedbystaff", "change_verifiedbystaff", "delete_verifiedbystaff", From 9ccea3f776ff45e4fbceac2c4fe51131d9092754 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 23 Sep 2024 12:21:02 -0600 Subject: [PATCH 08/42] Update admin.py --- src/registrar/admin.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 338311f67..9342460fa 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2899,7 +2899,7 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): class PortfolioAdmin(ListHeaderAdmin): - + class Meta: """Contains meta information about this class""" @@ -3003,12 +3003,10 @@ class PortfolioAdmin(ListHeaderAdmin): admin_users = User.objects.filter(portfolio_permissions__in=admin_permissions) return admin_users - + def get_user_portfolio_permission_admins(self, obj): """Returns each admin on UserPortfolioPermission for a given portfolio.""" - return obj.portfolio_users.filter( - portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) + return obj.portfolio_users.filter(portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) def get_non_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio that do NOT have the "Admin" role @@ -3020,13 +3018,10 @@ class PortfolioAdmin(ListHeaderAdmin): non_admin_users = User.objects.filter(portfolio_permissions__in=non_admin_permissions) return non_admin_users - + def get_user_portfolio_permission_non_admins(self, obj): """Returns each admin on UserPortfolioPermission for a given portfolio.""" - return obj.portfolio_users.exclude( - roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - + return obj.portfolio_users.exclude(roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) def display_admins(self, obj): """Get joined users who are Admin, unpack and return an HTML block. @@ -3046,13 +3041,17 @@ class PortfolioAdmin(ListHeaderAdmin): address_id = f"portfolio-administrator-{portfolio_admin.pk}" if len(admins) > 1: - admin_details += f'' + admin_details += ( + f'' + ) admin_details += f'
' admin_details += f'{escape(portfolio_admin.user)}
' admin_details += f"{escape(portfolio_admin.user.title)}
" admin_details += f"{escape(portfolio_admin.user.email)}" admin_details += "
" - admin_details += f"" + admin_details += ( + f"" + ) admin_details += ( "" - admin_details += "

" - admin_details += f"{escape(portfolio_admin.user.phone)}" - admin_details += "
" - return format_html(admin_details) - - display_admins.short_description = "Administrators" # type: ignore - - def display_members(self, obj): - """Get joined users who have roles/perms that are not Admin, unpack and return an HTML block. - - DJA readonly can't handle querysets, so we need to unpack and return html here. - Alternatively, we could return querysets in context but that would limit where this - data would display in a custom change form without extensive template customization. - - Will be used in the after_help_text block.""" - members = self.get_user_portfolio_permission_non_admins(obj) - if not members: - return "" - - member_details = ( - "" - + "" - ) - for member in members: - full_name = member.user.get_formatted_name() - member_details += "" - member_details += f"" - member_details += f"" - member_details += f"" - member_details += f"" - member_details += "" - member_details += "
NameTitleEmailPhoneRoles
{escape(full_name)}{escape(member.user.title)}{escape(member.user.email)}{escape(member.user.phone)}" - for role in member.user.portfolio_role_summary(obj): - member_details += f"{escape(role)} " - member_details += "
" - return format_html(member_details) - - display_members.short_description = "Members" # type: ignore - - def display_members_summary(self, obj): - """Will be passed as context and used in the field_readonly block.""" - members = self.get_user_portfolio_permission_non_admins(obj) - if not members: - return {} - - return self.get_field_links_as_list(members, "userportfoliopermission", attribute_name="user", separator=", ") - def federal_type(self, obj: models.Portfolio): """Returns the federal_type field""" return BranchChoices.get_branch_label(obj.federal_type) if obj.federal_type else "-" @@ -3181,6 +3096,28 @@ class PortfolioAdmin(ListHeaderAdmin): domain_requests.short_description = "Domain requests" # type: ignore + def display_admins(self, obj): + """Returns the number of administrators for this portfolio""" + admin_count = len(self.get_user_portfolio_permission_admins(obj)) + if admin_count > 0: + url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={obj.id}" + # Create a clickable link with the count + return format_html(f'{admin_count} administrators') + return "No administrators found." + + display_admins.short_description = "Administrators" # type: ignore + + def display_members(self, obj): + """Returns the number of members for this portfolio""" + member_count = len(self.get_user_portfolio_permission_non_admins(obj)) + if member_count > 0: + url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={obj.id}" + # Create a clickable link with the count + return format_html(f'{member_count} members') + return "No additional members found." + + display_members.short_description = "Members" # type: ignore + # Creates select2 fields (with search bars) autocomplete_fields = [ "creator", @@ -3254,7 +3191,8 @@ class PortfolioAdmin(ListHeaderAdmin): obj = self.get_object(request, object_id) extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True - extra_context["display_members_summary"] = self.display_members_summary(obj) + extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) + extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 032d9f84f..117be405d 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -882,7 +882,10 @@ function initializeWidgetOnList(list, parentId) { // Handle hiding the organization name field when the organization_type is federal. // Run this first one page load, then secondly on a change event. - let organizationNameContainer = document.querySelector(".field-organization_name") + let organizationNameContainer = document.querySelector(".field-organization_name"); + if (!organizationNameContainer) { + organizationNameContainer = document.querySelector("#id_organization_name"); + } handleOrganizationTypeChange(organizationType, organizationNameContainer); organizationType.addEventListener("change", function() { handleOrganizationTypeChange(organizationType, organizationNameContainer); diff --git a/src/registrar/templates/django/admin/includes/portfolio_admins_table.html b/src/registrar/templates/django/admin/includes/portfolio_admins_table.html new file mode 100644 index 000000000..78bc53a38 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio_admins_table.html @@ -0,0 +1,42 @@ +{% load static url_helpers %} + +
+ Details +
+ + + + + + + + + + + {% for admin in admins %} + {% url 'admin:registrar_userportfoliopermission_change' admin.pk as url %} + + + + + + + + {% endfor %} + +
NameTitleEmailPhone
{{ admin.user.get_formatted_name}}{{ admin.user.title }}{{ admin.user.email }}{{ admin.user.phone }} + + +
+
+
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html index aae096a2e..16f637a4e 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html @@ -3,16 +3,8 @@ {% load static url_helpers %} {% block field_readonly %} - {% if field.field.name == "display_admins" %} + {% if field.field.name == "display_admins" or field.field.name == "display_members" %}
{{ field.contents|safe }}
- {% elif field.field.name == "display_members" %} -
- {% if display_members_summary %} - {{ display_members_summary }} - {% else %} -

No additional members found.

- {% endif %} -
{% elif field.field.name == "roles" %}
{% if get_readable_roles %} @@ -40,12 +32,9 @@ {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly %}
- {% elif field.field.name == "display_members" and field.contents %} -
- Details -
- {{ field.contents|safe }} -
-
+ {% elif field.field.name == "display_admins" %} + {% include "django/admin/includes/portfolio_admins_table.html" with admins=admins %} + {% elif field.field.name == "display_members" %} + {% include "django/admin/includes/portfolio_members_table.html" with members=members %} {% endif %} {% endblock after_help_text %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio_members_table.html new file mode 100644 index 000000000..44b954077 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio_members_table.html @@ -0,0 +1,49 @@ +{% load custom_filters %} +{% load static url_helpers %} + +
+ Details +
+ + + + + + + + + + + + {% for member in members %} + {% url 'admin:registrar_userportfoliopermission_change' member.pk as url %} + + + + + + + + + {% endfor %} + +
NameTitleEmailPhoneRoles
{{ member.user.get_formatted_name}}{{ member.user.title }}{{ member.user.email }}{{ member.user.phone }} + {% for role in member.user|portfolio_role_summary:original %} + {{ role }} + {% endfor %} + + + +
+
+
\ No newline at end of file diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index c6c7c97d1..de9b7bfa1 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -239,3 +239,12 @@ def is_portfolio_subpage(path): "senior-official", ] return get_url_name(path) in url_names + + +@register.filter(name="portfolio_role_summary") +def portfolio_role_summary(user, portfolio): + """Returns the value of user.portfolio_role_summary""" + if user and portfolio: + return user.portfolio_role_summary(portfolio) + else: + return [] \ No newline at end of file From 707f9a0e60ad29d430dcc1b66a0606e2f9cc9c8f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 25 Sep 2024 10:54:44 -0600 Subject: [PATCH 13/42] Hide details when no data exists --- .../django/admin/includes/portfolio_fieldset.html | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html index 16f637a4e..2299a579a 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html @@ -33,8 +33,12 @@ {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly %} {% elif field.field.name == "display_admins" %} - {% include "django/admin/includes/portfolio_admins_table.html" with admins=admins %} + {% if admins|length > 0 %} + {% include "django/admin/includes/portfolio_admins_table.html" with admins=admins %} + {% endif %} {% elif field.field.name == "display_members" %} - {% include "django/admin/includes/portfolio_members_table.html" with members=members %} + {% if members|length > 0 %} + {% include "django/admin/includes/portfolio_members_table.html" with members=members %} + {% endif %} {% endif %} {% endblock after_help_text %} \ No newline at end of file From d6542f18b12660a75ab0de126ffb6d353e62f0f8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 25 Sep 2024 11:03:41 -0600 Subject: [PATCH 14/42] Handle blank emails --- .../django/admin/includes/portfolio_admins_table.html | 10 +++++++++- .../django/admin/includes/portfolio_members_table.html | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/portfolio_admins_table.html b/src/registrar/templates/django/admin/includes/portfolio_admins_table.html index 78bc53a38..84137cb3d 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_admins_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio_admins_table.html @@ -18,9 +18,16 @@ {{ admin.user.get_formatted_name}} {{ admin.user.title }} - {{ admin.user.email }} + + {% if admin.user.email %} + {{ admin.user.email }} + {% else %} + None + {% endif %} + {{ admin.user.phone }} + {% if admin.user.email %} + {% endif %} {% endfor %} diff --git a/src/registrar/templates/django/admin/includes/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio_members_table.html index 44b954077..3df12f06f 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_members_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio_members_table.html @@ -20,7 +20,13 @@ {{ member.user.get_formatted_name}} {{ member.user.title }} - {{ member.user.email }} + + {% if member.user.email %} + {{ member.user.email }} + {% else %} + None + {% endif %} + {{ member.user.phone }} {% for role in member.user|portfolio_role_summary:original %} @@ -28,6 +34,7 @@ {% endfor %} + {% if member.user.email %} + {% endif %} {% endfor %} From 7b2b4f0401c247ad67d48b048aba2536a0be2c5e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 25 Sep 2024 12:14:47 -0600 Subject: [PATCH 15/42] make userportfoliopermission editable again also removed help text --- src/registrar/admin.py | 1 - ...alter_portfolio_federal_agency_and_more.py | 48 +++++++++++++++++++ .../0130_alter_portfolio_organization_name.py | 18 ------- src/registrar/models/portfolio.py | 2 - src/registrar/models/user_group.py | 2 +- 5 files changed, 49 insertions(+), 22 deletions(-) create mode 100644 src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py delete mode 100644 src/registrar/migrations/0130_alter_portfolio_organization_name.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e75ffb65f..e5de71964 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1243,7 +1243,6 @@ class UserDomainRoleResource(resources.ModelResource): model = models.UserDomainRole -# Note: This is "viewonly" for analysts class UserPortfolioPermissionAdmin(ListHeaderAdmin): form = UserPortfolioPermissionsForm change_form_template = "django/admin/user_portfolio_permission_change_form.html" diff --git a/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py b/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py new file mode 100644 index 000000000..a0f1e931f --- /dev/null +++ b/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py @@ -0,0 +1,48 @@ +# Generated by Django 4.2.10 on 2024-09-25 17:59 + +from django.db import migrations, models +import django.db.models.deletion +import registrar.models.federal_agency + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0129_create_groups_v17"), + ] + + operations = [ + migrations.AlterField( + model_name="portfolio", + name="federal_agency", + field=models.ForeignKey( + default=registrar.models.federal_agency.FederalAgency.get_non_federal_agency, + on_delete=django.db.models.deletion.PROTECT, + to="registrar.federalagency", + ), + ), + migrations.AlterField( + model_name="portfolio", + name="organization_name", + field=models.CharField(blank=True, null=True), + ), + migrations.AlterField( + model_name="portfolio", + name="organization_type", + field=models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + max_length=255, + null=True, + ), + ), + ] diff --git a/src/registrar/migrations/0130_alter_portfolio_organization_name.py b/src/registrar/migrations/0130_alter_portfolio_organization_name.py deleted file mode 100644 index f8feaf672..000000000 --- a/src/registrar/migrations/0130_alter_portfolio_organization_name.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.2.10 on 2024-09-25 14:28 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0129_create_groups_v17"), - ] - - operations = [ - migrations.AlterField( - model_name="portfolio", - name="organization_name", - field=models.CharField(blank=True, null=True), - ), - ] diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 7e1bc7616..398c9a05c 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -41,7 +41,6 @@ class Portfolio(TimeStampedModel): choices=OrganizationChoices.choices, null=True, blank=True, - help_text="Type of organization", ) notes = models.TextField( @@ -52,7 +51,6 @@ class Portfolio(TimeStampedModel): federal_agency = models.ForeignKey( "registrar.FederalAgency", on_delete=models.PROTECT, - help_text="Associated federal agency", unique=False, default=FederalAgency.get_non_federal_agency, ) diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index 5c9bea8b4..e6bcaa4f4 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -79,7 +79,7 @@ class UserGroup(Group): { "app_label": "registrar", "model": "userportfoliopermission", - "permissions": ["view_userportfoliopermission"], + "permissions": ["add_userportfoliopermission", "change_userportfoliopermission", "delete_userportfoliopermission"], }, ] From 2430d3ecf6a9b11bce3345440857b08a19f22ff9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 25 Sep 2024 15:29:40 -0600 Subject: [PATCH 16/42] add detail style and hide fed agency --- src/registrar/assets/js/get-gov-admin.js | 14 +++++++------- src/registrar/assets/sass/_theme/_admin.scss | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 117be405d..9b5fc8bd6 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -883,22 +883,22 @@ function initializeWidgetOnList(list, parentId) { // Handle hiding the organization name field when the organization_type is federal. // Run this first one page load, then secondly on a change event. let organizationNameContainer = document.querySelector(".field-organization_name"); - if (!organizationNameContainer) { - organizationNameContainer = document.querySelector("#id_organization_name"); - } - handleOrganizationTypeChange(organizationType, organizationNameContainer); + let federalType = document.querySelector(".field-federal_type"); + handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); organizationType.addEventListener("change", function() { - handleOrganizationTypeChange(organizationType, organizationNameContainer); + handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); }); }); - function handleOrganizationTypeChange(organizationType, organizationNameContainer) { - if (organizationType && organizationNameContainer) { + function handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType) { + if (organizationType && organizationNameContainer && federalType) { let selectedValue = organizationType.value; if (selectedValue === "federal") { hideElement(organizationNameContainer); + showElement(federalType); } else { showElement(organizationNameContainer); + hideElement(federalType); } } } diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index b83882340..3859b3270 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -453,7 +453,8 @@ details.dja-detail-table { background-color: var(--body-bg); .dja-details-summary { cursor: pointer; - color: var(--body-quiet-color); + color: var(--link-fg); + text-decoration: underline; } @media (max-width: 1024px){ From 02bf9c4781965d4c1e475e5c523554d3d5ffd471 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 09:49:14 -0600 Subject: [PATCH 17/42] Make senior official readonly --- src/registrar/admin.py | 7 +++ src/registrar/assets/js/get-gov-admin.js | 54 +++++++++++-------- ...alter_portfolio_federal_agency_and_more.py | 13 ++++- src/registrar/models/portfolio.py | 1 + 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e5de71964..a83aedf8e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2982,6 +2982,8 @@ class PortfolioAdmin(ListHeaderAdmin): "display_admins", "display_members", "creator", + # As of now this means that only federal agency can update this, but this will change. + "senior_official", ] analyst_readonly_fields = [ @@ -3208,6 +3210,11 @@ class PortfolioAdmin(ListHeaderAdmin): is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL if is_federal and obj.organization_name is None: obj.organization_name = obj.federal_agency.agency + + # Remove this line when senior_official is no longer readonly in /admin. + if obj.federal_agency and obj.federal_agency.so_federal_agency.exists(): + obj.senior_official = obj.federal_agency.so_federal_agency.first() + super().save_model(request, obj, form, change) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 9b5fc8bd6..eb62f4b64 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -937,13 +937,6 @@ function initializeWidgetOnList(list, parentId) { } } - // Get the associated senior official with this federal agency - let $seniorOfficial = django.jQuery("#id_senior_official"); - if (!$seniorOfficial) { - console.log("Could not find the senior official field"); - return; - } - // 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; @@ -965,6 +958,7 @@ function initializeWidgetOnList(list, parentId) { // If we can update the contact information, it'll be shown again. hideElement(contactList.parentElement); + let $seniorOfficial = django.jQuery("#id_senior_official"); let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) .then(response => { @@ -986,30 +980,44 @@ function initializeWidgetOnList(list, parentId) { // Update the "contact details" blurb beneath senior official updateContactInfo(data); showElement(contactList.parentElement); - + + // Get the associated senior official with this federal agency let seniorOfficialId = data.id; let seniorOfficialName = [data.first_name, data.last_name].join(" "); - if (!seniorOfficialId || !seniorOfficialName || !seniorOfficialName.trim()){ - // Clear the field if the SO doesn't exist - $seniorOfficial.val("").trigger("change"); - return; - } - - // Add the senior official to the dropdown. - // This format supports select2 - if we decide to convert this field in the future. - if ($seniorOfficial.find(`option[value='${seniorOfficialId}']`).length) { - // Select the value that is associated with the current Senior Official. - $seniorOfficial.val(seniorOfficialId).trigger("change"); - } else { - // Create a DOM Option that matches the desired Senior Official. Then append it and select it. - let userOption = new Option(seniorOfficialName, seniorOfficialId, true, true); - $seniorOfficial.append(userOption).trigger("change"); + if (!$seniorOfficial) { + // If the senior official is a dropdown field, edit that + updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); + }else { + let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); + if (readonlySeniorOfficial) { + let seniorOfficialLink = `${seniorOfficialName}` + readonlySeniorOfficial.innerHTML = seniorOfficialName ? seniorOfficialLink : "-"; + } } }) .catch(error => console.error("Error fetching senior official: ", error)); } + function updateSeniorOfficialDropdown(dropdown, seniorOfficialId, seniorOfficialName) { + if (!seniorOfficialId || !seniorOfficialName || !seniorOfficialName.trim()){ + // Clear the field if the SO doesn't exist + dropdown.val("").trigger("change"); + return; + } + + // Add the senior official to the dropdown. + // This format supports select2 - if we decide to convert this field in the future. + if (dropdown.find(`option[value='${seniorOfficialId}']`).length) { + // Select the value that is associated with the current Senior Official. + dropdown.val(seniorOfficialId).trigger("change"); + } else { + // Create a DOM Option that matches the desired Senior Official. Then append it and select it. + let userOption = new Option(seniorOfficialName, seniorOfficialId, true, true); + dropdown.append(userOption).trigger("change"); + } + } + function handleStateTerritoryChange(stateTerritory, urbanizationField) { let selectedValue = stateTerritory.value; if (selectedValue === "PR") { diff --git a/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py b/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py index a0f1e931f..f3372b405 100644 --- a/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py +++ b/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-09-25 17:59 +# Generated by Django 4.2.10 on 2024-09-26 15:09 from django.db import migrations, models import django.db.models.deletion @@ -45,4 +45,15 @@ class Migration(migrations.Migration): null=True, ), ), + migrations.AlterField( + model_name="portfolio", + name="senior_official", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="portfolios", + to="registrar.seniorofficial", + ), + ), ] diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 398c9a05c..55ca64da5 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -61,6 +61,7 @@ class Portfolio(TimeStampedModel): unique=False, null=True, blank=True, + related_name="portfolios", ) address_line1 = models.CharField( From 1a9002c1b44bd4fab9009e4be9e598dda85aeb08 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:36:58 -0600 Subject: [PATCH 18/42] Add tables for domains / requests and readonly senior official logic --- src/registrar/admin.py | 6 +++- src/registrar/assets/js/get-gov-admin.js | 11 +++++-- .../admin/includes/contact_detail_list.html | 2 +- .../portfolio_domain_requests_table.html | 28 ++++++++++++++++ .../includes/portfolio_domains_table.html | 32 +++++++++++++++++++ .../admin/includes/portfolio_fieldset.html | 19 ++++++++++- 6 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html create mode 100644 src/registrar/templates/django/admin/includes/portfolio_domains_table.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index a83aedf8e..2c1bd5a5a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2938,7 +2938,7 @@ class PortfolioAdmin(ListHeaderAdmin): }, ), ("Portfolio members", {"fields": ["display_admins", "display_members"]}), - ("Portfolio domains", {"fields": ["domains", "domain_requests"]}), + ("Domains and requests", {"fields": ["domains", "domain_requests"]}), ("Suborganizations", {"fields": ["suborganizations"]}), ("Senior official", {"fields": ["senior_official"]}), ] @@ -3192,8 +3192,12 @@ class PortfolioAdmin(ListHeaderAdmin): obj = self.get_object(request, object_id) extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True + + # We repeat these calls twice. extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) + extra_context["domains"] = obj.get_domains() + extra_context["domain_requests"] = obj.get_domain_requests() return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index eb62f4b64..fd2d6330e 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -959,6 +959,7 @@ function initializeWidgetOnList(list, parentId) { hideElement(contactList.parentElement); let $seniorOfficial = django.jQuery("#id_senior_official"); + let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) .then(response => { @@ -969,7 +970,12 @@ function initializeWidgetOnList(list, parentId) { if (data.error) { // Clear the field if the SO doesn't exist. if (statusCode === 404) { - $seniorOfficial.val("").trigger("change"); + if ($seniorOfficial && $seniorOfficial.length > 0) { + $seniorOfficial.val("").trigger("change"); + }else { + // Show the "create one now" text if this field is none in readonly mode. + readonlySeniorOfficial.innerHTML = 'No senior official has been found. Create one now.' + } console.warn("Record not found: " + data.error); }else { console.error("Error in AJAX call: " + data.error); @@ -984,11 +990,10 @@ function initializeWidgetOnList(list, parentId) { // Get the associated senior official with this federal agency let seniorOfficialId = data.id; let seniorOfficialName = [data.first_name, data.last_name].join(" "); - if (!$seniorOfficial) { + if ($seniorOfficial && $seniorOfficial.length > 0) { // If the senior official is a dropdown field, edit that updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); }else { - let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); if (readonlySeniorOfficial) { let seniorOfficialLink = `${seniorOfficialName}` readonlySeniorOfficial.innerHTML = seniorOfficialName ? seniorOfficialLink : "-"; diff --git a/src/registrar/templates/django/admin/includes/contact_detail_list.html b/src/registrar/templates/django/admin/includes/contact_detail_list.html index 7cc72e8e1..0a28a6532 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_list.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html @@ -39,7 +39,7 @@ None
{% endif %} - {% else %} + {% elif not hide_no_contact_info_message %} No additional contact information found.
{% endif %} diff --git a/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html new file mode 100644 index 000000000..2887c2179 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html @@ -0,0 +1,28 @@ +{% load static url_helpers %} + +
+ Details +
+ + + + + + + + + {% for domain_request in domain_requests %} + {% url 'admin:registrar_domainrequest_change' domain_request.pk as url %} + + + {% if domain_request.get_status_display %} + + {% else %} + + {% endif %} + + {% endfor %} + +
NameStatus
{{ domain_request }}{{ domain_request.get_status_display }}None
+
+
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio_domains_table.html new file mode 100644 index 000000000..8d958c8e8 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio_domains_table.html @@ -0,0 +1,32 @@ +{% load static url_helpers %} + +
+ Details +
+ + + + + + + + + {% for domain_info in domains %} + {% if domain_info.domain %} + {% with domain=domain_info.domain %} + {% url 'admin:registrar_domain_change' domain.pk as url %} + + + {% if domain and domain.get_state_display %} + + {% else %} + + {% endif %} + + {% endwith %} + {% endif %} + {% endfor %} + +
NameState
{{ domain }}{{ domain.get_state_display }}None
+
+
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html index 2299a579a..c63964d80 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio_fieldset.html @@ -21,6 +21,15 @@

No additional permissions found.

{% endif %} + {% elif field.field.name == "senior_official" %} + {% if original_object.senior_official %} +
{{ field.contents }}
+ {% else %} + {% url "admin:registrar_seniorofficial_add" as url %} +
+ No senior official has been found. Create one now. +
+ {% endif %} {% else %}
{{ field.contents }}
{% endif %} @@ -30,7 +39,7 @@ {% if field.field.name == "senior_official" %}
- {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly hide_no_contact_info_message=True %}
{% elif field.field.name == "display_admins" %} {% if admins|length > 0 %} @@ -40,5 +49,13 @@ {% if members|length > 0 %} {% include "django/admin/includes/portfolio_members_table.html" with members=members %} {% endif %} + {% elif field.field.name == "domains" %} + {% if domains|length > 0 %} + {% include "django/admin/includes/portfolio_domains_table.html" with domains=domains %} + {% endif %} + {% elif field.field.name == "domain_requests" %} + {% if domain_requests|length > 0 %} + {% include "django/admin/includes/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} + {% endif %} {% endif %} {% endblock after_help_text %} \ No newline at end of file From 75b34dab3b7ee1e333384dcb274772dda44d425d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:55:20 -0600 Subject: [PATCH 19/42] Cleanup --- src/registrar/admin.py | 9 --- .../models/user_portfolio_permission.py | 14 ----- src/registrar/registrar_middleware.py | 2 + .../django/admin/includes/details_button.html | 9 +++ .../portfolio/portfolio_admins_table.html | 48 ++++++++++++++++ .../portfolio_domain_requests_table.html | 26 +++++++++ .../portfolio/portfolio_domains_table.html | 30 ++++++++++ .../{ => portfolio}/portfolio_fieldset.html | 8 +-- .../portfolio/portfolio_members_table.html | 55 ++++++++++++++++++ .../includes/portfolio_admins_table.html | 50 ---------------- .../portfolio_domain_requests_table.html | 28 --------- .../includes/portfolio_domains_table.html | 32 ----------- .../includes/portfolio_members_table.html | 57 ------------------- .../user_portfolio_permission_fieldset.html | 26 --------- .../django/admin/portfolio_change_form.html | 2 +- 15 files changed, 175 insertions(+), 221 deletions(-) create mode 100644 src/registrar/templates/django/admin/includes/details_button.html create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html rename src/registrar/templates/django/admin/includes/{ => portfolio}/portfolio_fieldset.html (82%) create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_admins_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_domains_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_members_table.html delete mode 100644 src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2c1bd5a5a..f2e9a65e8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1245,7 +1245,6 @@ class UserDomainRoleResource(resources.ModelResource): class UserPortfolioPermissionAdmin(ListHeaderAdmin): form = UserPortfolioPermissionsForm - change_form_template = "django/admin/user_portfolio_permission_change_form.html" class Meta: """Contains meta information about this class""" @@ -1263,14 +1262,6 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): autocomplete_fields = ["user", "portfolio"] - def change_view(self, request, object_id, form_url="", extra_context=None): - """Adds a readonly display for roles and permissions""" - obj = self.get_object(request, object_id) - extra_context = extra_context or {} - extra_context["display_roles"] = ", ".join(obj.get_readable_roles()) - extra_context["display_permissions"] = ", ".join(obj.get_readable_additional_permissions()) - return super().change_view(request, object_id, form_url, extra_context) - class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 112d93009..5479b6f3d 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -82,20 +82,6 @@ class UserPortfolioPermission(TimeStampedModel): ) return f"{self.user}" f" " if self.roles else "" - def get_readable_roles(self): - """Returns a list of labels of each role in self.roles""" - role_labels = [] - for role in self.roles: - role_labels.append(UserPortfolioRoleChoices.get_user_portfolio_role_label(role)) - return role_labels - - def get_readable_additional_permissions(self): - """Returns a list of labels of each additional_permission in self.additional_permissions""" - perm_labels = [] - for perm in self.additional_permissions: - perm_labels.append(UserPortfolioPermissionChoices.get_user_portfolio_permission_label(perm)) - return perm_labels - def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 6346ed4fd..2ccea9321 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -49,6 +49,8 @@ class CheckUserProfileMiddleware: self.setup_page, self.logout_page, "/admin", + # These are here as there is a bug with this middleware that breaks djangos built in debug console. + # The debug console uses this directory, but since this overrides that, it throws errors. "/__debug__", ] self.other_excluded_pages = [ diff --git a/src/registrar/templates/django/admin/includes/details_button.html b/src/registrar/templates/django/admin/includes/details_button.html new file mode 100644 index 000000000..9ae039b04 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/details_button.html @@ -0,0 +1,9 @@ + +{% comment %} This view provides a detail button that can be used to show/hide content {% endcomment %} +
+ Details +
+ {% block detail_content %} + {% endblock detail_content%} +
+
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html new file mode 100644 index 000000000..4ea9225da --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html @@ -0,0 +1,48 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + + + {% for admin in admins %} + {% url 'admin:registrar_userportfoliopermission_change' admin.pk as url %} + + + + + + + + {% endfor %} + +
NameTitleEmailPhone
{{ admin.user.get_formatted_name}}{{ admin.user.title }} + {% if admin.user.email %} + {{ admin.user.email }} + {% else %} + None + {% endif %} + {{ admin.user.phone }} + {% if admin.user.email %} + + + {% endif %} +
+{% endblock detail_content %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html new file mode 100644 index 000000000..5086721f7 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html @@ -0,0 +1,26 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + {% for domain_request in domain_requests %} + {% url 'admin:registrar_domainrequest_change' domain_request.pk as url %} + + + {% if domain_request.get_status_display %} + + {% else %} + + {% endif %} + + {% endfor %} + +
NameStatus
{{ domain_request }}{{ domain_request.get_status_display }}None
+{% endblock detail_content %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html new file mode 100644 index 000000000..8b2aa018c --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html @@ -0,0 +1,30 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + {% for domain_info in domains %} + {% if domain_info.domain %} + {% with domain=domain_info.domain %} + {% url 'admin:registrar_domain_change' domain.pk as url %} + + + {% if domain and domain.get_state_display %} + + {% else %} + + {% endif %} + + {% endwith %} + {% endif %} + {% endfor %} + +
NameState
{{ domain }}{{ domain.get_state_display }}None
+{% endblock detail_content%} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html similarity index 82% rename from src/registrar/templates/django/admin/includes/portfolio_fieldset.html rename to src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html index c63964d80..26c84800f 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html @@ -43,19 +43,19 @@ {% elif field.field.name == "display_admins" %} {% if admins|length > 0 %} - {% include "django/admin/includes/portfolio_admins_table.html" with admins=admins %} + {% include "django/admin/includes/portfolio/portfolio_admins_table.html" with admins=admins %} {% endif %} {% elif field.field.name == "display_members" %} {% if members|length > 0 %} - {% include "django/admin/includes/portfolio_members_table.html" with members=members %} + {% include "django/admin/includes/portfolio/portfolio_members_table.html" with members=members %} {% endif %} {% elif field.field.name == "domains" %} {% if domains|length > 0 %} - {% include "django/admin/includes/portfolio_domains_table.html" with domains=domains %} + {% include "django/admin/includes/portfolio/portfolio_domains_table.html" with domains=domains %} {% endif %} {% elif field.field.name == "domain_requests" %} {% if domain_requests|length > 0 %} - {% include "django/admin/includes/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} + {% include "django/admin/includes/portfolio/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} {% endif %} {% endif %} {% endblock after_help_text %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html new file mode 100644 index 000000000..2f3389032 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html @@ -0,0 +1,55 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load custom_filters %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + + + + {% for member in members %} + {% url 'admin:registrar_userportfoliopermission_change' member.pk as url %} + + + + + + + + + {% endfor %} + +
NameTitleEmailPhoneRoles
{{ member.user.get_formatted_name}}{{ member.user.title }} + {% if member.user.email %} + {{ member.user.email }} + {% else %} + None + {% endif %} + {{ member.user.phone }} + {% for role in member.user|portfolio_role_summary:original %} + {{ role }} + {% endfor %} + + {% if member.user.email %} + + + {% endif %} +
+{% endblock %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_admins_table.html b/src/registrar/templates/django/admin/includes/portfolio_admins_table.html deleted file mode 100644 index 84137cb3d..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_admins_table.html +++ /dev/null @@ -1,50 +0,0 @@ -{% load static url_helpers %} - -
- Details -
- - - - - - - - - - - {% for admin in admins %} - {% url 'admin:registrar_userportfoliopermission_change' admin.pk as url %} - - - - - - - - {% endfor %} - -
NameTitleEmailPhone
{{ admin.user.get_formatted_name}}{{ admin.user.title }} - {% if admin.user.email %} - {{ admin.user.email }} - {% else %} - None - {% endif %} - {{ admin.user.phone }} - {% if admin.user.email %} - - - {% endif %} -
-
-
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html deleted file mode 100644 index 2887c2179..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html +++ /dev/null @@ -1,28 +0,0 @@ -{% load static url_helpers %} - -
- Details -
- - - - - - - - - {% for domain_request in domain_requests %} - {% url 'admin:registrar_domainrequest_change' domain_request.pk as url %} - - - {% if domain_request.get_status_display %} - - {% else %} - - {% endif %} - - {% endfor %} - -
NameStatus
{{ domain_request }}{{ domain_request.get_status_display }}None
-
-
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio_domains_table.html deleted file mode 100644 index 8d958c8e8..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_domains_table.html +++ /dev/null @@ -1,32 +0,0 @@ -{% load static url_helpers %} - -
- Details -
- - - - - - - - - {% for domain_info in domains %} - {% if domain_info.domain %} - {% with domain=domain_info.domain %} - {% url 'admin:registrar_domain_change' domain.pk as url %} - - - {% if domain and domain.get_state_display %} - - {% else %} - - {% endif %} - - {% endwith %} - {% endif %} - {% endfor %} - -
NameState
{{ domain }}{{ domain.get_state_display }}None
-
-
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio_members_table.html deleted file mode 100644 index 3df12f06f..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_members_table.html +++ /dev/null @@ -1,57 +0,0 @@ -{% load custom_filters %} -{% load static url_helpers %} - -
- Details -
- - - - - - - - - - - - {% for member in members %} - {% url 'admin:registrar_userportfoliopermission_change' member.pk as url %} - - - - - - - - - {% endfor %} - -
NameTitleEmailPhoneRoles
{{ member.user.get_formatted_name}}{{ member.user.title }} - {% if member.user.email %} - {{ member.user.email }} - {% else %} - None - {% endif %} - {{ member.user.phone }} - {% for role in member.user|portfolio_role_summary:original %} - {{ role }} - {% endfor %} - - {% if member.user.email %} - - - {% endif %} -
-
-
\ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html b/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html deleted file mode 100644 index a7ffc889c..000000000 --- a/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html +++ /dev/null @@ -1,26 +0,0 @@ -{% extends "django/admin/includes/detail_table_fieldset.html" %} -{% load custom_filters %} -{% load static url_helpers %} - - -{% block field_readonly %} - {% if field.field.name == "roles" %} -
- {% if display_roles %} - {{ display_roles }} - {% else %} - No roles found. - {% endif %} -
- {% elif field.field.name == "additional_permissions" %} -
- {% if display_permissions %} - {{ display_permissions }} - {% else %} - No additional permissions found. - {% endif %} -
- {% else %} -
{{ field.contents }}
- {% endif %} -{% endblock field_readonly%} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 38b155ce2..0a87f8e49 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -20,7 +20,7 @@ When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. {% endcomment %} - {% include "django/admin/includes/portfolio_fieldset.html" with original_object=original %} + {% include "django/admin/includes/portfolio/portfolio_fieldset.html" with original_object=original %} {% endfor %} {% endblock %} From 237d06623329129caf87789683d4ddc2620175e5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:05:58 -0600 Subject: [PATCH 20/42] Fix some tests + fix add view --- src/registrar/assets/js/get-gov-admin.js | 10 +++++++--- src/registrar/tests/test_admin.py | 13 ++----------- src/registrar/tests/test_api.py | 1 - 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index fd2d6330e..7271cf7b8 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -891,14 +891,18 @@ function initializeWidgetOnList(list, parentId) { }); function handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType) { - if (organizationType && organizationNameContainer && federalType) { + if (organizationType && organizationNameContainer) { let selectedValue = organizationType.value; if (selectedValue === "federal") { hideElement(organizationNameContainer); - showElement(federalType); + if (federalType) { + showElement(federalType); + } } else { showElement(organizationNameContainer); - hideElement(federalType); + if (federalType) { + hideElement(federalType); + } } } } diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index daf5e7086..e125baf60 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2097,17 +2097,8 @@ class TestPortfolioAdmin(TestCase): ) display_admins = self.admin.display_admins(self.portfolio) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_1.pk]) - self.assertIn( - f'Gerald Meoward meaoward@gov.gov', - display_admins, - ) - self.assertIn("Captain", display_admins) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_2.pk]) - self.assertIn(f'Arnold Poopy poopy@gov.gov', display_admins) - self.assertIn("Major", display_admins) + url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={self.portfolio.id}" + self.assertIn(f'2 administrators', display_admins) display_members_summary = self.admin.display_members_summary(self.portfolio) diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index 2597e65c2..7a432bcb8 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -100,7 +100,6 @@ class GetFederalPortfolioTypeJsonTest(TestCase): self.assertEqual(response.status_code, 200) 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): From e1ed5a5ed5d59083d8f20e6240f39c7a25fa856c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:36:33 -0600 Subject: [PATCH 21/42] linting + fix tests --- src/registrar/admin.py | 53 ++++++++++--------- src/registrar/models/portfolio.py | 1 - src/registrar/models/user_group.py | 6 ++- .../django/admin/includes/details_button.html | 2 +- .../portfolio_domain_requests_table.html | 2 +- .../portfolio/portfolio_domains_table.html | 2 +- .../portfolio/portfolio_fieldset.html | 2 +- .../portfolio/portfolio_members_table.html | 2 +- .../django/admin/portfolio_change_form.html | 7 --- src/registrar/templatetags/custom_filters.py | 2 +- src/registrar/tests/test_admin.py | 30 ++--------- src/registrar/tests/test_migrations.py | 5 +- 12 files changed, 47 insertions(+), 67 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f2e9a65e8..49cf08935 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -5,7 +5,6 @@ import json from django.template.loader import get_template from django import forms from django.db.models import Value, CharField, Q -from django.template.loader import render_to_string from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect from django.conf import settings @@ -2981,28 +2980,6 @@ class PortfolioAdmin(ListHeaderAdmin): "organization_name", ] - def get_readonly_fields(self, request, obj=None): - """Set the read-only state on form elements. - We have 2 conditions that determine which fields are read-only: - admin user permissions and the creator's status, so - we'll use the baseline readonly_fields and extend it as needed. - """ - readonly_fields = list(self.readonly_fields) - - # Check if the creator is restricted - if obj and obj.creator.status == models.User.RESTRICTED: - # For fields like CharField, IntegerField, etc., the widget used is - # straightforward and the readonly_fields list can control their behavior - readonly_fields.extend([field.name for field in self.model._meta.fields]) - - if request.user.has_perm("registrar.full_access_permission"): - return readonly_fields - - # Return restrictive Read-only fields for analysts and - # users who might not belong to groups - readonly_fields.extend([field for field in self.analyst_readonly_fields]) - return readonly_fields - def get_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio admin_permissions = self.get_user_portfolio_permission_admins(obj) @@ -3015,7 +2992,9 @@ class PortfolioAdmin(ListHeaderAdmin): def get_user_portfolio_permission_admins(self, obj): """Returns each admin on UserPortfolioPermission for a given portfolio.""" if obj: - return obj.portfolio_users.filter(portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) + return obj.portfolio_users.filter( + portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) else: return [] @@ -3177,6 +3156,28 @@ class PortfolioAdmin(ListHeaderAdmin): return self.add_fieldsets return super().get_fieldsets(request, obj) + def get_readonly_fields(self, request, obj=None): + """Set the read-only state on form elements. + We have 2 conditions that determine which fields are read-only: + admin user permissions and the creator's status, so + we'll use the baseline readonly_fields and extend it as needed. + """ + readonly_fields = list(self.readonly_fields) + + # Check if the creator is restricted + if obj and obj.creator.status == models.User.RESTRICTED: + # For fields like CharField, IntegerField, etc., the widget used is + # straightforward and the readonly_fields list can control their behavior + readonly_fields.extend([field.name for field in self.model._meta.fields]) + + if request.user.has_perm("registrar.full_access_permission"): + return readonly_fields + + # Return restrictive Read-only fields for analysts and + # users who might not belong to groups + readonly_fields.extend([field for field in self.analyst_readonly_fields]) + return readonly_fields + def change_view(self, request, object_id, form_url="", extra_context=None): """Add related suborganizations and domain groups. Add the summary for the portfolio members field (list of members that link to change_forms).""" @@ -3184,7 +3185,7 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True - # We repeat these calls twice. + # We repeat these calls twice. extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) extra_context["domains"] = obj.get_domains() @@ -3205,7 +3206,7 @@ class PortfolioAdmin(ListHeaderAdmin): is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL if is_federal and obj.organization_name is None: obj.organization_name = obj.federal_agency.agency - + # Remove this line when senior_official is no longer readonly in /admin. if obj.federal_agency and obj.federal_agency.so_federal_agency.exists(): obj.senior_official = obj.federal_agency.so_federal_agency.first() diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 55ca64da5..9acec8c64 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -2,7 +2,6 @@ from django.db import models from registrar.models.domain_request import DomainRequest from registrar.models.federal_agency import FederalAgency -from registrar.utility.constants import BranchChoices from .utility.time_stamped_model import TimeStampedModel diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index e6bcaa4f4..41ae67c06 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -79,7 +79,11 @@ class UserGroup(Group): { "app_label": "registrar", "model": "userportfoliopermission", - "permissions": ["add_userportfoliopermission", "change_userportfoliopermission", "delete_userportfoliopermission"], + "permissions": [ + "add_userportfoliopermission", + "change_userportfoliopermission", + "delete_userportfoliopermission", + ], }, ] diff --git a/src/registrar/templates/django/admin/includes/details_button.html b/src/registrar/templates/django/admin/includes/details_button.html index 9ae039b04..73748f170 100644 --- a/src/registrar/templates/django/admin/includes/details_button.html +++ b/src/registrar/templates/django/admin/includes/details_button.html @@ -6,4 +6,4 @@ {% block detail_content %} {% endblock detail_content%} - \ No newline at end of file + diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html index 5086721f7..46303efce 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html @@ -23,4 +23,4 @@ {% endfor %} -{% endblock detail_content %} \ No newline at end of file +{% endblock detail_content %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html index 8b2aa018c..56621b769 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html @@ -27,4 +27,4 @@ {% endfor %} -{% endblock detail_content%} \ No newline at end of file +{% endblock detail_content%} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html index 26c84800f..c5c6f510a 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html @@ -58,4 +58,4 @@ {% include "django/admin/includes/portfolio/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} {% endif %} {% endif %} -{% endblock after_help_text %} \ No newline at end of file +{% endblock after_help_text %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html index 2f3389032..136fe3a5a 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html @@ -52,4 +52,4 @@ {% endfor %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 0a87f8e49..fec1538d9 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -13,13 +13,6 @@ {% block field_sets %} {% for fieldset in adminform %} - {% comment %} - This is a placeholder for now. - - Disclaimer: - When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. - detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. - {% endcomment %} {% include "django/admin/includes/portfolio/portfolio_fieldset.html" with original_object=original %} {% endfor %} {% endblock %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index de9b7bfa1..0bde40bb9 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -247,4 +247,4 @@ def portfolio_role_summary(user, portfolio): if user and portfolio: return user.portfolio_role_summary(portfolio) else: - return [] \ No newline at end of file + return [] diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e125baf60..cdc3c97de 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2051,7 +2051,7 @@ class TestPortfolioAdmin(TestCase): email="meaoward@gov.gov", ) - perm_1 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) @@ -2063,7 +2063,7 @@ class TestPortfolioAdmin(TestCase): email="poopy@gov.gov", ) - perm_2 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) @@ -2075,7 +2075,7 @@ class TestPortfolioAdmin(TestCase): email="madmax@gov.gov", ) - perm_3 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] ) @@ -2087,7 +2087,7 @@ class TestPortfolioAdmin(TestCase): email="thematrix@gov.gov", ) - perm_4 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_4, portfolio=self.portfolio, additional_permissions=[ @@ -2100,28 +2100,8 @@ class TestPortfolioAdmin(TestCase): url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={self.portfolio.id}" self.assertIn(f'2 administrators', display_admins) - display_members_summary = self.admin.display_members_summary(self.portfolio) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_3.pk]) - self.assertIn( - f'Mad Max madmax@gov.gov', - display_members_summary, - ) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_4.pk]) - self.assertIn( - f'Agent Smith thematrix@gov.gov', - display_members_summary, - ) - display_members = self.admin.display_members(self.portfolio) - - self.assertIn("Mad Max", display_members) - self.assertIn("Member", display_members) - self.assertIn("Road warrior", display_members) - self.assertIn("Agent Smith", display_members) - self.assertIn("Domain requestor", display_members) - self.assertIn("Program", display_members) + self.assertIn(f'2 members', display_members) class TestTransferUser(WebTest): diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index 5dde4831c..d646a03de 100644 --- a/src/registrar/tests/test_migrations.py +++ b/src/registrar/tests/test_migrations.py @@ -50,7 +50,9 @@ class TestGroups(TestCase): "change_user", "delete_userdomainrole", "view_userdomainrole", - "view_userportfoliopermission", + "add_userportfoliopermission", + "change_userportfoliopermission", + "delete_userportfoliopermission", "add_verifiedbystaff", "change_verifiedbystaff", "delete_verifiedbystaff", @@ -58,6 +60,7 @@ class TestGroups(TestCase): # Get the codenames of actual permissions associated with the group actual_permissions = [p.codename for p in cisa_analysts_group.permissions.all()] + self.maxDiff = None # Assert that the actual permissions match the expected permissions self.assertListEqual(actual_permissions, expected_permissions) From 0612e1bcc5613bbd1957532b7a5c13a5ffe83dc3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:40:45 -0600 Subject: [PATCH 22/42] Update src/registrar/admin.py --- src/registrar/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 49cf08935..c5dfecde3 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3185,7 +3185,6 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True - # We repeat these calls twice. extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) extra_context["domains"] = obj.get_domains() From 73264cce96a90fd4a7687149af558168e9a0e565 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:43:40 -0600 Subject: [PATCH 23/42] Remove unused portfolio_type (thanks, Linter!) --- src/registrar/admin.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 49cf08935..920411737 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2968,7 +2968,6 @@ class PortfolioAdmin(ListHeaderAdmin): "domains", "domain_requests", "suborganizations", - "portfolio_type", "display_admins", "display_members", "creator", @@ -3029,12 +3028,6 @@ class PortfolioAdmin(ListHeaderAdmin): created_on.short_description = "Created on" # type: ignore - def portfolio_type(self, obj: models.Portfolio): - """Returns the portfolio type, or "-" if the result is empty""" - return obj.portfolio_type if obj.portfolio_type else "-" - - portfolio_type.short_description = "Portfolio type" # type: ignore - def suborganizations(self, obj: models.Portfolio): """Returns a list of links for each related suborg""" queryset = obj.get_suborganizations() From 6aa7546c2ee306e34d7d00e3ade59756f0099bb1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:12:20 -0600 Subject: [PATCH 24/42] Fix SO message and fix search text --- src/registrar/admin.py | 2 +- src/registrar/assets/js/get-gov-admin.js | 2 +- .../django/admin/includes/portfolio/portfolio_fieldset.html | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7bf92125f..ea84f3191 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2957,7 +2957,7 @@ class PortfolioAdmin(ListHeaderAdmin): list_display = ("organization_name", "organization_type", "federal_type", "creator") search_fields = ["organization_name"] - search_help_text = "Search by portfolio organization." + search_help_text = "Search by organization name." readonly_fields = [ # This is the created_at field "created_on", diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 7271cf7b8..d98d11437 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -978,7 +978,7 @@ function initializeWidgetOnList(list, parentId) { $seniorOfficial.val("").trigger("change"); }else { // Show the "create one now" text if this field is none in readonly mode. - readonlySeniorOfficial.innerHTML = 'No senior official has been found. Create one now.' + readonlySeniorOfficial.innerHTML = 'No senior official found. Create one now.' } console.warn("Record not found: " + data.error); }else { diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html index c5c6f510a..87b56cb60 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html @@ -27,7 +27,7 @@ {% else %} {% url "admin:registrar_seniorofficial_add" as url %}
- No senior official has been found. Create one now. + No senior official found. Create one now.
{% endif %} {% else %} From d80287a428cee71b9283da408156144dedc9ff68 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:37:31 -0600 Subject: [PATCH 25/42] User portfolio suggestions --- src/registrar/admin.py | 18 +++++++++++++----- src/registrar/models/portfolio.py | 15 +++++++++++---- .../models/user_portfolio_permission.py | 14 ++++++++++---- src/registrar/tests/test_models.py | 5 ++++- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ea84f3191..ffea1fb24 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -10,8 +10,7 @@ from django.http import HttpResponseRedirect from django.conf import settings from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField -from registrar.models.domain_information import DomainInformation -from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models import DomainInformation, Portfolio, UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from waffle.decorators import flag_is_active from django.contrib import admin, messages @@ -1257,9 +1256,18 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): list_display = [ "user", "portfolio", + "get_roles", ] autocomplete_fields = ["user", "portfolio"] + search_fields = ["user__first_name", "user__last_name", "user__email", "portfolio__organization_name"] + search_help_text = "Search by first name, last name, email, or portfolio." + + def get_roles(self, obj): + readable_roles = obj.get_readable_roles() + return ", ".join(readable_roles) + + get_roles.short_description = "Roles" class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): @@ -3174,14 +3182,14 @@ class PortfolioAdmin(ListHeaderAdmin): def change_view(self, request, object_id, form_url="", extra_context=None): """Add related suborganizations and domain groups. Add the summary for the portfolio members field (list of members that link to change_forms).""" - obj = self.get_object(request, object_id) + obj: Portfolio = self.get_object(request, object_id) extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) - extra_context["domains"] = obj.get_domains() - extra_context["domain_requests"] = obj.get_domain_requests() + extra_context["domains"] = obj.get_domains(order_by=["domain_information__name"]) + extra_context["domain_requests"] = obj.get_domain_requests(order_by=["domain_information__name"]) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 9acec8c64..e6f7162d6 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -132,13 +132,20 @@ class Portfolio(TimeStampedModel): return federal_agency.federal_type if federal_agency else None # == Getters for domains == # - def get_domains(self): + def get_domains(self, order_by=None): """Returns all DomainInformations associated with this portfolio""" - return self.information_portfolio.all() + if not order_by: + return self.information_portfolio.all() + else: + return self.information_portfolio.all().order_by(*order_by) - def get_domain_requests(self): + def get_domain_requests(self, order_by=None): """Returns all DomainRequests associated with this portfolio""" - return self.DomainRequest_portfolio.all() + if not order_by: + return self.DomainRequest_portfolio.all() + else: + return self.DomainRequest_portfolio.all().order_by(*order_by) + # == Getters for suborganization == # def get_suborganizations(self): diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 5479b6f3d..4cf85d4fc 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -77,11 +77,16 @@ class UserPortfolioPermission(TimeStampedModel): def __str__(self): readable_roles = [] if self.roles: - readable_roles = sorted( - [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles] - ) + readable_roles = self.get_readable_roles() return f"{self.user}" f" " if self.roles else "" + def get_readable_roles(self): + """Returns a readable list of self.roles""" + readable_roles = [] + if self.roles: + readable_roles = sorted([UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles]) + return readable_roles + def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. @@ -108,7 +113,8 @@ class UserPortfolioPermission(TimeStampedModel): existing_permissions = UserPortfolioPermission.objects.filter(user=self.user) if not flag_is_active_for_user(self.user, "multiple_portfolios") and existing_permissions.exists(): raise ValidationError( - "Only one portfolio permission is allowed per user when multiple portfolios are disabled." + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) # Check if portfolio is set without accessing the related object. diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a6cac1389..015c67dab 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1332,7 +1332,10 @@ class TestUserPortfolioPermission(TestCase): self.assertEqual( cm.exception.message, - "Only one portfolio permission is allowed per user when multiple portfolios are disabled.", + ( + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." + ), ) From 2260269e3d5e444c79b37c1fcf369907cc800541 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:39:21 -0600 Subject: [PATCH 26/42] fix order by --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ffea1fb24..de1b96e6c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3188,8 +3188,8 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) - extra_context["domains"] = obj.get_domains(order_by=["domain_information__name"]) - extra_context["domain_requests"] = obj.get_domain_requests(order_by=["domain_information__name"]) + extra_context["domains"] = obj.get_domains(order_by=["domain__name"]) + extra_context["domain_requests"] = obj.get_domain_requests(order_by=["requested_domain__name"]) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): From 81e76fe04a74800826308b68c2f378edc69240f9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:49:19 -0600 Subject: [PATCH 27/42] fix fed agency bug --- src/registrar/assets/js/get-gov-admin.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index d98d11437..c01f5d784 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -860,10 +860,13 @@ function initializeWidgetOnList(list, parentId) { let organizationType = document.getElementById("id_organization_type"); let readonlyOrganizationType = document.querySelector(".field-organization_type .readonly"); + let organizationNameContainer = document.querySelector(".field-organization_name"); + let federalType = document.querySelector(".field-federal_type"); + if ($federalAgency && (organizationType || readonlyOrganizationType)) { // Attach the change event listener $federalAgency.on("change", function() { - handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType); + handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType); }); } @@ -882,8 +885,6 @@ function initializeWidgetOnList(list, parentId) { // Handle hiding the organization name field when the organization_type is federal. // Run this first one page load, then secondly on a change event. - let organizationNameContainer = document.querySelector(".field-organization_name"); - let federalType = document.querySelector(".field-federal_type"); handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); organizationType.addEventListener("change", function() { handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); @@ -907,7 +908,7 @@ function initializeWidgetOnList(list, parentId) { } } - function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType) { + function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType) { // Don't do anything on page load if (isInitialPageLoad) { isInitialPageLoad = false; @@ -941,6 +942,8 @@ function initializeWidgetOnList(list, parentId) { } } + handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); + // 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; From 88b38baaa7e96b37a50406c057587d1c9eea5dbb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:50:08 -0600 Subject: [PATCH 28/42] lint --- src/registrar/models/portfolio.py | 1 - src/registrar/models/user_portfolio_permission.py | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index e6f7162d6..8d820e105 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -145,7 +145,6 @@ class Portfolio(TimeStampedModel): return self.DomainRequest_portfolio.all() else: return self.DomainRequest_portfolio.all().order_by(*order_by) - # == Getters for suborganization == # def get_suborganizations(self): diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 4cf85d4fc..f021fc6bf 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -84,7 +84,9 @@ class UserPortfolioPermission(TimeStampedModel): """Returns a readable list of self.roles""" readable_roles = [] if self.roles: - readable_roles = sorted([UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles]) + readable_roles = sorted( + [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles] + ) return readable_roles def _get_portfolio_permissions(self): From a7b3fc71a5d251b4f24486f095107ef9aa897c75 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:57:04 -0600 Subject: [PATCH 29/42] Update admin.py --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index de1b96e6c..8406ac3a6 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1267,7 +1267,7 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): readable_roles = obj.get_readable_roles() return ", ".join(readable_roles) - get_roles.short_description = "Roles" + get_roles.short_description = "Roles" # type: ignore class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): From a128ebcf6937a652143f5f040239d5401ee4b550 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:46:21 -0600 Subject: [PATCH 30/42] Update settings.py --- src/registrar/config/settings.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 9eb649ad8..30882cd5d 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -476,6 +476,8 @@ class JsonServerFormatter(ServerFormatter): def format(self, record): formatted_record = super().format(record) + if not hasattr(record, "server_time"): + record.server_time = self.formatTime(record, self.datefmt) log_entry = {"server_time": record.server_time, "level": record.levelname, "message": formatted_record} return json.dumps(log_entry) From 14a1bd53ba67b8b65d8879bb0a028b110d4b1bee Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 13:19:30 -0600 Subject: [PATCH 31/42] Fix url bug --- src/registrar/admin.py | 16 ++++++++++------ src/registrar/assets/js/get-gov-admin.js | 3 ++- .../django/admin/portfolio_change_form.html | 2 ++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8406ac3a6..56f5310e0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3186,10 +3186,11 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True - extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) - extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) - extra_context["domains"] = obj.get_domains(order_by=["domain__name"]) - extra_context["domain_requests"] = obj.get_domain_requests(order_by=["requested_domain__name"]) + if obj: + extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) + extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) + extra_context["domains"] = obj.get_domains(order_by=["domain__name"]) + extra_context["domain_requests"] = obj.get_domain_requests(order_by=["requested_domain__name"]) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): @@ -3208,8 +3209,11 @@ class PortfolioAdmin(ListHeaderAdmin): obj.organization_name = obj.federal_agency.agency # Remove this line when senior_official is no longer readonly in /admin. - if obj.federal_agency and obj.federal_agency.so_federal_agency.exists(): - obj.senior_official = obj.federal_agency.so_federal_agency.first() + if obj.federal_agency: + if obj.federal_agency.so_federal_agency.exists(): + obj.senior_official = obj.federal_agency.so_federal_agency.first() + else: + obj.senior_official = None super().save_model(request, obj, form, change) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index c01f5d784..25e35b73b 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -965,6 +965,7 @@ function initializeWidgetOnList(list, parentId) { // If we can update the contact information, it'll be shown again. hideElement(contactList.parentElement); + let seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; let $seniorOfficial = django.jQuery("#id_senior_official"); let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; @@ -981,7 +982,7 @@ function initializeWidgetOnList(list, parentId) { $seniorOfficial.val("").trigger("change"); }else { // Show the "create one now" text if this field is none in readonly mode. - readonlySeniorOfficial.innerHTML = 'No senior official found. Create one now.' + readonlySeniorOfficial.innerHTML = `No senior official found. Create one now.`; } console.warn("Record not found: " + data.error); }else { diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index fec1538d9..8de6cd5eb 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -8,6 +8,8 @@ {% url 'get-federal-and-portfolio-types-from-federal-agency-json' as url %} + {% url "admin:registrar_seniorofficial_add" as url %} + {{ block.super }} {% endblock content %} From 9a7a65d958af3a3278b06ad1e2e01a7a425d2011 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 13:23:31 -0600 Subject: [PATCH 32/42] fix merge conflict --- .../templates/django/admin/includes/detail_table_fieldset.html | 2 +- 1 file changed, 1 insertion(+), 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 f6ada5166..6b755724e 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -119,7 +119,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endfor %} {% endwith %} - {% elif field.field.name == "display_admins" or field.field.name == "domain_managers" or field.field.namd == "invited_domain_managers" %} + {% elif field.field.name == "domain_managers" or field.field.name == "invited_domain_managers" %}
{{ field.contents|safe }}
{% elif field.field.name == "display_members" %}
From 3290137833bc683e3c68607080ddd470fdecc1d9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:20:03 -0600 Subject: [PATCH 33/42] Federal agency changes --- src/registrar/admin.py | 2 +- ...pulate_federal_agency_initials_and_fceb.py | 2 +- ...nitials_federalagency_acronym_and_more.py} | 33 ++++++++++++++++++- src/registrar/models/federal_agency.py | 7 ++-- .../tests/test_management_scripts.py | 10 +++--- 5 files changed, 42 insertions(+), 12 deletions(-) rename src/registrar/migrations/{0130_alter_portfolio_federal_agency_and_more.py => 0130_remove_federalagency_initials_federalagency_acronym_and_more.py} (62%) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2e81d1d4b..c93159c6b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3291,7 +3291,7 @@ class FederalAgencyResource(resources.ModelResource): class FederalAgencyAdmin(ListHeaderAdmin, ImportExportModelAdmin): list_display = ["agency"] search_fields = ["agency"] - search_help_text = "Search by agency name." + search_help_text = "Search by federal agency." ordering = ["agency"] resource_classes = [FederalAgencyResource] diff --git a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py index 506405b78..30ae08b47 100644 --- a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py +++ b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py @@ -42,7 +42,7 @@ class Command(BaseCommand, PopulateScriptTemplate): """For each record, update the initials and is_fceb field if data exists for it""" initials, agency_status = self.federal_agency_dict.get(record.agency) - record.initials = initials + record.acronym = initials if agency_status and isinstance(agency_status, str) and agency_status.strip().upper() == "FCEB": record.is_fceb = True else: diff --git a/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py similarity index 62% rename from src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py rename to src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py index f3372b405..665b2115f 100644 --- a/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py +++ b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-09-26 15:09 +# Generated by Django 4.2.10 on 2024-09-27 20:12 from django.db import migrations, models import django.db.models.deletion @@ -12,6 +12,37 @@ class Migration(migrations.Migration): ] operations = [ + migrations.RemoveField( + model_name="federalagency", + name="initials", + ), + migrations.AddField( + model_name="federalagency", + name="acronym", + field=models.CharField( + blank=True, + help_text="Acronym commonly used to reference the federal agency (Optional)", + max_length=10, + null=True, + ), + ), + migrations.AlterField( + model_name="federalagency", + name="federal_type", + field=models.CharField( + blank=True, + choices=[("executive", "Executive"), ("judicial", "Judicial"), ("legislative", "Legislative")], + max_length=20, + null=True, + ), + ), + migrations.AlterField( + model_name="federalagency", + name="is_fceb", + field=models.BooleanField( + blank=True, help_text="Federal Civilian Executive Branch (FCEB)", null=True, verbose_name="FCEB" + ), + ), migrations.AlterField( model_name="portfolio", name="federal_agency", diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 5cc87b38c..aeeebac8c 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -22,21 +22,20 @@ class FederalAgency(TimeStampedModel): choices=BranchChoices.choices, null=True, blank=True, - help_text="Federal agency type (executive, judicial, legislative, etc.)", ) - initials = models.CharField( + acronym = models.CharField( max_length=10, null=True, blank=True, - help_text="Agency initials", + help_text="Acronym commonly used to reference the federal agency (Optional)", ) is_fceb = models.BooleanField( null=True, blank=True, verbose_name="FCEB", - help_text="Determines if this agency is FCEB", + help_text="Federal Civilian Executive Branch (FCEB)", ) def __str__(self) -> str: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index cbdc2c034..9fcd261f7 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1387,18 +1387,18 @@ class TestPopulateFederalAgencyInitialsAndFceb(TestCase): self.agency4.refresh_from_db() # Check if FederalAgency objects were updated correctly - self.assertEqual(self.agency1.initials, "ABMC") + self.assertEqual(self.agency1.acronym, "ABMC") self.assertTrue(self.agency1.is_fceb) - self.assertEqual(self.agency2.initials, "ACHP") + self.assertEqual(self.agency2.acronym, "ACHP") self.assertTrue(self.agency2.is_fceb) # We expect that this field doesn't have any data, # as none is specified in the CSV - self.assertIsNone(self.agency3.initials) + self.assertIsNone(self.agency3.acronym) self.assertIsNone(self.agency3.is_fceb) - self.assertEqual(self.agency4.initials, "KC") + self.assertEqual(self.agency4.acronym, "KC") self.assertFalse(self.agency4.is_fceb) @less_console_noise_decorator @@ -1411,7 +1411,7 @@ class TestPopulateFederalAgencyInitialsAndFceb(TestCase): # Verify that the missing agency was not updated missing_agency.refresh_from_db() - self.assertIsNone(missing_agency.initials) + self.assertIsNone(missing_agency.acronym) self.assertIsNone(missing_agency.is_fceb) From fbd82d5e6f2d6b5c29daa5fb3a0b83bd0f2d9619 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:22:03 -0600 Subject: [PATCH 34/42] suborg changes + portfolio on user --- src/registrar/admin.py | 70 ++++--------------- ...initials_federalagency_acronym_and_more.py | 7 +- src/registrar/models/suborganization.py | 2 +- .../models/utility/generic_helper.py | 8 +++ .../django/admin/suborg_change_form.html | 40 ++++++----- .../django/admin/user_change_form.html | 20 ------ src/registrar/utility/admin_helpers.py | 51 ++++++++++++++ 7 files changed, 104 insertions(+), 94 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c93159c6b..e3a3b2b2d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -20,7 +20,7 @@ from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email +from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email, get_field_links_as_list from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes @@ -755,9 +755,10 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): }, ), ("Important dates", {"fields": ("last_login", "date_joined")}), + ("Associated portfolios", {"fields": ("portfolios",)}), ) - readonly_fields = ("verification_type",) + readonly_fields = ("verification_type", "portfolios") analyst_fieldsets = ( ( @@ -780,6 +781,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): }, ), ("Important dates", {"fields": ("last_login", "date_joined")}), + ("Associated portfolios", {"fields": ("portfolios",)}), ) # TODO: delete after we merge organization feature @@ -859,6 +861,14 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): ordering = ["first_name", "last_name", "email"] search_help_text = "Search by first name, last name, or email." + def portfolios(self, obj: models.User): + """Returns a list of links for each related suborg""" + portfolio_ids = obj.get_portfolios().values_list("portfolio", flat=True) + queryset = models.Portfolio.objects.filter(id__in=portfolio_ids) + return get_field_links_as_list(queryset, "portfolio") + + portfolios.short_description = "Portfolios" # type: ignore + def get_search_results(self, request, queryset, search_term): """ Override for get_search_results. This affects any upstream model using autocomplete_fields, @@ -3101,7 +3111,7 @@ class PortfolioAdmin(ListHeaderAdmin): def suborganizations(self, obj: models.Portfolio): """Returns a list of links for each related suborg""" queryset = obj.get_suborganizations() - return self.get_field_links_as_list(queryset, "suborganization") + return get_field_links_as_list(queryset, "suborganization") suborganizations.short_description = "Suborganizations" # type: ignore @@ -3159,59 +3169,6 @@ class PortfolioAdmin(ListHeaderAdmin): "senior_official", ] - def get_field_links_as_list( - self, queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None - ): - """ - Generate HTML links for items in a queryset, using a specified attribute for link text. - - Args: - queryset: The queryset of items to generate links for. - model_name: The model name used to construct the admin change URL. - attribute_name: The attribute or method name to use for link text. If None, the item itself is used. - link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. - separator: The separator to use between links in the resulting HTML. - If none, an unordered list is returned. - - Returns: - A formatted HTML string with links to the admin change pages for each item. - """ - links = [] - for item in queryset: - - # This allows you to pass in attribute_name="get_full_name" for instance. - if attribute_name: - item_display_value = self.value_of_attribute(item, attribute_name) - else: - item_display_value = item - - if item_display_value: - change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) - - link = f'{escape(item_display_value)}' - if link_info_attribute: - link += f" ({self.value_of_attribute(item, link_info_attribute)})" - - if separator: - links.append(link) - else: - links.append(f"
  • {link}
    • ") - - # If no separator is specified, just return an unordered list. - if separator: - return format_html(separator.join(links)) if links else "-" - else: - links = "".join(links) - return format_html(f'
      {links}
    ') if links else "-" - - def value_of_attribute(self, obj, attribute_name: str): - """Returns the value of getattr if the attribute isn't callable. - If it is, execute the underlying function and return that result instead.""" - value = getattr(obj, attribute_name) - if callable(value): - value = value() - return value - def get_fieldsets(self, request, obj=None): """Override of the default get_fieldsets definition to add an add_fieldsets view""" # This is the add view if no obj exists @@ -3348,6 +3305,7 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "portfolio", ] search_fields = ["name"] + search_help_text = "Search by suborganization." change_form_template = "django/admin/suborg_change_form.html" diff --git a/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py index 665b2115f..6e5935748 100644 --- a/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py +++ b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-09-27 20:12 +# Generated by Django 4.2.10 on 2024-09-27 20:20 from django.db import migrations, models import django.db.models.deletion @@ -87,4 +87,9 @@ class Migration(migrations.Migration): to="registrar.seniorofficial", ), ), + migrations.AlterField( + model_name="suborganization", + name="name", + field=models.CharField(max_length=1000, unique=True, verbose_name="Suborganization"), + ), ] diff --git a/src/registrar/models/suborganization.py b/src/registrar/models/suborganization.py index feeee0669..6ad80fdc0 100644 --- a/src/registrar/models/suborganization.py +++ b/src/registrar/models/suborganization.py @@ -10,7 +10,7 @@ class Suborganization(TimeStampedModel): name = models.CharField( unique=True, max_length=1000, - help_text="Suborganization", + verbose_name="Suborganization", ) portfolio = models.ForeignKey( diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 3cafe87c4..b02068de0 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -334,3 +334,11 @@ def get_url_name(path): except Resolver404: logger.error(f"No matching URL name found for path: {path}") return None + +def value_of_attribute(obj, attribute_name: str): + """Returns the value of getattr if the attribute isn't callable. + If it is, execute the underlying function and return that result instead.""" + value = getattr(obj, attribute_name) + if callable(value): + value = value() + return value diff --git a/src/registrar/templates/django/admin/suborg_change_form.html b/src/registrar/templates/django/admin/suborg_change_form.html index 005d67aec..25fe5700d 100644 --- a/src/registrar/templates/django/admin/suborg_change_form.html +++ b/src/registrar/templates/django/admin/suborg_change_form.html @@ -8,27 +8,35 @@

    Domain requests

    Domains

      - {% for domain in domains %} -
    • - - {{ domain.name }} - - ({{ domain.state }}) -
      • - {% endfor %} + {% if domains|length > 0 %} + {% for domain in domains %} +
    • + + {{ domain.name }} + + ({{ domain.state }}) +
      • + {% endfor %} + {% else %} +
    • No domains.
      • + {% endif %}
    diff --git a/src/registrar/templates/django/admin/user_change_form.html b/src/registrar/templates/django/admin/user_change_form.html index 736f12ba4..c0ddd8caf 100644 --- a/src/registrar/templates/django/admin/user_change_form.html +++ b/src/registrar/templates/django/admin/user_change_form.html @@ -17,26 +17,6 @@ {% endblock %} {% block after_related_objects %} - {% if portfolios %} -
    -

    Portfolio information

    -
    -
    -

    Portfolios

    - -
    -
    -
    - {% endif %} -

    Associated requests and domains

    diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 0b99bba13..32d2ad09d 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -1,5 +1,9 @@ from registrar.models.domain_request import DomainRequest from django.template.loader import get_template +from django.utils.html import format_html +from django.urls import reverse +from django.utils.html import escape +from registrar.models.utility.generic_helper import value_of_attribute def get_all_action_needed_reason_emails(request, domain_request): @@ -34,3 +38,50 @@ def get_action_needed_reason_default_email(request, domain_request, action_neede email_body_text_cleaned = email_body_text.strip().lstrip("\n") return email_body_text_cleaned + + +def get_field_links_as_list( + queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None + ): + """ + Generate HTML links for items in a queryset, using a specified attribute for link text. + + Args: + queryset: The queryset of items to generate links for. + model_name: The model name used to construct the admin change URL. + attribute_name: The attribute or method name to use for link text. If None, the item itself is used. + link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. + separator: The separator to use between links in the resulting HTML. + If none, an unordered list is returned. + + Returns: + A formatted HTML string with links to the admin change pages for each item. + """ + links = [] + for item in queryset: + + # This allows you to pass in attribute_name="get_full_name" for instance. + if attribute_name: + item_display_value = value_of_attribute(item, attribute_name) + else: + item_display_value = item + + if item_display_value: + change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) + + link = f'{escape(item_display_value)}' + if link_info_attribute: + link += f" ({value_of_attribute(item, link_info_attribute)})" + + if separator: + links.append(link) + else: + links.append(f"
  • {link}
    • ") + + # If no separator is specified, just return an unordered list. + if separator: + return format_html(separator.join(links)) if links else "-" + else: + links = "".join(links) + return format_html(f'
      {links}
    ') if links else "-" + From 4c756bba69a950763cf6ced2189aeeb0b376ccef Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:24:42 -0600 Subject: [PATCH 35/42] User changes --- src/registrar/admin.py | 2 +- src/registrar/utility/admin_helpers.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e3a3b2b2d..1ac081eeb 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -865,7 +865,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): """Returns a list of links for each related suborg""" portfolio_ids = obj.get_portfolios().values_list("portfolio", flat=True) queryset = models.Portfolio.objects.filter(id__in=portfolio_ids) - return get_field_links_as_list(queryset, "portfolio") + return get_field_links_as_list(queryset, "portfolio", msg_for_none="No portfolios.") portfolios.short_description = "Portfolios" # type: ignore diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 32d2ad09d..a6428f826 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -41,7 +41,7 @@ def get_action_needed_reason_default_email(request, domain_request, action_neede def get_field_links_as_list( - queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None + queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None, msg_for_none="-", ): """ Generate HTML links for items in a queryset, using a specified attribute for link text. @@ -53,6 +53,8 @@ def get_field_links_as_list( link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. separator: The separator to use between links in the resulting HTML. If none, an unordered list is returned. + msg_for_none: What to return when the field would otherwise display None. + Defaults to `-`. Returns: A formatted HTML string with links to the admin change pages for each item. @@ -80,8 +82,8 @@ def get_field_links_as_list( # If no separator is specified, just return an unordered list. if separator: - return format_html(separator.join(links)) if links else "-" + return format_html(separator.join(links)) if links else msg_for_none else: links = "".join(links) - return format_html(f'
      {links}
    ') if links else "-" + return format_html(f'
      {links}
    ') if links else msg_for_none From e798410ac4cf942a44b6a1456ba42588ba4912d6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:32:46 -0600 Subject: [PATCH 36/42] update domain info / domain request --- ...er_domaininformation_portfolio_and_more.py | 64 +++++++++++++++++++ src/registrar/models/domain_information.py | 5 +- src/registrar/models/domain_request.py | 5 +- 3 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py diff --git a/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py new file mode 100644 index 000000000..bf1513f7d --- /dev/null +++ b/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py @@ -0,0 +1,64 @@ +# Generated by Django 4.2.10 on 2024-09-30 14:31 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0130_remove_federalagency_initials_federalagency_acronym_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="domaininformation", + name="portfolio", + field=models.ForeignKey( + blank=True, + help_text="If blank, domain is not associated with a portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="sub_organization", + field=models.ForeignKey( + blank=True, + help_text="If blank, domain is associated with the overarching organization for this portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_sub_organization", + to="registrar.suborganization", + verbose_name="Suborganization", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="portfolio", + field=models.ForeignKey( + blank=True, + help_text="If blank, request is not associated with a portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="DomainRequest_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="sub_organization", + field=models.ForeignKey( + blank=True, + help_text="If blank, request is associated with the overarching organization for this portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="request_sub_organization", + to="registrar.suborganization", + verbose_name="Suborganization", + ), + ), + ] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index d04f09c07..6e99dfbee 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -63,7 +63,7 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, related_name="information_portfolio", - help_text="Portfolio associated with this domain", + help_text="If blank, domain is not associated with a portfolio.", ) sub_organization = models.ForeignKey( @@ -72,7 +72,8 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, related_name="information_sub_organization", - help_text="The suborganization that this domain is included under", + help_text="If blank, domain is associated with the overarching organization for this portfolio.", + verbose_name="Suborganization", ) domain_request = models.OneToOneField( diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index bb8693ac1..35c121f3e 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -327,7 +327,7 @@ class DomainRequest(TimeStampedModel): null=True, blank=True, related_name="DomainRequest_portfolio", - help_text="Portfolio associated with this domain request", + help_text="If blank, request is not associated with a portfolio.", ) sub_organization = models.ForeignKey( @@ -336,7 +336,8 @@ class DomainRequest(TimeStampedModel): null=True, blank=True, related_name="request_sub_organization", - help_text="The suborganization that this domain request is included under", + help_text="If blank, request is associated with the overarching organization for this portfolio.", + verbose_name="Suborganization", ) # This is the domain request user who created this domain request. From df3f37bf404372edc1dda26869b644cc1a2dc15c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:36:19 -0600 Subject: [PATCH 37/42] fix script --- docs/operations/data_migration.md | 2 +- .../commands/populate_federal_agency_initials_and_fceb.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 5e1aa688a..a234d882b 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -754,7 +754,7 @@ Example: `cf ssh getgov-za` | 1 | **emailTo** | Specifies where the email will be emailed. Defaults to help@get.gov on production. | ## Populate federal agency initials and FCEB -This script adds to the "is_fceb" and "initials" fields on the FederalAgency model. This script expects a CSV of federal CIOs to pull from, which can be sourced from [here](https://docs.google.com/spreadsheets/d/14oXHFpKyUXS5_mDWARPusghGdHCrP67jCleOknaSx38/edit?gid=479328070#gid=479328070). +This script adds to the "is_fceb" and "acronym" fields on the FederalAgency model. This script expects a CSV of federal CIOs to pull from, which can be sourced from [here](https://docs.google.com/spreadsheets/d/14oXHFpKyUXS5_mDWARPusghGdHCrP67jCleOknaSx38/edit?gid=479328070#gid=479328070). ### Running on sandboxes diff --git a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py index 30ae08b47..50b481e7f 100644 --- a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py +++ b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py @@ -36,7 +36,7 @@ class Command(BaseCommand, PopulateScriptTemplate): self.federal_agency_dict[agency_name.strip()] = (initials, agency_status) # Update every federal agency record - self.mass_update_records(FederalAgency, {"agency__isnull": False}, ["initials", "is_fceb"]) + self.mass_update_records(FederalAgency, {"agency__isnull": False}, ["acronym", "is_fceb"]) def update_record(self, record: FederalAgency): """For each record, update the initials and is_fceb field if data exists for it""" From 7602629dc5fade38c81aecf2444dd1e991e8938c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 09:08:01 -0600 Subject: [PATCH 38/42] linting --- src/registrar/admin.py | 6 +- .../models/utility/generic_helper.py | 1 + src/registrar/utility/admin_helpers.py | 86 ++++++++++--------- 3 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1ac081eeb..ca51e8b72 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -20,7 +20,11 @@ from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email, get_field_links_as_list +from registrar.utility.admin_helpers import ( + get_all_action_needed_reason_emails, + get_action_needed_reason_default_email, + get_field_links_as_list, +) from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index b02068de0..5e425f5a3 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -335,6 +335,7 @@ def get_url_name(path): logger.error(f"No matching URL name found for path: {path}") return None + def value_of_attribute(obj, attribute_name: str): """Returns the value of getattr if the attribute isn't callable. If it is, execute the underlying function and return that result instead.""" diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index a6428f826..2af9d0b3c 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -41,49 +41,53 @@ def get_action_needed_reason_default_email(request, domain_request, action_neede def get_field_links_as_list( - queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None, msg_for_none="-", - ): - """ - Generate HTML links for items in a queryset, using a specified attribute for link text. + queryset, + model_name, + attribute_name=None, + link_info_attribute=None, + separator=None, + msg_for_none="-", +): + """ + Generate HTML links for items in a queryset, using a specified attribute for link text. - Args: - queryset: The queryset of items to generate links for. - model_name: The model name used to construct the admin change URL. - attribute_name: The attribute or method name to use for link text. If None, the item itself is used. - link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. - separator: The separator to use between links in the resulting HTML. - If none, an unordered list is returned. - msg_for_none: What to return when the field would otherwise display None. - Defaults to `-`. + Args: + queryset: The queryset of items to generate links for. + model_name: The model name used to construct the admin change URL. + attribute_name: The attribute or method name to use for link text. If None, the item itself is used. + link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. + separator: The separator to use between links in the resulting HTML. + If none, an unordered list is returned. + msg_for_none: What to return when the field would otherwise display None. + Defaults to `-`. - Returns: - A formatted HTML string with links to the admin change pages for each item. - """ - links = [] - for item in queryset: + Returns: + A formatted HTML string with links to the admin change pages for each item. + """ + links = [] + for item in queryset: - # This allows you to pass in attribute_name="get_full_name" for instance. - if attribute_name: - item_display_value = value_of_attribute(item, attribute_name) - else: - item_display_value = item - - if item_display_value: - change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) - - link = f'{escape(item_display_value)}' - if link_info_attribute: - link += f" ({value_of_attribute(item, link_info_attribute)})" - - if separator: - links.append(link) - else: - links.append(f"
  • {link}
    • ") - - # If no separator is specified, just return an unordered list. - if separator: - return format_html(separator.join(links)) if links else msg_for_none + # This allows you to pass in attribute_name="get_full_name" for instance. + if attribute_name: + item_display_value = value_of_attribute(item, attribute_name) else: - links = "".join(links) - return format_html(f'
      {links}
    ') if links else msg_for_none + item_display_value = item + if item_display_value: + change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) + + link = f'{escape(item_display_value)}' + if link_info_attribute: + link += f" ({value_of_attribute(item, link_info_attribute)})" + + if separator: + links.append(link) + else: + links.append(f"
  • {link}
    • ") + + # If no separator is specified, just return an unordered list. + if separator: + return format_html(separator.join(links)) if links else msg_for_none + else: + links = "".join(links) + return format_html(f'
      {links}
    ') if links else msg_for_none From 1122edc5c588d2722c9a29234c89708880c85d12 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:06:19 -0600 Subject: [PATCH 39/42] Fix migrations --- ...nitials_federalagency_acronym_and_more.py} | 61 ++++++++++++++++-- ...roups_v17.py => 0130_create_groups_v17.py} | 2 +- ...er_domaininformation_portfolio_and_more.py | 64 ------------------- src/registrar/models/user_group.py | 5 ++ 4 files changed, 62 insertions(+), 70 deletions(-) rename src/registrar/migrations/{0130_remove_federalagency_initials_federalagency_acronym_and_more.py => 0129_remove_federalagency_initials_federalagency_acronym_and_more.py} (56%) rename src/registrar/migrations/{0129_create_groups_v17.py => 0130_create_groups_v17.py} (93%) delete mode 100644 src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py diff --git a/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py b/src/registrar/migrations/0129_remove_federalagency_initials_federalagency_acronym_and_more.py similarity index 56% rename from src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py rename to src/registrar/migrations/0129_remove_federalagency_initials_federalagency_acronym_and_more.py index 6e5935748..8371d8136 100644 --- a/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py +++ b/src/registrar/migrations/0129_remove_federalagency_initials_federalagency_acronym_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-09-27 20:20 +# Generated by Django 4.2.10 on 2024-09-30 17:59 from django.db import migrations, models import django.db.models.deletion @@ -8,15 +8,16 @@ import registrar.models.federal_agency class Migration(migrations.Migration): dependencies = [ - ("registrar", "0129_create_groups_v17"), + ("registrar", "0128_alter_domaininformation_state_territory_and_more"), ] operations = [ - migrations.RemoveField( + migrations.RenameField( model_name="federalagency", - name="initials", + old_name="initials", + new_name="acronym", ), - migrations.AddField( + migrations.AlterField( model_name="federalagency", name="acronym", field=models.CharField( @@ -26,6 +27,56 @@ class Migration(migrations.Migration): null=True, ), ), + migrations.AlterField( + model_name="domaininformation", + name="portfolio", + field=models.ForeignKey( + blank=True, + help_text="If blank, domain is not associated with a portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="sub_organization", + field=models.ForeignKey( + blank=True, + help_text="If blank, domain is associated with the overarching organization for this portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_sub_organization", + to="registrar.suborganization", + verbose_name="Suborganization", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="portfolio", + field=models.ForeignKey( + blank=True, + help_text="If blank, request is not associated with a portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="DomainRequest_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="sub_organization", + field=models.ForeignKey( + blank=True, + help_text="If blank, request is associated with the overarching organization for this portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="request_sub_organization", + to="registrar.suborganization", + verbose_name="Suborganization", + ), + ), migrations.AlterField( model_name="federalagency", name="federal_type", diff --git a/src/registrar/migrations/0129_create_groups_v17.py b/src/registrar/migrations/0130_create_groups_v17.py similarity index 93% rename from src/registrar/migrations/0129_create_groups_v17.py rename to src/registrar/migrations/0130_create_groups_v17.py index 7e0ae99ad..c7d10693a 100644 --- a/src/registrar/migrations/0129_create_groups_v17.py +++ b/src/registrar/migrations/0130_create_groups_v17.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0128_alter_domaininformation_state_territory_and_more"), + ("registrar", "0129_remove_federalagency_initials_federalagency_acronym_and_more"), ] operations = [ diff --git a/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py deleted file mode 100644 index bf1513f7d..000000000 --- a/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py +++ /dev/null @@ -1,64 +0,0 @@ -# Generated by Django 4.2.10 on 2024-09-30 14:31 - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0130_remove_federalagency_initials_federalagency_acronym_and_more"), - ] - - operations = [ - migrations.AlterField( - model_name="domaininformation", - name="portfolio", - field=models.ForeignKey( - blank=True, - help_text="If blank, domain is not associated with a portfolio.", - null=True, - on_delete=django.db.models.deletion.PROTECT, - related_name="information_portfolio", - to="registrar.portfolio", - ), - ), - migrations.AlterField( - model_name="domaininformation", - name="sub_organization", - field=models.ForeignKey( - blank=True, - help_text="If blank, domain is associated with the overarching organization for this portfolio.", - null=True, - on_delete=django.db.models.deletion.PROTECT, - related_name="information_sub_organization", - to="registrar.suborganization", - verbose_name="Suborganization", - ), - ), - migrations.AlterField( - model_name="domainrequest", - name="portfolio", - field=models.ForeignKey( - blank=True, - help_text="If blank, request is not associated with a portfolio.", - null=True, - on_delete=django.db.models.deletion.PROTECT, - related_name="DomainRequest_portfolio", - to="registrar.portfolio", - ), - ), - migrations.AlterField( - model_name="domainrequest", - name="sub_organization", - field=models.ForeignKey( - blank=True, - help_text="If blank, request is associated with the overarching organization for this portfolio.", - null=True, - on_delete=django.db.models.deletion.PROTECT, - related_name="request_sub_organization", - to="registrar.suborganization", - verbose_name="Suborganization", - ), - ), - ] diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index 41ae67c06..84b4da701 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -76,6 +76,11 @@ class UserGroup(Group): "model": "suborganization", "permissions": ["add_suborganization", "change_suborganization", "delete_suborganization"], }, + { + "app_label": "registrar", + "model": "seniorofficial", + "permissions": ["add_seniorofficial", "change_seniorofficial", "delete_seniorofficial"], + }, { "app_label": "registrar", "model": "userportfoliopermission", From 0e7969f7c781e16e85af529f9468575219df900a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 12:27:41 -0600 Subject: [PATCH 40/42] fix test --- src/registrar/tests/test_migrations.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index d646a03de..eaaae8727 100644 --- a/src/registrar/tests/test_migrations.py +++ b/src/registrar/tests/test_migrations.py @@ -43,6 +43,9 @@ class TestGroups(TestCase): "add_portfolio", "change_portfolio", "delete_portfolio", + "add_seniorofficial", + "change_seniorofficial", + "delete_seniorofficial", "add_suborganization", "change_suborganization", "delete_suborganization", From f67c4e0da2b3ea47d9e90f19db3268bdb61a14bc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 2 Oct 2024 08:17:29 -0600 Subject: [PATCH 41/42] Remove helper text --- ...er_domaininformation_portfolio_and_more.py | 36 +++++++++++++++++++ src/registrar/models/domain_information.py | 1 - src/registrar/models/domain_request.py | 1 - 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py diff --git a/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py new file mode 100644 index 000000000..ee0232a17 --- /dev/null +++ b/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py @@ -0,0 +1,36 @@ +# Generated by Django 4.2.10 on 2024-10-02 14:17 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0130_create_groups_v17"), + ] + + operations = [ + migrations.AlterField( + model_name="domaininformation", + name="portfolio", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="portfolio", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="DomainRequest_portfolio", + to="registrar.portfolio", + ), + ), + ] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 6e99dfbee..5f98197bd 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -63,7 +63,6 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, related_name="information_portfolio", - help_text="If blank, domain is not associated with a portfolio.", ) sub_organization = models.ForeignKey( diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 35c121f3e..79bc223e9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -327,7 +327,6 @@ class DomainRequest(TimeStampedModel): null=True, blank=True, related_name="DomainRequest_portfolio", - help_text="If blank, request is not associated with a portfolio.", ) sub_organization = models.ForeignKey( From 8d0c1ba6bf43e21d345be2fa3e7f9946edc5346c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 3 Oct 2024 13:06:12 -0600 Subject: [PATCH 42/42] fix migrations (merge conflict --- ...ve_federalagency_initials_federalagency_acronym_and_more.py} | 2 +- .../{0130_create_groups_v17.py => 0131_create_groups_v17.py} | 2 +- ...re.py => 0132_alter_domaininformation_portfolio_and_more.py} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename src/registrar/migrations/{0129_remove_federalagency_initials_federalagency_acronym_and_more.py => 0130_remove_federalagency_initials_federalagency_acronym_and_more.py} (98%) rename src/registrar/migrations/{0130_create_groups_v17.py => 0131_create_groups_v17.py} (95%) rename src/registrar/migrations/{0131_alter_domaininformation_portfolio_and_more.py => 0132_alter_domaininformation_portfolio_and_more.py} (95%) diff --git a/src/registrar/migrations/0129_remove_federalagency_initials_federalagency_acronym_and_more.py b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py similarity index 98% rename from src/registrar/migrations/0129_remove_federalagency_initials_federalagency_acronym_and_more.py rename to src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py index 8371d8136..c0e2a0b22 100644 --- a/src/registrar/migrations/0129_remove_federalagency_initials_federalagency_acronym_and_more.py +++ b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py @@ -8,7 +8,7 @@ import registrar.models.federal_agency class Migration(migrations.Migration): dependencies = [ - ("registrar", "0128_alter_domaininformation_state_territory_and_more"), + ("registrar", "0129_alter_portfolioinvitation_portfolio_roles_and_more"), ] operations = [ diff --git a/src/registrar/migrations/0130_create_groups_v17.py b/src/registrar/migrations/0131_create_groups_v17.py similarity index 95% rename from src/registrar/migrations/0130_create_groups_v17.py rename to src/registrar/migrations/0131_create_groups_v17.py index c7d10693a..04cd5163c 100644 --- a/src/registrar/migrations/0130_create_groups_v17.py +++ b/src/registrar/migrations/0131_create_groups_v17.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0129_remove_federalagency_initials_federalagency_acronym_and_more"), + ("registrar", "0130_remove_federalagency_initials_federalagency_acronym_and_more"), ] operations = [ diff --git a/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py similarity index 95% rename from src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py rename to src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py index ee0232a17..e0d2b35f9 100644 --- a/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py +++ b/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py @@ -7,7 +7,7 @@ import django.db.models.deletion class Migration(migrations.Migration): dependencies = [ - ("registrar", "0130_create_groups_v17"), + ("registrar", "0131_create_groups_v17"), ] operations = [