diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 18c1052fc..423c0a01b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -9,8 +9,6 @@ from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField -from registrar.models.domain_group import DomainGroup -from registrar.models.suborganization import Suborganization from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from waffle.decorators import flag_is_active from django.contrib import admin, messages @@ -23,6 +21,7 @@ from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial +from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -39,7 +38,7 @@ from import_export import resources from import_export.admin import ImportExportModelAdmin from django.core.exceptions import ObjectDoesNotExist from django.contrib.admin.widgets import FilteredSelectMultiple - +from django.utils.html import format_html from django.utils.translation import gettext_lazy as _ logger = logging.getLogger(__name__) @@ -2864,16 +2863,112 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): class PortfolioAdmin(ListHeaderAdmin): - 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"]}), + # TODO - uncomment in #2521 + # ("Portfolio members", { + # "classes": ("collapse", "closed"), + # "fields": ["administrators", "members"]} + # ), + ("Portfolio domains", {"classes": ("collapse", "closed"), "fields": ["domains", "domain_requests"]}), + ("Type of organization", {"fields": ["organization_type", "federal_type"]}), + ( + "Organization name and mailing address", + { + "fields": [ + "federal_agency", + "state_territory", + "address_line1", + "address_line2", + "city", + "zipcode", + "urbanization", + ] + }, + ), + ("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"]}), + ("Type of organization", {"fields": ["organization_type"]}), + ( + "Organization name and mailing address", + { + "fields": [ + "federal_agency", + "state_territory", + "address_line1", + "address_line2", + "city", + "zipcode", + "urbanization", + ] + }, + ), + ("Senior official", {"fields": ["senior_official"]}), + ] list_display = ("organization_name", "federal_agency", "creator") search_fields = ["organization_name"] search_help_text = "Search by organization name." readonly_fields = [ - "creator", + # This is the created_at field + "created_on", + # Custom fields such as these must be defined as readonly. + "federal_type", + "domains", + "domain_requests", + "suborganizations", + "portfolio_type", ] + 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 "-" + + federal_type.short_description = "Federal type" # type: ignore + + def created_on(self, obj: models.Portfolio): + """Returns the created_at field, with a different short description""" + # Format: Dec 12, 2024 + return obj.created_at.strftime("%b %d, %Y") if obj.created_at else "-" + + 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") + + suborganizations.short_description = "Suborganizations" # type: ignore + + def domains(self, obj: models.Portfolio): + """Returns a list of links for each related domain""" + queryset = obj.get_domains() + return self.get_field_links_as_list( + queryset, "domaininformation", link_info_attribute="get_state_display_of_domain" + ) + + domains.short_description = "Domains" # type: ignore + + def domain_requests(self, obj: models.Portfolio): + """Returns a list of links for each related domain request""" + queryset = obj.get_domain_requests() + return self.get_field_links_as_list(queryset, "domainrequest", link_info_attribute="get_status_display") + + domain_requests.short_description = "Domain requests" # type: ignore + # Creates select2 fields (with search bars) autocomplete_fields = [ "creator", @@ -2881,17 +2976,91 @@ class PortfolioAdmin(ListHeaderAdmin): "senior_official", ] + def get_field_links_as_list( + self, queryset, model_name, attribute_name=None, link_info_attribute=None, seperator=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 seperator: + links.append(link) + else: + links.append(f"
  • {link}
  • ") + + # If no seperator is specified, just return an unordered list. + if seperator: + return format_html(seperator.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 + if not obj: + 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""" - obj = self.get_object(request, object_id) - - # ---- Domain Groups - domain_groups = DomainGroup.objects.filter(portfolio=obj) - - # ---- Suborganizations - suborganizations = Suborganization.objects.filter(portfolio=obj) - - extra_context = {"domain_groups": domain_groups, "suborganizations": suborganizations} + extra_context = {"skip_additional_contact_info": True} 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 93b8359bf..01c93abf6 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -765,20 +765,22 @@ function initializeWidgetOnList(list, parentId) { */ (function dynamicPortfolioFields(){ + // the federal agency change listener fires on page load, which we don't want. + var isInitialPageLoad = true + + // This is the additional information that exists beneath the SO element. + var contactList = document.querySelector(".field-senior_official .dja-address-contact-list"); document.addEventListener('DOMContentLoaded', function() { - + let isPortfolioPage = document.getElementById("portfolio_form"); if (!isPortfolioPage) { return; } - + // $ symbolically denotes that this is using jQuery let $federalAgency = django.jQuery("#id_federal_agency"); let organizationType = document.getElementById("id_organization_type"); if ($federalAgency && organizationType) { - // Execute this function once on load - handleFederalAgencyChange($federalAgency, organizationType); - // Attach the change event listener $federalAgency.on("change", function() { handleFederalAgencyChange($federalAgency, organizationType); @@ -800,6 +802,12 @@ function initializeWidgetOnList(list, parentId) { }); function handleFederalAgencyChange(federalAgency, organizationType) { + // Don't do anything on page load + if (isInitialPageLoad) { + isInitialPageLoad = false; + return; + } + // Set the org type to federal if an agency is selected let selectedText = federalAgency.find("option:selected").text(); @@ -825,6 +833,10 @@ function initializeWidgetOnList(list, parentId) { return; } + // Hide the contactList initially. + // If we can update the contact information, it'll be shown again. + hideElement(contactList.parentElement); + let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) .then(response => { @@ -843,6 +855,10 @@ function initializeWidgetOnList(list, parentId) { return; } + // Update the "contact details" blurb beneath senior official + updateContactInfo(data); + showElement(contactList.parentElement); + let seniorOfficialId = data.id; let seniorOfficialName = [data.first_name, data.last_name].join(" "); if (!seniorOfficialId || !seniorOfficialName || !seniorOfficialName.trim()){ @@ -873,4 +889,35 @@ function initializeWidgetOnList(list, parentId) { hideElement(urbanizationField) } } + + function updateContactInfo(data) { + if (!contactList) return; + + const titleSpan = contactList.querySelector("#contact_info_title"); + const emailSpan = contactList.querySelector("#contact_info_email"); + const phoneSpan = contactList.querySelector("#contact_info_phone"); + + if (titleSpan) { + titleSpan.textContent = data.title || "None"; + }; + + // Update the email field and the content for the clipboard + if (emailSpan) { + let copyButton = contactList.querySelector(".admin-icon-group"); + emailSpan.textContent = data.email || "None"; + if (data.email) { + const clipboardInput = contactList.querySelector(".admin-icon-group input"); + if (clipboardInput) { + clipboardInput.value = data.email; + }; + showElement(copyButton); + }else { + hideElement(copyButton); + } + } + + if (phoneSpan) { + phoneSpan.textContent = data.phone || "None"; + }; + } })(); diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index 711bfe960..8ca6b5465 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -847,3 +847,8 @@ div.dja__model-description{ } } } + +ul.add-list-reset { + padding: 0 !important; + margin: 0 !important; +} diff --git a/src/registrar/migrations/0118_alter_portfolio_options_alter_portfolio_creator_and_more.py b/src/registrar/migrations/0118_alter_portfolio_options_alter_portfolio_creator_and_more.py new file mode 100644 index 000000000..8f84187a2 --- /dev/null +++ b/src/registrar/migrations/0118_alter_portfolio_options_alter_portfolio_creator_and_more.py @@ -0,0 +1,50 @@ +# Generated by Django 4.2.10 on 2024-08-15 15:32 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0117_alter_portfolioinvitation_portfolio_additional_permissions_and_more"), + ] + + operations = [ + migrations.AlterModelOptions( + name="portfolio", + options={"ordering": ["organization_name"]}, + ), + migrations.AlterField( + model_name="portfolio", + name="creator", + field=models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + related_name="created_portfolios", + to=settings.AUTH_USER_MODEL, + verbose_name="Portfolio creator", + ), + ), + migrations.AlterField( + model_name="portfolio", + name="organization_name", + field=models.CharField(blank=True, null=True, verbose_name="Portfolio organization"), + ), + migrations.AlterField( + model_name="portfolio", + name="senior_official", + field=models.ForeignKey( + blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to="registrar.seniorofficial" + ), + ), + migrations.AlterField( + model_name="suborganization", + name="portfolio", + field=models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + related_name="portfolio_suborganizations", + to="registrar.portfolio", + ), + ), + ] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 894bbe6fe..bdd67e582 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -424,3 +424,10 @@ class DomainInformation(TimeStampedModel): def _get_many_to_many_fields(): """Returns a set of each field.name that has the many to many relation""" return {field.name for field in DomainInformation._meta.many_to_many} # type: ignore + + def get_state_display_of_domain(self): + """Returns the state display of the underlying domain record""" + if self.domain: + return self.domain.get_state_display() + else: + return None diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 484c45e8c..0f9904c31 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -2,6 +2,7 @@ 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 @@ -12,6 +13,10 @@ class Portfolio(TimeStampedModel): manageable groups. """ + # Addresses the UnorderedObjectListWarning + class Meta: + ordering = ["organization_name"] + # use the short names in Django admin OrganizationChoices = DomainRequest.OrganizationChoices StateTerritoryChoices = DomainRequest.StateTerritoryChoices @@ -21,11 +26,25 @@ class Portfolio(TimeStampedModel): creator = models.ForeignKey( "registrar.User", on_delete=models.PROTECT, - help_text="Associated user", + verbose_name="Portfolio creator", related_name="created_portfolios", unique=False, ) + organization_name = models.CharField( + null=True, + blank=True, + verbose_name="Portfolio organization", + ) + + organization_type = models.CharField( + max_length=255, + choices=OrganizationChoices.choices, + null=True, + blank=True, + help_text="Type of organization", + ) + notes = models.TextField( null=True, blank=True, @@ -42,25 +61,11 @@ class Portfolio(TimeStampedModel): senior_official = models.ForeignKey( "registrar.SeniorOfficial", on_delete=models.PROTECT, - help_text="Associated senior official", unique=False, null=True, blank=True, ) - organization_type = models.CharField( - max_length=255, - choices=OrganizationChoices.choices, - null=True, - blank=True, - help_text="Type of organization", - ) - - organization_name = models.CharField( - null=True, - blank=True, - ) - address_line1 = models.CharField( null=True, blank=True, @@ -109,7 +114,7 @@ class Portfolio(TimeStampedModel): ) def __str__(self) -> str: - return f"{self.organization_name}" + return str(self.organization_name) def save(self, *args, **kwargs): """Save override for custom properties""" @@ -119,3 +124,35 @@ class Portfolio(TimeStampedModel): self.urbanization = None 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. + """ + org_type_label = self.OrganizationChoices.get_org_label(self.organization_type) + agency_type_label = BranchChoices.get_branch_label(self.federal_type) + if self.organization_type == self.OrganizationChoices.FEDERAL and agency_type_label: + return " - ".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""" + return self.federal_agency.federal_type if self.federal_agency else None + + # == Getters for domains == # + def get_domains(self): + """Returns all DomainInformations associated with this portfolio""" + return self.information_portfolio.all() + + def get_domain_requests(self): + """Returns all DomainRequests associated with this portfolio""" + return self.DomainRequest_portfolio.all() + + # == Getters for suborganization == # + def get_suborganizations(self): + """Returns all suborganizations associated with this portfolio""" + return self.portfolio_suborganizations.all() diff --git a/src/registrar/models/suborganization.py b/src/registrar/models/suborganization.py index b1e010953..feeee0669 100644 --- a/src/registrar/models/suborganization.py +++ b/src/registrar/models/suborganization.py @@ -16,6 +16,7 @@ class Suborganization(TimeStampedModel): portfolio = models.ForeignKey( "registrar.Portfolio", on_delete=models.PROTECT, + related_name="portfolio_suborganizations", ) def __str__(self) -> str: 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 418d1464b..7cc72e8e1 100644 --- a/src/registrar/templates/django/admin/includes/contact_detail_list.html +++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html @@ -1,6 +1,7 @@ {% load i18n static %} +{% load custom_filters %} -
    +
    {% if show_formatted_name %} @@ -9,10 +10,10 @@ {% else %} None {% endif %} +
    {% endif %} -
    - {% if user.has_contact_info %} + {% if user|has_contact_info %} {# Title #} {% if user.title %} {{ user.title }} @@ -42,7 +43,7 @@ No additional contact information found.
    {% endif %} - {% if user_verification_type %} + {% if user_verification_type and not skip_additional_contact_info %} {{ user_verification_type }} {% 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 067b69c07..683f33117 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -184,7 +184,9 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly user_verification_type=original_object.creator.get_verification_type_display%} - {% include "django/admin/includes/user_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly %} + {% if not skip_additional_contact_info %} + {% include "django/admin/includes/user_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly %} + {% endif%} {% elif field.field.name == "submitter" %}
    diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index f6382758b..9d59aae42 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -8,34 +8,16 @@ {{ block.super }} {% endblock content %} -{% block after_related_objects %} -
    -

    Associated groups and suborganizations

    -
    -
    -

    Domain groups

    - -
    -
    -

    Suborganizations

    - -
    -
    -
    +{% block field_sets %} + {% for fieldset in adminform %} + {% comment %} + This is a placeholder for now. + + Disclaimer: + When extending the fieldset view - *make a new one* that extends from detail_table_fieldset. + For instance, "portfolio_fieldset.html". + 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 %} + {% endfor %} {% endblock %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index e90b3b17f..728478a51 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -159,3 +159,13 @@ def and_filter(value, arg): Usage: {{ value|and:arg }} """ return bool(value and arg) + + +@register.filter(name="has_contact_info") +def has_contact_info(user): + """Checks if the given object has the attributes: title, email, phone + and checks if at least one of those is not null.""" + if not hasattr(user, "title") or not hasattr(user, "email") or not hasattr(user, "phone"): + return False + else: + return bool(user.title or user.email or user.phone) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index a4c3e2ef4..ceb3b6e92 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -906,6 +906,7 @@ def completed_domain_request( # noqa federal_agency=None, federal_type=None, action_needed_reason=None, + portfolio=None, ): """A completed domain request.""" if not user: @@ -976,6 +977,9 @@ def completed_domain_request( # noqa if action_needed_reason: domain_request_kwargs["action_needed_reason"] = action_needed_reason + if portfolio: + domain_request_kwargs["portfolio"] = portfolio + domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 4ec3336ba..827742ef1 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -23,6 +23,7 @@ from registrar.admin import ( PublicContactAdmin, TransitionDomainAdmin, UserGroupAdmin, + PortfolioAdmin, ) from registrar.models import ( Domain, @@ -38,6 +39,8 @@ from registrar.models import ( FederalAgency, UserGroup, TransitionDomain, + Portfolio, + Suborganization, ) from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial @@ -2042,3 +2045,79 @@ class TestUserGroup(TestCase): response, "Groups are a way to bundle admin permissions so they can be easily assigned to multiple users." ) self.assertContains(response, "Show more") + + +class TestPortfolioAdmin(TestCase): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.site = AdminSite() + cls.superuser = create_superuser() + cls.admin = PortfolioAdmin(model=Portfolio, admin_site=cls.site) + cls.factory = RequestFactory() + + def setUp(self): + self.client = Client(HTTP_HOST="localhost:8080") + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser) + + def tearDown(self): + Suborganization.objects.all().delete() + DomainInformation.objects.all().delete() + DomainRequest.objects.all().delete() + Domain.objects.all().delete() + Portfolio.objects.all().delete() + + @less_console_noise_decorator + def test_created_on_display(self): + """Tests the custom created on which is a reskin of the created_at field""" + created_on = self.admin.created_on(self.portfolio) + expected_date = self.portfolio.created_at.strftime("%b %d, %Y") + self.assertEqual(created_on, expected_date) + + @less_console_noise_decorator + def test_suborganizations_display(self): + """Tests the custom suborg field which displays all related suborgs""" + Suborganization.objects.create(name="Sub1", portfolio=self.portfolio) + Suborganization.objects.create(name="Sub2", portfolio=self.portfolio) + + suborganizations = self.admin.suborganizations(self.portfolio) + self.assertIn("Sub1", suborganizations) + self.assertIn("Sub2", suborganizations) + self.assertIn('