Merge pull request #2562 from cisagov/bob/2378-portfolio-senior-official

(on getgov-bob) Ticket #2378: Add readonly portfolio senior official page

src/registrar/admin.py

@@ -493,6 +493,8 @@ def changelist_view(self, request, extra_context=None):
     #     return super().change_view(request, object_id, form_url, extra_context=extra_context)
 
 
+# TODO #2571 - this should be refactored. This is shared among every class that inherits this,
+# and it breaks the senior_official field because it exists both as model "Contact" and "SeniorOfficial".
 class AdminSortFields:
     _name_sort = ["first_name", "last_name", "email"]
 
@@ -555,15 +557,16 @@ def history_view(self, request, object_id, extra_context=None):
             )
         )
 
-    def formfield_for_manytomany(self, db_field, request, **kwargs):
+    def formfield_for_manytomany(self, db_field, request, use_admin_sort_fields=True, **kwargs):
         """customize the behavior of formfields with manytomany relationships.  the customized
         behavior includes sorting of objects in lists as well as customizing helper text"""
 
         # Define a queryset. Note that in the super of this,
         # a new queryset will only be generated if one does not exist.
         # Thus, the order in which we define queryset matters.
+
         queryset = AdminSortFields.get_queryset(db_field)
-        if queryset:
+        if queryset and use_admin_sort_fields:
             kwargs["queryset"] = queryset
 
         formfield = super().formfield_for_manytomany(db_field, request, **kwargs)
@@ -574,15 +577,15 @@ def formfield_for_manytomany(self, db_field, request, **kwargs):
         )
         return formfield
 
-    def formfield_for_foreignkey(self, db_field, request, **kwargs):
+    def formfield_for_foreignkey(self, db_field, request, use_admin_sort_fields=True, **kwargs):
         """Customize the behavior of formfields with foreign key relationships. This will customize
         the behavior of selects. Customized behavior includes sorting of objects in list."""
 
         # Define a queryset. Note that in the super of this,
         # a new queryset will only be generated if one does not exist.
         # Thus, the order in which we define queryset matters.
         queryset = AdminSortFields.get_queryset(db_field)
-        if queryset:
+        if queryset and use_admin_sort_fields:
             kwargs["queryset"] = queryset
 
         return super().formfield_for_foreignkey(db_field, request, **kwargs)
@@ -1544,6 +1547,17 @@ def changelist_view(self, request, extra_context=None):
         # Get the filtered values
         return super().changelist_view(request, extra_context=extra_context)
 
+    def formfield_for_foreignkey(self, db_field, request, **kwargs):
+        """Customize the behavior of formfields with foreign key relationships. This will customize
+        the behavior of selects. Customized behavior includes sorting of objects in list."""
+        # TODO #2571
+        # Remove this check on senior_official if this underlying model changes from
+        # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields.
+        # Removing this will cause the list on django admin to return SeniorOffical
+        # objects rather than Contact objects.
+        use_sort = db_field.name != "senior_official"
+        return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs)
+
 
 class DomainRequestResource(FsmModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -2211,6 +2225,17 @@ def process_log_entry(self, log_entry):
 
         return None
 
+    def formfield_for_foreignkey(self, db_field, request, **kwargs):
+        """Customize the behavior of formfields with foreign key relationships. This will customize
+        the behavior of selects. Customized behavior includes sorting of objects in list."""
+        # TODO #2571
+        # Remove this check on senior_official if this underlying model changes from
+        # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields.
+        # Removing this will cause the list on django admin to return SeniorOffical
+        # objects rather than Contact objects.
+        use_sort = db_field.name != "senior_official"
+        return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs)
+
 
 class TransitionDomainAdmin(ListHeaderAdmin):
     """Custom transition domain admin class."""
@@ -2260,6 +2285,7 @@ def has_change_permission(self, request, obj=None):
     def formfield_for_manytomany(self, db_field, request, **kwargs):
         """customize the behavior of formfields with manytomany relationships.  the customized
         behavior includes sorting of objects in lists as well as customizing helper text"""
+
         queryset = AdminSortFields.get_queryset(db_field)
         if queryset:
             kwargs["queryset"] = queryset
@@ -2274,8 +2300,12 @@ def formfield_for_manytomany(self, db_field, request, **kwargs):
     def formfield_for_foreignkey(self, db_field, request, **kwargs):
         """Customize the behavior of formfields with foreign key relationships. This will customize
         the behavior of selects. Customized behavior includes sorting of objects in list."""
+        # Remove this check on senior_official if this underlying model changes from
+        # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields.
+        # Removing this will cause the list on django admin to return SeniorOffical
+        # objects rather than Contact objects.
         queryset = AdminSortFields.get_queryset(db_field)
-        if queryset:
+        if queryset and db_field.name != "senior_official":
             kwargs["queryset"] = queryset
         return super().formfield_for_foreignkey(db_field, request, **kwargs)
 
@@ -2848,6 +2878,7 @@ class PortfolioAdmin(ListHeaderAdmin):
     autocomplete_fields = [
         "creator",
         "federal_agency",
+        "senior_official",
     ]
 
     def change_view(self, request, object_id, form_url="", extra_context=None):

