diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 5e1aa688ac..a234d882b7 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/admin.py b/src/registrar/admin.py index 8718da9bad..ca51e8b72e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -8,9 +8,7 @@ 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.domain_invitation import DomainInvitation -from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models import DomainInformation, Portfolio, UserPortfolioPermission, DomainInvitation from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from waffle.decorators import flag_is_active from django.contrib import admin, messages @@ -22,7 +20,11 @@ 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 @@ -757,9 +759,10 @@ def overridden_email_field(self, obj): }, ), ("Important dates", {"fields": ("last_login", "date_joined")}), + ("Associated portfolios", {"fields": ("portfolios",)}), ) - readonly_fields = ("verification_type",) + readonly_fields = ("verification_type", "portfolios") analyst_fieldsets = ( ( @@ -782,6 +785,7 @@ def overridden_email_field(self, obj): }, ), ("Important dates", {"fields": ("last_login", "date_joined")}), + ("Associated portfolios", {"fields": ("portfolios",)}), ) # TODO: delete after we merge organization feature @@ -861,6 +865,14 @@ def overridden_email_field(self, obj): 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", msg_for_none="No portfolios.") + + 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, @@ -1257,9 +1269,18 @@ class Meta: 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" # type: ignore class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): @@ -1543,33 +1564,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 = [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 +1859,6 @@ def status_history(self, obj): ] 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 @@ -3006,39 +2973,59 @@ def save_model(self, request, obj, form, change): 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}" - (None, {"fields": ["portfolio_type", "organization_name", "creator", "created_on", "notes"]}), - ("Portfolio members", {"fields": ["display_admins", "display_members"]}), - ("Portfolio domains", {"fields": ["domains", "domain_requests"]}), + # created_on is the created_at field + (None, {"fields": ["creator", "created_on", "notes"]}), ("Type of organization", {"fields": ["organization_type", "federal_type"]}), ( "Organization name and mailing address", { "fields": [ + "organization_name", "federal_agency", + ] + }, + ), + ( + "Show details", + { + "classes": ["collapse--dgfieldset"], + "description": "Extends organization name and mailing address", + "fields": [ "state_territory", "address_line1", "address_line2", "city", "zipcode", "urbanization", - ] + ], }, ), + ("Portfolio members", {"fields": ["display_admins", "display_members"]}), + ("Domains and requests", {"fields": ["domains", "domain_requests"]}), ("Suborganizations", {"fields": ["suborganizations"]}), ("Senior official", {"fields": ["senior_official"]}), ] # This is the fieldset display when adding a new model add_fieldsets = [ - (None, {"fields": ["organization_name", "creator", "notes"]}), + (None, {"fields": ["creator", "notes"]}), ("Type of organization", {"fields": ["organization_type"]}), ( "Organization name and mailing address", { "fields": [ + "organization_name", "federal_agency", "state_territory", "address_line1", @@ -3052,7 +3039,7 @@ 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." readonly_fields = [ @@ -3065,23 +3052,35 @@ class PortfolioAdmin(ListHeaderAdmin): "domains", "domain_requests", "suborganizations", - "portfolio_type", "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 = [ + "organization_name", ] 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.""" + if obj: + return obj.portfolio_users.filter( + portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + else: + return [] + def get_non_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio that do NOT have the "Admin" role non_admin_permissions = UserPortfolioPermission.objects.filter(portfolio=obj).exclude( @@ -3093,82 +3092,12 @@ def get_non_admin_users(self, obj): return non_admin_users - def display_admins(self, obj): - """Get joined users who are 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 field_readonly block""" - admins = self.get_admin_users(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)}" - admin_details += "
" - admin_details += f"" - admin_details += ( - "" - admin_details += "

