diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5da46f110..74126ab46 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2894,6 +2894,15 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): class PortfolioAdmin(ListHeaderAdmin): + + class Meta: + """Contains meta information about this class""" + + model = models.Portfolio + fields = "__all__" + + _meta = Meta() + change_form_template = "django/admin/portfolio_change_form.html" fieldsets = [ # created_on is the created_at field, and portfolio_type is f"{organization_type} - {federal_type}" @@ -2940,16 +2949,12 @@ class PortfolioAdmin(ListHeaderAdmin): ("Senior official", {"fields": ["senior_official"]}), ] - list_display = ("organization_name", "federal_agency", "creator") + list_display = ("organization_name", "organization_type", "federal_type", "creator") search_fields = ["organization_name"] - search_help_text = "Search by organization name." + search_help_text = "Search by portfolio organization." readonly_fields = [ # This is the created_at field "created_on", - # Django admin doesn't allow methods to be directly listed in fieldsets. We can - # display the custom methods display_admins amd display_members in the admin form if - # they are readonly. - "federal_type", "domains", "domain_requests", "suborganizations", @@ -2959,16 +2964,47 @@ class PortfolioAdmin(ListHeaderAdmin): "creator", ] + analyst_readonly_fields = [ + "organization_name", + "organization_type", + ] + + def get_readonly_fields(self, request, obj=None): + """Set the read-only state on form elements. + We have 2 conditions that determine which fields are read-only: + admin user permissions and the creator's status, so + we'll use the baseline readonly_fields and extend it as needed. + """ + readonly_fields = list(self.readonly_fields) + + # Check if the creator is restricted + if obj and obj.creator.status == models.User.RESTRICTED: + # For fields like CharField, IntegerField, etc., the widget used is + # straightforward and the readonly_fields list can control their behavior + readonly_fields.extend([field.name for field in self.model._meta.fields]) + + if request.user.has_perm("registrar.full_access_permission"): + return readonly_fields + + # Return restrictive Read-only fields for analysts and + # users who might not belong to groups + readonly_fields.extend([field for field in self.analyst_readonly_fields]) + return readonly_fields + def get_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio - admin_permissions = UserPortfolioPermission.objects.filter( - portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) + admin_permissions = self.get_user_portfolio_permission_admins(obj) # Get the user objects associated with these permissions admin_users = User.objects.filter(portfolio_permissions__in=admin_permissions) return admin_users + + def get_user_portfolio_permission_admins(self, obj): + """Returns each admin on UserPortfolioPermission for a given portfolio.""" + return obj.portfolio_users.filter( + portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) def get_non_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio that do NOT have the "Admin" role @@ -2980,6 +3016,13 @@ class PortfolioAdmin(ListHeaderAdmin): non_admin_users = User.objects.filter(portfolio_permissions__in=non_admin_permissions) return non_admin_users + + def get_user_portfolio_permission_non_admins(self, obj): + """Returns each admin on UserPortfolioPermission for a given portfolio.""" + return obj.portfolio_users.exclude( + roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + def display_admins(self, obj): """Get joined users who are Admin, unpack and return an HTML block. @@ -2989,19 +3032,23 @@ class PortfolioAdmin(ListHeaderAdmin): data would display in a custom change form without extensive template customization. Will be used in the field_readonly block""" - admins = self.get_admin_users(obj) + admins = self.get_user_portfolio_permission_admins(obj) if not admins: return format_html("

No admins found.

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

No additional members found.

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

No additional members found.

+ {% endif %} +
+ {% else %} +
{{ field.contents }}
+ {% endif %} +{% endblock field_readonly%} + +{% block after_help_text %} + {% if field.field.name == "senior_official" %} +
+ + {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly %} +
+ {% elif field.field.name == "display_members" and field.contents %} +
+ Details +
+ {{ field.contents|safe }} +
+
+ {% endif %} +{% endblock after_help_text %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 8dae8a080..38b155ce2 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -20,7 +20,7 @@ When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. {% endcomment %} - {% include "django/admin/includes/detail_table_fieldset.html" with original_object=original %} + {% include "django/admin/includes/portfolio_fieldset.html" with original_object=original %} {% endfor %} {% endblock %} diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 97eb7e86c..430c3416b 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -58,7 +58,7 @@ def get_federal_and_portfolio_types_from_federal_agency_json(request): organization_type = request.GET.get("organization_type") agency = FederalAgency.objects.filter(agency=agency_name).first() if agency: - federal_type = Portfolio.get_federal_type(agency) + federal_type = agency.federal_type portfolio_type = Portfolio.get_portfolio_type(organization_type, federal_type) federal_type = BranchChoices.get_branch_label(federal_type) if federal_type else "-"