src/registrar/config/urls.py

@@ -74,6 +74,11 @@
         views.PortfolioOrganizationView.as_view(),
         name="organization",
     ),
+    path(
+        "senior-official/",
+        views.PortfolioSeniorOfficialView.as_view(),
+        name="senior-official",
+    ),
     path(
         "admin/logout/",
         RedirectView.as_view(pattern_name="logout", permanent=False),

src/registrar/forms/domain.py

@@ -358,10 +358,14 @@ class SeniorOfficialContactForm(ContactForm):
     """Form for updating senior official contacts."""
 
     JOIN = "senior_official"
+    full_name = forms.CharField(label="Full name", required=False)
 
     def __init__(self, disable_fields=False, *args, **kwargs):
         super().__init__(*args, **kwargs)
 
+        if self.instance and self.instance.id:
+            self.fields["full_name"].initial = self.instance.get_formatted_name()
+
         # Overriding bc phone not required in this form
         self.fields["phone"] = forms.IntegerField(required=False)
 
@@ -384,6 +388,12 @@ def __init__(self, disable_fields=False, *args, **kwargs):
         if disable_fields:
             DomainHelper.mass_disable_fields(fields=self.fields, disable_required=True, disable_maxlength=True)
 
+    def clean(self):
+        """Clean override to remove unused fields"""
+        cleaned_data = super().clean()
+        cleaned_data.pop("full_name", None)
+        return cleaned_data
+
     def save(self, commit=True):
         """
         Override the save() method of the BaseModelForm.

src/registrar/forms/portfolio.py

@@ -4,7 +4,7 @@
 from django import forms
 from django.core.validators import RegexValidator
 
-from ..models import DomainInformation, Portfolio
+from ..models import DomainInformation, Portfolio, SeniorOfficial
 
 logger = logging.getLogger(__name__)
 
@@ -67,3 +67,31 @@ def __init__(self, *args, **kwargs):
             self.fields[field_name].required = True
         self.fields["state_territory"].widget.attrs.pop("maxlength", None)
         self.fields["zipcode"].widget.attrs.pop("maxlength", None)
+
+
+class PortfolioSeniorOfficialForm(forms.ModelForm):
+    """
+    Form for updating the portfolio senior official.
+    This form is readonly for now.
+    """
+
+    JOIN = "senior_official"
+    full_name = forms.CharField(label="Full name", required=False)
+
+    class Meta:
+        model = SeniorOfficial
+        fields = [
+            "title",
+            "email",
+        ]
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        if self.instance and self.instance.id:
+            self.fields["full_name"].initial = self.instance.get_formatted_name()
+
+    def clean(self):
+        """Clean override to remove unused fields"""
+        cleaned_data = super().clean()
+        cleaned_data.pop("full_name", None)
+        return cleaned_data

src/registrar/management/commands/load_senior_official_table.py

@@ -35,7 +35,6 @@ def handle(self, federal_cio_csv_path, **kwargs):
 
             Note: 
                 - If the row is missing SO data - it will not be added.
-                - Given we can add the row, any blank first_name will be replaced with "-".
             """,  # noqa: W291
             prompt_title="Do you wish to load records into the SeniorOfficial table?",
         )
@@ -64,7 +63,11 @@ def handle(self, federal_cio_csv_path, **kwargs):
                 # Clean the returned data
                 for key, value in so_kwargs.items():
                     if isinstance(value, str):
-                        so_kwargs[key] = value.strip()
+                        clean_string = value.strip()
+                        if clean_string:
+                            so_kwargs[key] = clean_string
+                        else:
+                            so_kwargs[key] = None
 
                 # Handle the federal_agency record seperately (db call)
                 agency_name = row.get("Agency").strip() if row.get("Agency") else None
@@ -95,17 +98,11 @@ def handle(self, federal_cio_csv_path, **kwargs):
     def create_senior_official(self, so_kwargs):
         """Creates a senior official object from kwargs but does not add it to the DB"""
 
-        # WORKAROUND: Placeholder value for first name,
-        # as not having these makes it impossible to access through DJA.
-        old_first_name = so_kwargs["first_name"]
-        if not so_kwargs["first_name"]:
-            so_kwargs["first_name"] = "-"
-
         # Create a new SeniorOfficial object
         new_so = SeniorOfficial(**so_kwargs)
 
         # Store a variable for the console logger
-        if all([old_first_name, new_so.last_name]):
+        if all([new_so.first_name, new_so.last_name]):
             record_display = new_so
         else:
             record_display = so_kwargs

src/registrar/templates/domain_senior_official.html

@@ -4,45 +4,9 @@
 {% block title %}Senior official | {{ domain.name }} | {% endblock %}
 
 {% block domain_content %}