" - admin_details += f"{escape(portfolio_admin.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_non_admin_users(obj) - if not members: - return "" - - member_details = ( - "" - + "" - ) - for member in members: - full_name = member.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.title)}{escape(member.email)}{escape(member.phone)}" - for role in member.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_non_admin_users(obj) - if not members: - return {} - - return self.get_field_links_as_list(members, "user", separator=", ") + def get_user_portfolio_permission_non_admins(self, obj): + """Returns each admin on UserPortfolioPermission for a given portfolio.""" + if obj: + return obj.portfolio_users.exclude(roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) + else: + return [] def federal_type(self, obj: models.Portfolio): """Returns the federal_type field""" @@ -3183,16 +3112,10 @@ def created_on(self, obj: models.Portfolio): 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() - return self.get_field_links_as_list(queryset, "suborganization") + return get_field_links_as_list(queryset, "suborganization") suborganizations.short_description = "Suborganizations" # type: ignore @@ -3221,6 +3144,28 @@ def domain_requests(self, obj: models.Portfolio): 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", @@ -3228,59 +3173,6 @@ def domain_requests(self, obj: models.Portfolio): "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'') 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 @@ -3313,10 +3205,15 @@ def get_readonly_fields(self, request, obj=None): 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["display_members_summary"] = self.display_members_summary(obj) + + 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): @@ -3333,6 +3230,14 @@ def save_model(self, request, obj, form, change): 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: + 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) @@ -3347,7 +3252,7 @@ class Meta: 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] @@ -3404,6 +3309,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/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 7e3c086c44..73f3dded16 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -802,10 +802,15 @@ document.addEventListener('DOMContentLoaded', function() { // $ 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 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); + handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType); }); } @@ -821,9 +826,33 @@ document.addEventListener('DOMContentLoaded', function() { handleStateTerritoryChange(stateTerritory, urbanizationField); }); } + + // Handle hiding the organization name field when the organization_type is federal. + // Run this first one page load, then secondly on a change event. + handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); + organizationType.addEventListener("change", function() { + handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); + }); }); - function handleFederalAgencyChange(federalAgency, organizationType) { + function handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType) { + if (organizationType && organizationNameContainer) { + let selectedValue = organizationType.value; + if (selectedValue === "federal") { + hideElement(organizationNameContainer); + if (federalType) { + showElement(federalType); + } + } else { + showElement(organizationNameContainer); + if (federalType) { + hideElement(federalType); + } + } + } + } + + function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType) { // Don't do anything on page load if (isInitialPageLoad) { isInitialPageLoad = false; @@ -838,27 +867,31 @@ document.addEventListener('DOMContentLoaded', function() { 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 = "-" + } } } - // 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; - } + 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; - fetch(`${federalPortfolioApi}?organization_type=${organizationType.value}&agency_name=${selectedText}`) + fetch(`${federalPortfolioApi}?&agency_name=${selectedText}`) .then(response => { const statusCode = response.status; return response.json().then(data => ({ statusCode, data })); @@ -869,7 +902,6 @@ document.addEventListener('DOMContentLoaded', function() { return; } updateReadOnly(data.federal_type, '.field-federal_type'); - updateReadOnly(data.portfolio_type, '.field-portfolio_type'); }) .catch(error => console.error("Error fetching federal and portfolio types: ", error)); @@ -877,6 +909,9 @@ document.addEventListener('DOMContentLoaded', function() { // 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; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) .then(response => { @@ -887,7 +922,12 @@ document.addEventListener('DOMContentLoaded', function() { 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 found. Create one now.`; + } console.warn("Record not found: " + data.error); }else { console.error("Error in AJAX call: " + data.error); @@ -898,30 +938,43 @@ document.addEventListener('DOMContentLoaded', function() { // 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 && $seniorOfficial.length > 0) { + // If the senior official is a dropdown field, edit that + updateSeniorOfficialDropdown($seniorOfficial, seniorOfficialId, seniorOfficialName); + }else { + 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/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 681bebc7cf..5cea72c4c0 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -455,7 +455,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){ @@ -922,4 +923,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; +} diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 9eb649ad81..30882cd5d4 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) 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 506405b78e..50b481e7fc 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,13 +36,13 @@ def handle(self, federal_cio_csv_path, **kwargs): 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""" 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_remove_federalagency_initials_federalagency_acronym_and_more.py b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py new file mode 100644 index 0000000000..c0e2a0b228 --- /dev/null +++ b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py @@ -0,0 +1,146 @@ +# Generated by Django 4.2.10 on 2024-09-30 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_alter_portfolioinvitation_portfolio_roles_and_more"), + ] + + operations = [ + migrations.RenameField( + model_name="federalagency", + old_name="initials", + new_name="acronym", + ), + migrations.AlterField( + 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="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", + 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", + 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, + ), + ), + 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", + ), + ), + migrations.AlterField( + model_name="suborganization", + name="name", + field=models.CharField(max_length=1000, unique=True, verbose_name="Suborganization"), + ), + ] diff --git a/src/registrar/migrations/0131_create_groups_v17.py b/src/registrar/migrations/0131_create_groups_v17.py new file mode 100644 index 0000000000..04cd5163c2 --- /dev/null +++ b/src/registrar/migrations/0131_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", "0130_remove_federalagency_initials_federalagency_acronym_and_more"), + ] + + operations = [ + migrations.RunPython( + create_groups, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ] diff --git a/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0132_alter_domaininformation_portfolio_and_more.py new file mode 100644 index 0000000000..e0d2b35f99 --- /dev/null +++ b/src/registrar/migrations/0132_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", "0131_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 d04f09c07e..5f98197bd8 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -63,7 +63,6 @@ class Meta: null=True, blank=True, related_name="information_portfolio", - help_text="Portfolio associated with this domain", ) sub_organization = models.ForeignKey( @@ -72,7 +71,8 @@ class Meta: 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 bb8693ac13..79bc223e92 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -327,7 +327,6 @@ def get_action_needed_reason_label(cls, action_needed_reason: str): null=True, blank=True, related_name="DomainRequest_portfolio", - help_text="Portfolio associated with this domain request", ) sub_organization = models.ForeignKey( @@ -336,7 +335,8 @@ def get_action_needed_reason_label(cls, action_needed_reason: str): 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. diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 5cc87b38c4..aeeebac8c3 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -22,21 +22,20 @@ class Meta: 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/models/portfolio.py b/src/registrar/models/portfolio.py index 61d4f7a305..8d820e105b 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -2,7 +2,6 @@ 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 @@ -34,7 +33,6 @@ class Meta: organization_name = models.CharField( null=True, blank=True, - verbose_name="Portfolio organization", ) organization_type = models.CharField( @@ -42,7 +40,6 @@ class Meta: choices=OrganizationChoices.choices, null=True, blank=True, - help_text="Type of organization", ) notes = models.TextField( @@ -53,7 +50,6 @@ class Meta: federal_agency = models.ForeignKey( "registrar.FederalAgency", on_delete=models.PROTECT, - help_text="Associated federal agency", unique=False, default=FederalAgency.get_non_federal_agency, ) @@ -64,6 +60,7 @@ class Meta: unique=False, null=True, blank=True, + related_name="portfolios", ) address_line1 = models.CharField( @@ -125,23 +122,6 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) - @property - def portfolio_type(self): - """ - Returns a combination of organization_type / federal_type, seperated by ' - '. - If no federal_type is found, we just return the org type. - """ - return self.get_portfolio_type(self.organization_type, self.federal_type) - - @classmethod - def get_portfolio_type(cls, organization_type, federal_type): - org_type_label = cls.OrganizationChoices.get_org_label(organization_type) - agency_type_label = BranchChoices.get_branch_label(federal_type) - if organization_type == cls.OrganizationChoices.FEDERAL and agency_type_label: - return " - ".join([org_type_label, agency_type_label]) - else: - return org_type_label - @property def federal_type(self): """Returns the federal_type value on the underlying federal_agency field""" @@ -152,13 +132,19 @@ def get_federal_type(cls, federal_agency): 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/suborganization.py b/src/registrar/models/suborganization.py index feeee0669a..6ad80fdc0c 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/user_group.py b/src/registrar/models/user_group.py index 182d16e956..4770f34bc2 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -66,6 +66,30 @@ def create_cisa_analyst_group(apps, schema_editor): "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": "seniorofficial", + "permissions": ["add_seniorofficial", "change_seniorofficial", "delete_seniorofficial"], + }, + { + "app_label": "registrar", + "model": "userportfoliopermission", + "permissions": [ + "add_userportfoliopermission", + "change_userportfoliopermission", + "delete_userportfoliopermission", + ], + }, ] # Avoid error: You can't execute queries until the end diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 6acd651dbc..241afd3281 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -65,7 +65,19 @@ class Meta: ) def __str__(self): - return f"User '{self.user}' on Portfolio '{self.portfolio}' " f"" if self.roles else "" + readable_roles = [] + if 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): """ @@ -93,7 +105,8 @@ def clean(self): 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/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 3cafe87c4a..5e425f5a37 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -334,3 +334,12 @@ 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/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 29e2a377c9..ddb487f710 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -9,6 +9,10 @@ class UserPortfolioRoleChoices(models.TextChoices): ORGANIZATION_ADMIN = "organization_admin", "Admin" 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): """ """ @@ -28,3 +32,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 diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 5a75577df8..2ccea93216 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -49,11 +49,15 @@ def __init__(self, get_response): 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 = [ self.profile_page, self.logout_page, "/admin", + "/__debug__", ] self.excluded_pages = { 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 7cc72e8e1a..0a28a65329 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/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index bee42d24e2..6b755724e6 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 @@ {% 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" %}
    @@ -288,13 +288,6 @@

    {% 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/details_button.html b/src/registrar/templates/django/admin/includes/details_button.html new file mode 100644 index 0000000000..73748f170a --- /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%} +
    +
    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 0000000000..4ea9225daa --- /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 0000000000..46303efced --- /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 %} 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 0000000000..56621b7693 --- /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%} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html new file mode 100644 index 0000000000..87b56cb60e --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html @@ -0,0 +1,61 @@ +{% extends "django/admin/includes/detail_table_fieldset.html" %} +{% load custom_filters %} +{% load static url_helpers %} + +{% block field_readonly %} + {% if field.field.name == "display_admins" or field.field.name == "display_members" %} +
    {{ field.contents|safe }}
    + {% 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 %} +
    + {% elif field.field.name == "senior_official" %} + {% if original_object.senior_official %} +
    {{ field.contents }}
    + {% else %} + {% url "admin:registrar_seniorofficial_add" as url %} +
    + No senior official found. Create one now. +
    + {% 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 hide_no_contact_info_message=True %} +
    + {% elif field.field.name == "display_admins" %} + {% if admins|length > 0 %} + {% 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/portfolio_members_table.html" with members=members %} + {% endif %} + {% elif field.field.name == "domains" %} + {% if domains|length > 0 %} + {% 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/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} + {% endif %} + {% endif %} +{% 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 new file mode 100644 index 0000000000..136fe3a5a0 --- /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 %} diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 8dae8a0802..8de6cd5eb0 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -8,19 +8,14 @@ {% url 'get-federal-and-portfolio-types-from-federal-agency-json' as url %} + {% url "admin:registrar_seniorofficial_add" as url %} + {{ block.super }} {% endblock content %} {% 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/detail_table_fieldset.html" with original_object=original %} + {% include "django/admin/includes/portfolio/portfolio_fieldset.html" with original_object=original %} {% endfor %} {% endblock %} diff --git a/src/registrar/templates/django/admin/suborg_change_form.html b/src/registrar/templates/django/admin/suborg_change_form.html index 005d67aecd..25fe5700d6 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 @@

    Associated requests and domains

    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 736f12ba46..c0ddd8cafa 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/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 0000000000..1249a486c4 --- /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 diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 2cf0568587..a3f35ae8e2 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -250,3 +250,12 @@ def is_members_subpage(path): "members", ] 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 [] diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 88482e4db9..cdc3c97dee 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2097,36 +2097,11 @@ def test_portfolio_members_display(self): ) display_admins = self.admin.display_admins(self.portfolio) - - self.assertIn( - f'Gerald Meoward meaoward@gov.gov', - display_admins, - ) - self.assertIn("Captain", display_admins) - 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) - - self.assertIn( - f'Mad Max madmax@gov.gov', - display_members_summary, - ) - self.assertIn( - f'Agent Smith thematrix@gov.gov', - display_members_summary, - ) + url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={self.portfolio.id}" + self.assertIn(f'2 administrators', display_admins) 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_api.py b/src/registrar/tests/test_api.py index ef5385d726..218c63d4fa 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -100,7 +100,6 @@ def test_get_federal_and_portfolio_types_json_authenticated_superuser(self): 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): diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index cbdc2c0346..9fcd261f7c 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1387,18 +1387,18 @@ def test_populate_federal_agency_initials_and_fceb(self): 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 @@ def test_populate_federal_agency_initials_and_fceb_missing_agency(self): # 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) diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index 6d8ff7151a..eaaae87277 100644 --- a/src/registrar/tests/test_migrations.py +++ b/src/registrar/tests/test_migrations.py @@ -40,10 +40,22 @@ def test_groups_created(self): "add_federalagency", "change_federalagency", "delete_federalagency", + "add_portfolio", + "change_portfolio", + "delete_portfolio", + "add_seniorofficial", + "change_seniorofficial", + "delete_seniorofficial", + "add_suborganization", + "change_suborganization", + "delete_suborganization", "analyst_access_permission", "change_user", "delete_userdomainrole", "view_userdomainrole", + "add_userportfoliopermission", + "change_userportfoliopermission", + "delete_userportfoliopermission", "add_verifiedbystaff", "change_verifiedbystaff", "delete_verifiedbystaff", @@ -51,6 +63,7 @@ def test_groups_created(self): # 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) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a6cac1389b..015c67dab2 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1332,7 +1332,10 @@ def test_clean_on_creates_multiple_portfolios(self): 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." + ), ) diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 0b99bba13c..2af9d0b3cb 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,56 @@ 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, + 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 `-`. + + 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 + else: + links = "".join(links) + return format_html(f'
      {links}
    ') if links else msg_for_none diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 6a6269baaa..f9522e2e95 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -55,11 +55,9 @@ def get_federal_and_portfolio_types_from_federal_agency_json(request): portfolio_type = None agency_name = request.GET.get("agency_name") - organization_type = request.GET.get("organization_type") agency = FederalAgency.objects.filter(agency=agency_name).first() if agency: federal_type = Portfolio.get_federal_type(agency) - portfolio_type = Portfolio.get_portfolio_type(organization_type, federal_type) federal_type = BranchChoices.get_branch_label(federal_type) if federal_type else "-" response_data = {