-  {# this is right after the messages block in the parent template #}
-  {% include "includes/form_errors.html" with form=form %}
-
-  <h1>Senior official</h1>
-
-  <p>Your senior official is a person within your organization who can
-  authorize domain requests. This person must be in a role of significant, executive responsibility within the organization. Read more about <a class="usa-link" rel="noopener noreferrer" target="_blank" href="{% public_site_url 'domains/eligibility/#you-must-have-approval-from-a-senior-official-within-your-organization' %}">who can serve as a senior official</a>.</p>
-
   {% if generic_org_type == "federal" or generic_org_type == "tribal" %}
-    <p>
-      The senior official for your organization can’t be updated here.
-      To suggest an update, email <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>.
-    </p>
+    {% include "includes/senior_official.html" with can_edit=False include_read_more_text=True %}
   {% else %}
-  {% include "includes/required_fields.html" %}
+    {% include "includes/senior_official.html" with can_edit=True include_read_more_text=True %}
   {% endif %}
-
-
-  <form class="usa-form usa-form--large" method="post" novalidate id="form-container">
-    {% csrf_token %}
-
-    {% if generic_org_type == "federal" or generic_org_type == "tribal" %}
-    {# If all fields are disabled, add SR content #}
-    <div class="usa-sr-only" aria-labelledby="id_first_name" id="sr-so-first-name">{{ form.first_name.value }}</div>
-    <div class="usa-sr-only" aria-labelledby="id_last_name" id="sr-so-last-name">{{ form.last_name.value }}</div>
-    <div class="usa-sr-only" aria-labelledby="id_title" id="sr-so-title">{{ form.title.value }}</div>
-    <div class="usa-sr-only" aria-labelledby="id_email" id="sr-so-email">{{ form.email.value }}</div>
-    {% endif %}
-
-    {% input_with_errors form.first_name %}
-
-    {% input_with_errors form.last_name %}
-
-    {% input_with_errors form.title %}
-
-    {% input_with_errors form.email %}
-
-    {% if generic_org_type != "federal" and generic_org_type != "tribal" %}
-    <button type="submit" class="usa-button">Save</button>
-    {% endif %}
-  </form>
-{% endblock %}  {# domain_content #}
+{% endblock %}

src/registrar/templates/includes/senior_official.html

@@ -0,0 +1,49 @@
+{% load static field_helpers url_helpers %}
+
+{% if can_edit %}
+    {% include "includes/form_errors.html" with form=form %}
+{% endif %}
+
+<h1>Senior Official</h1>
+
+<p>
+    Your senior official is a person within your organization who can authorize domain requests.
+    {% if include_read_more_text %} 
+    This person must be in a role of significant, executive responsibility within the organization.
+    Read more about <a class="usa-link" rel="noopener noreferrer" target="_blank" href="{% public_site_url 'domains/eligibility/#you-must-have-approval-from-a-senior-official-within-your-organization' %}">who can serve as a senior official</a>.
+    {% endif %}
+</p>
+
+{% if can_edit %}
+    {% include "includes/required_fields.html" %}
+{% else %}
+    <p>
+        The senior official for your organization can’t be updated here. 
+        To suggest an update, email <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>.
+    </p>
+{% endif %}
+
+{% if can_edit %}
+    <form class="usa-form usa-form--large" method="post" novalidate id="form-container">
+        {% csrf_token %}
+        {% input_with_errors form.first_name %}
+        {% input_with_errors form.last_name %}
+        {% input_with_errors form.title %}
+        {% input_with_errors form.email %}
+        <button type="submit" class="usa-button">Save</button>
+    </form>
+{% elif not form.full_name.value and not form.title.value and not form.email.value %}
+    <h4>No senior official was found.</h4>
+{% else %}
+    {% if form.full_name.value is not None %}
+        {% include "includes/input_read_only.html" with field=form.full_name %}
+    {% endif %}
+
+    {% if form.title.value is not None %}
+        {% include "includes/input_read_only.html" with field=form.title %}
+    {% endif %}
+
+    {% if form.email.value is not None %}
+        {% include "includes/input_read_only.html" with field=form.email %}
+    {% endif %}
+{% endif %}

src/registrar/templates/portfolio_organization_sidebar.html

@@ -13,7 +13,9 @@
       </li>
 
       <li class="usa-sidenav__item">
-        <a href="#"
+        {% url 'senior-official' as url %} 
+        <a href="{{ url }}"
+        {% if request.path == url %}class="usa-current"{% endif %}
         >
             Senior official
         </a>

src/registrar/templates/portfolio_senior_official.html

@@ -0,0 +1,24 @@
+{% extends 'portfolio_base.html' %}
+{% load static field_helpers%}
+
+{% block title %}Senior Official | {{ portfolio.name }} | {% endblock %}
+
+{% load static %}
+
+{% block portfolio_content %}
+<div class="grid-row grid-gap">
+    <div class="tablet:grid-col-3">
+        <p class="font-body-md margin-top-0 margin-bottom-2
+                    text-primary-darker text-semibold"
+        >
+            <span class="usa-sr-only"> Portfolio name:</span> {{ portfolio }}
+        </p>
+
+        {% include 'portfolio_organization_sidebar.html' %}
+    </div>
+
+    <div class="tablet:grid-col-9">
+        {% include "includes/senior_official.html" with can_edit=False %}
+    </div>
+</div>
+{% endblock %}