From 6954c1c2c1ecb29f3fe1f2fd802a5736dce34a8e Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 20 Sep 2024 12:28:42 -0600 Subject: [PATCH 01/79] Initial Scaffolding (in progress) --- src/registrar/assets/js/get-gov.js | 155 +++++++++++++ .../templates/includes/members_table.html | 216 ++++++++++++++++++ .../templates/portfolio_members.html | 20 ++ src/registrar/views/portfolio_members_json.py | 171 ++++++++++++++ src/registrar/views/portfolios.py | 39 ++++ 5 files changed, 601 insertions(+) create mode 100644 src/registrar/templates/includes/members_table.html create mode 100644 src/registrar/templates/portfolio_members.html create mode 100644 src/registrar/views/portfolio_members_json.py diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 027ef4344..918e2c451 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1853,6 +1853,144 @@ class DomainRequestsTable extends LoadTableBase { } } +class MembersTable extends LoadTableBase { + + constructor() { + super('.members__table', '.members__table-wrapper', '#members__search-field', '#members__search-field-submit', '.members__reset-search', '.members__reset-filters', '.members__no-data', '.members__no-search-results'); + } + /** + * Loads rows in the members list, as well as updates pagination around the members list + * based on the supplied attributes. + * @param {*} page - the page number of the results (starts with 1) + * @param {*} sortBy - the sort column option + * @param {*} order - the sort order {asc, desc} + * @param {*} scroll - control for the scrollToElement functionality + * @param {*} status - control for the status filter + * @param {*} searchTerm - the search term + * @param {*} portfolio - the portfolio id + */ + loadTable(page, sortBy = this.currentSortBy, order = this.currentOrder, scroll = this.scrollToTable, status = this.currentStatus, searchTerm =this.currentSearchTerm, portfolio = this.portfolioValue) { + + // fetch json of page of domais, given params + let baseUrl = document.getElementById("get_members_json_url"); + if (!baseUrl) { + return; + } + + let baseUrlValue = baseUrl.innerHTML; + if (!baseUrlValue) { + return; + } + + // fetch json of page of members, given params + let searchParams = new URLSearchParams( + { + "page": page, + "sort_by": sortBy, + "order": order, + "status": status, + "search_term": searchTerm + } + ); + if (portfolio) + searchParams.append("portfolio", portfolio) + + let url = `${baseUrlValue}?${searchParams.toString()}` + fetch(url) + .then(response => response.json()) + .then(data => { + if (data.error) { + console.error('Error in AJAX call: ' + data.error); + return; + } + + // handle the display of proper messaging in the event that no members exist in the list or search returns no results + this.updateDisplay(data, this.tableWrapper, this.noTableWrapper, this.noSearchResultsWrapper, this.currentSearchTerm); + + // identify the DOM element where the domain list will be inserted into the DOM + const domainList = document.querySelector('.members__table tbody'); + domainList.innerHTML = ''; + + data.members.forEach(domain => { + const options = { year: 'numeric', month: 'short', day: 'numeric' }; + const expirationDate = domain.expiration_date ? new Date(domain.expiration_date) : null; + const expirationDateFormatted = expirationDate ? expirationDate.toLocaleDateString('en-US', options) : ''; + const expirationDateSortValue = expirationDate ? expirationDate.getTime() : ''; + const actionUrl = domain.action_url; + const suborganization = domain.domain_info__sub_organization ? domain.domain_info__sub_organization : '⎯'; + + const row = document.createElement('tr'); + + let markupForSuborganizationRow = ''; + + if (this.portfolioValue) { + markupForSuborganizationRow = ` + + ${suborganization} + + ` + } + + row.innerHTML = ` + + ${domain.name} + + + ${expirationDateFormatted} + + + ${domain.state_display} + + + + + ${markupForSuborganizationRow} + + + + ${domain.action_label} ${domain.name} + + + `; + domainList.appendChild(row); + }); + // initialize tool tips immediately after the associated DOM elements are added + initializeTooltips(); + + // Do not scroll on first page load + if (scroll) + ScrollToElement('class', 'members'); + this.scrollToTable = true; + + // update pagination + this.updatePagination( + 'domain', + '#members-pagination', + '#members-pagination .usa-pagination__counter', + '#members', + data.page, + data.num_pages, + data.has_previous, + data.has_next, + data.total, + ); + this.currentSortBy = sortBy; + this.currentOrder = order; + this.currentSearchTerm = searchTerm; + }) + .catch(error => console.error('Error fetching members:', error)); + } +} + /** * An IIFE that listens for DOM Content to be loaded, then executes. This function @@ -1926,6 +2064,23 @@ const utcDateString = (dateString) => { }; + +/** + * An IIFE that listens for DOM Content to be loaded, then executes. This function + * initializes the domains list and associated functionality on the home page of the app. + * + */ +document.addEventListener('DOMContentLoaded', function() { + const isMembersPage = document.querySelector("#members") + if (isMembersPage){ + const membersTable = new MembersTable(); + if (membersTable.tableWrapper) { + // Initial load + membersTable.loadTable(1); + } + } +}); + /** * An IIFE that displays confirmation modal on the user profile page */ diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html new file mode 100644 index 000000000..980a30179 --- /dev/null +++ b/src/registrar/templates/includes/members_table.html @@ -0,0 +1,216 @@ +{% load static %} + +{% comment %} Stores the json endpoint in a url for easier access {% endcomment %} +{% url 'get_portfolio_members_json' as url %} + +
+
+ {% if not portfolio %} +

Members

+ {% else %} + + + {% endif %} + +
+
+ +
+
+ + {% if portfolio_members_count and portfolio_members_count > 0 %} + + + {% endif %} +
+ {% if portfolio %} + + + + + {% endif %} + + + + + +
+ diff --git a/src/registrar/templates/portfolio_members.html b/src/registrar/templates/portfolio_members.html new file mode 100644 index 000000000..f2d0b5650 --- /dev/null +++ b/src/registrar/templates/portfolio_members.html @@ -0,0 +1,20 @@ +{% extends 'portfolio_base.html' %} + +{% load static %} + +{% block title %} Members | {% endblock %} + +{% block wrapper_class %} + {{ block.super }} dashboard--grey-1 +{% endblock %} + +{% block portfolio_content %} +{% block messages %} + {% include "includes/form_messages.html" %} +{% endblock %} + +
+

Members

+ {% include "includes/members_table.html" with portfolio=portfolio portfolio_members_count=portfolio_members_count %} +
+{% endblock %} diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py new file mode 100644 index 000000000..de35d56b7 --- /dev/null +++ b/src/registrar/views/portfolio_members_json.py @@ -0,0 +1,171 @@ +from django.http import JsonResponse +from django.core.paginator import Paginator +from registrar.models import DomainRequest +from django.utils.dateformat import format +from django.contrib.auth.decorators import login_required +from django.urls import reverse +from django.db.models import Q + +from registrar.models.user_portfolio_permission import UserPortfolioPermission + + +@login_required +def get_portfolio_members_json(request): + """Given the current request, + get all members that are associated with the given portfolio""" + + member_ids = get_member_ids_from_request(request) + unfiltered_total = member_ids.count() + +# objects = apply_search(objects, request) +# objects = apply_status_filter(objects, request) +# objects = apply_sorting(objects, request) + + paginator = Paginator(member_ids, 10) + page_number = request.GET.get("page", 1) + page_obj = paginator.get_page(page_number) + members = [ + serialize_members(request, member, request.user) for member in page_obj.object_list + ] + +# return JsonResponse( +# { +# "domain_requests": domain_requests, +# "has_next": page_obj.has_next(), +# "has_previous": page_obj.has_previous(), +# "page": page_obj.number, +# "num_pages": paginator.num_pages, +# "total": paginator.count, +# "unfiltered_total": unfiltered_total, +# } +# ) + + +def get_member_ids_from_request(request): + """Given the current request, + get all members that are associated with the given portfolio""" + portfolio = request.GET.get("portfolio") + # filter_condition = Q(creator=request.user) + if portfolio: + # TODO: Permissions?? + # if request.user.is_org_user(request) and request.user.has_view_all_requests_portfolio_permission(portfolio): + # filter_condition = Q(portfolio=portfolio) + # else: + # filter_condition = Q(portfolio=portfolio, creator=request.user) + + member_ids = UserPortfolioPermission.objects.filter( + portfolio=portfolio + ).values_list("user__id", flat=True) + return member_ids + + +# def apply_search(queryset, request): +# search_term = request.GET.get("search_term") +# is_portfolio = request.GET.get("portfolio") + +# if search_term: +# search_term_lower = search_term.lower() +# new_domain_request_text = "new domain request" + +# # Check if the search term is a substring of 'New domain request' +# # If yes, we should return domain requests that do not have a +# # requested_domain (those display as New domain request in the UI) +# if search_term_lower in new_domain_request_text: +# queryset = queryset.filter( +# Q(requested_domain__name__icontains=search_term) | Q(requested_domain__isnull=True) +# ) +# elif is_portfolio: +# queryset = queryset.filter( +# Q(requested_domain__name__icontains=search_term) +# | Q(creator__first_name__icontains=search_term) +# | Q(creator__last_name__icontains=search_term) +# | Q(creator__email__icontains=search_term) +# ) +# # For non org users +# else: +# queryset = queryset.filter(Q(requested_domain__name__icontains=search_term)) +# return queryset + + +# def apply_status_filter(queryset, request): +# status_param = request.GET.get("status") +# if status_param: +# status_list = status_param.split(",") +# statuses = [status for status in status_list if status in DomainRequest.DomainRequestStatus.values] +# # Construct Q objects for statuses that can be queried through ORM +# status_query = Q() +# if statuses: +# status_query |= Q(status__in=statuses) +# # Apply the combined query +# queryset = queryset.filter(status_query) + +# return queryset + + +# def apply_sorting(queryset, request): +# sort_by = request.GET.get("sort_by", "id") # Default to 'id' +# order = request.GET.get("order", "asc") # Default to 'asc' + +# if order == "desc": +# sort_by = f"-{sort_by}" +# return queryset.order_by(sort_by) + + +def serialize_members(request, member, user): + +# ------- DELETABLE +# deletable_statuses = [ +# DomainRequest.DomainRequestStatus.STARTED, +# DomainRequest.DomainRequestStatus.WITHDRAWN, +# ] + +# # Determine if the request is deletable +# if not user.is_org_user(request): +# is_deletable = member.status in deletable_statuses +# else: +# portfolio = request.session.get("portfolio") +# is_deletable = ( +# member.status in deletable_statuses and user.has_edit_request_portfolio_permission(portfolio) +# ) and member.creator == user + + +# ------- EDIT / VIEW +# # Determine action label based on user permissions and request status +# editable_statuses = [ +# DomainRequest.DomainRequestStatus.STARTED, +# DomainRequest.DomainRequestStatus.ACTION_NEEDED, +# DomainRequest.DomainRequestStatus.WITHDRAWN, +# ] + +# if user.has_edit_request_portfolio_permission and member.creator == user: +# action_label = "Edit" if member.status in editable_statuses else "Manage" +# else: +# action_label = "View" + +# # Map the action label to corresponding URLs and icons +# action_url_map = { +# "Edit": reverse("edit-domain-request", kwargs={"id": member.id}), +# "Manage": reverse("domain-request-status", kwargs={"pk": member.id}), +# "View": "#", +# } + +# svg_icon_map = {"Edit": "edit", "Manage": "settings", "View": "visibility"} + + +# ------- INVITED +# TODO:.... + + +# ------- SERIALIZE +# return { +# "requested_domain": member.requested_domain.name if member.requested_domain else None, +# "last_submitted_date": member.last_submitted_date, +# "status": member.get_status_display(), +# "created_at": format(member.created_at, "c"), # Serialize to ISO 8601 +# "creator": member.creator.email, +# "id": member.id, +# "is_deletable": is_deletable, +# "action_url": action_url_map.get(action_label), +# "action_label": action_label, +# "svg_icon": svg_icon_map.get(action_label), +# } diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 885dca636..4037a72b8 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -41,6 +41,45 @@ class PortfolioDomainRequestsView(PortfolioDomainRequestsPermissionView, View): return render(request, "portfolio_requests.html") +class PortfolioMembersView(PortfolioMembersPermissionView, View): + + template_name = "portfolio_members.html" + + def get(self, request): + """Add additional context data to the template.""" + # We can override the base class. This view only needs this item. + context = {} + portfolio = self.request.session.get("portfolio") + if portfolio: + + # # ------ Gets admin members + # admin_ids = UserPortfolioPermission.objects.filter( + # portfolio=portfolio, + # roles__overlap=[ + # UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + # ], + # ).values_list("user__id", flat=True) + + + # # ------ Gets non-admin members + # # Filter UserPortfolioPermission objects related to the portfolio that do NOT have the "Admin" role + # non_admin_permissions = UserPortfolioPermission.objects.filter(portfolio=obj).exclude( + # roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + # ) + # # Get the user objects associated with these permissions + # non_admin_users = User.objects.filter(portfolio_permissions__in=non_admin_permissions) + + + # ------- Gets all members + member_ids = UserPortfolioPermission.objects.filter( + portfolio=portfolio + ).values_list("user__id", flat=True) + + all_members = User.objects.filter(id__in=member_ids) + context["portfolio_members"] = all_members + context["portfolio_members_count"] = all_members.count() + return render(request, "portfolio_members.html") + class PortfolioNoDomainsView(NoPortfolioDomainsPermissionView, View): """Some users have access to the underlying portfolio, but not any domains. This is a custom view which explains that to the user - and denotes who to contact. From 5d2fec86afed8bb4b11eff1bfac4f618b47e6190 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 20 Sep 2024 14:32:12 -0600 Subject: [PATCH 02/79] Expose portfolios to analysts --- src/registrar/admin.py | 54 ------------------- .../migrations/0128_create_groups_v17.py | 37 +++++++++++++ src/registrar/models/user_group.py | 15 ++++++ 3 files changed, 52 insertions(+), 54 deletions(-) create mode 100644 src/registrar/migrations/0128_create_groups_v17.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1c8551d4e..5da46f110 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1543,33 +1543,6 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): change_form_template = "django/admin/domain_information_change_form.html" - superuser_only_fields = [ - "portfolio", - "sub_organization", - ] - - # DEVELOPER's NOTE: - # Normally, to exclude a field from an Admin form, we could simply utilize - # Django's "exclude" feature. However, it causes a "missing key" error if we - # go that route for this particular form. The error gets thrown by our - # custom fieldset.html code and is due to the fact that "exclude" removes - # fields from base_fields but not fieldsets. Rather than reworking our - # custom frontend, it seems more straightforward (and easier to read) to simply - # modify the fieldsets list so that it excludes any fields we want to remove - # based on permissions (eg. superuser_only_fields) or other conditions. - def get_fieldsets(self, request, obj=None): - fieldsets = self.fieldsets - - # Create a modified version of fieldsets to exclude certain fields - if not request.user.has_perm("registrar.full_access_permission"): - modified_fieldsets = [] - for name, data in fieldsets: - fields = data.get("fields", []) - fields = tuple(field for field in fields if field not in DomainInformationAdmin.superuser_only_fields) - modified_fieldsets.append((name, {**data, "fields": fields})) - return modified_fieldsets - return fieldsets - def get_readonly_fields(self, request, obj=None): """Set the read-only state on form elements. We have 1 conditions that determine which fields are read-only: @@ -1865,33 +1838,6 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] filter_horizontal = ("current_websites", "alternative_domains", "other_contacts") - superuser_only_fields = [ - "portfolio", - "sub_organization", - ] - - # DEVELOPER's NOTE: - # Normally, to exclude a field from an Admin form, we could simply utilize - # Django's "exclude" feature. However, it causes a "missing key" error if we - # go that route for this particular form. The error gets thrown by our - # custom fieldset.html code and is due to the fact that "exclude" removes - # fields from base_fields but not fieldsets. Rather than reworking our - # custom frontend, it seems more straightforward (and easier to read) to simply - # modify the fieldsets list so that it excludes any fields we want to remove - # based on permissions (eg. superuser_only_fields) or other conditions. - def get_fieldsets(self, request, obj=None): - fieldsets = super().get_fieldsets(request, obj) - - # Create a modified version of fieldsets to exclude certain fields - if not request.user.has_perm("registrar.full_access_permission"): - modified_fieldsets = [] - for name, data in fieldsets: - fields = data.get("fields", []) - fields = tuple(field for field in fields if field not in self.superuser_only_fields) - modified_fieldsets.append((name, {**data, "fields": fields})) - return modified_fieldsets - return fieldsets - # Table ordering # NOTE: This impacts the select2 dropdowns (combobox) # Currentl, there's only one for requests on DomainInfo diff --git a/src/registrar/migrations/0128_create_groups_v17.py b/src/registrar/migrations/0128_create_groups_v17.py new file mode 100644 index 000000000..7ac392da7 --- /dev/null +++ b/src/registrar/migrations/0128_create_groups_v17.py @@ -0,0 +1,37 @@ +# This migration creates the create_full_access_group and create_cisa_analyst_group groups +# It is dependent on 0079 (which populates federal agencies) +# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS +# in the user_group model then: +# [NOT RECOMMENDED] +# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions +# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups +# step 3: fake run the latest migration in the migrations list +# [RECOMMENDED] +# Alternatively: +# step 1: duplicate the migration that loads data +# step 2: docker-compose exec app ./manage.py migrate + +from django.db import migrations +from registrar.models import UserGroup +from typing import Any + + +# For linting: RunPython expects a function reference, +# so let's give it one +def create_groups(apps, schema_editor) -> Any: + UserGroup.create_cisa_analyst_group(apps, schema_editor) + UserGroup.create_full_access_group(apps, schema_editor) + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0127_remove_domaininformation_submitter_and_more"), + ] + + operations = [ + migrations.RunPython( + create_groups, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ] diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index 76657fe29..e6bcaa4f4 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -66,6 +66,21 @@ class UserGroup(Group): "model": "federalagency", "permissions": ["add_federalagency", "change_federalagency", "delete_federalagency"], }, + { + "app_label": "registrar", + "model": "portfolio", + "permissions": ["add_portfolio", "change_portfolio", "delete_portfolio"], + }, + { + "app_label": "registrar", + "model": "suborganization", + "permissions": ["add_suborganization", "change_suborganization", "delete_suborganization"], + }, + { + "app_label": "registrar", + "model": "userportfoliopermission", + "permissions": ["add_userportfoliopermission", "change_userportfoliopermission", "delete_userportfoliopermission"], + }, ] # Avoid error: You can't execute queries until the end From 64b62f151c263012290f38080094a1ca7d2414d8 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sat, 21 Sep 2024 15:56:30 -0600 Subject: [PATCH 03/79] Initial scaffold for members page completed --- src/registrar/assets/js/get-gov.js | 98 +++++++------------ src/registrar/config/urls.py | 12 +++ .../templates/includes/header_extended.html | 2 +- .../templates/includes/members_table.html | 9 +- src/registrar/views/portfolio_members_json.py | 76 +++++++++----- src/registrar/views/portfolios.py | 1 + src/registrar/views/utility/mixins.py | 2 +- 7 files changed, 109 insertions(+), 91 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 918e2c451..3b0da926c 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1871,6 +1871,21 @@ class MembersTable extends LoadTableBase { */ loadTable(page, sortBy = this.currentSortBy, order = this.currentOrder, scroll = this.scrollToTable, status = this.currentStatus, searchTerm =this.currentSearchTerm, portfolio = this.portfolioValue) { + // --------- SEARCH + // let searchParams = new URLSearchParams( + // { + // "page": page, + // "sort_by": sortBy, + // "order": order, + // "status": status, + // "search_term": searchTerm + // } + // ); + // if (portfolio) + // searchParams.append("portfolio", portfolio) + + + // --------- FETCH DATA // fetch json of page of domais, given params let baseUrl = document.getElementById("get_members_json_url"); if (!baseUrl) { @@ -1882,19 +1897,6 @@ class MembersTable extends LoadTableBase { return; } - // fetch json of page of members, given params - let searchParams = new URLSearchParams( - { - "page": page, - "sort_by": sortBy, - "order": order, - "status": status, - "search_term": searchTerm - } - ); - if (portfolio) - searchParams.append("portfolio", portfolio) - let url = `${baseUrlValue}?${searchParams.toString()}` fetch(url) .then(response => response.json()) @@ -1908,60 +1910,34 @@ class MembersTable extends LoadTableBase { this.updateDisplay(data, this.tableWrapper, this.noTableWrapper, this.noSearchResultsWrapper, this.currentSearchTerm); // identify the DOM element where the domain list will be inserted into the DOM - const domainList = document.querySelector('.members__table tbody'); - domainList.innerHTML = ''; - - data.members.forEach(domain => { - const options = { year: 'numeric', month: 'short', day: 'numeric' }; - const expirationDate = domain.expiration_date ? new Date(domain.expiration_date) : null; - const expirationDateFormatted = expirationDate ? expirationDate.toLocaleDateString('en-US', options) : ''; - const expirationDateSortValue = expirationDate ? expirationDate.getTime() : ''; - const actionUrl = domain.action_url; - const suborganization = domain.domain_info__sub_organization ? domain.domain_info__sub_organization : '⎯'; + const memberList = document.querySelector('.members__table tbody'); + memberList.innerHTML = ''; + data.members.forEach(member => { + // const actionUrl = domain.action_url; + const row = document.createElement('tr'); - let markupForSuborganizationRow = ''; - - if (this.portfolioValue) { - markupForSuborganizationRow = ` - - ${suborganization} - - ` - } - row.innerHTML = ` - - ${domain.name} + + TEMP -- member ID - - ${expirationDateFormatted} - - - ${domain.state_display} - - - - - ${markupForSuborganizationRow} - - - - ${domain.action_label} ${domain.name} - + + ${member.id} `; - domainList.appendChild(row); + + // + // + // + // ${domain.action_label} ${domain.name} + // + // + // `; + + memberList.appendChild(row); }); // initialize tool tips immediately after the associated DOM elements are added initializeTooltips(); @@ -1973,7 +1949,7 @@ class MembersTable extends LoadTableBase { // update pagination this.updatePagination( - 'domain', + 'member', '#members-pagination', '#members-pagination .usa-pagination__counter', '#members', diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 9b9ed569e..9035557d4 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -73,6 +73,17 @@ urlpatterns = [ views.PortfolioNoDomainsView.as_view(), name="no-portfolio-domains", ), + + path( + "members/", + views.PortfolioMembersView.as_view(), + name="members", + ), + # path( + # "no-organization-members/", + # views.PortfolioNoMembersView.as_view(), + # name="no-portfolio-members", + # ), path( "requests/", views.PortfolioDomainRequestsView.as_view(), @@ -264,6 +275,7 @@ urlpatterns = [ ), path("get-domains-json/", get_domains_json, name="get_domains_json"), path("get-domain-requests-json/", get_domain_requests_json, name="get_domain_requests_json"), + path("get-portfolio-members-json/", get_domains_json, name="get_portfolio_members_json"), ] # Djangooidc strips out context data from that context, so we define a custom error diff --git a/src/registrar/templates/includes/header_extended.html b/src/registrar/templates/includes/header_extended.html index 5a6a3fa3f..0eeec924c 100644 --- a/src/registrar/templates/includes/header_extended.html +++ b/src/registrar/templates/includes/header_extended.html @@ -93,7 +93,7 @@ {% if has_organization_members_flag %}
  • - + Members
    • diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index 980a30179..95e13aebf 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -12,7 +12,12 @@ {% endif %} -
    + + {% if portfolio_members_count and portfolio_members_count > 0 %} - +
    {% if portfolio_members_count and portfolio_members_count > 0 %} -
    + {% elif field.field.name == "senior_official" %} + {% if original_object.senior_official %} +
    {{ field.contents }}
    + {% else %} + {% url "admin:registrar_seniorofficial_add" as url %} +
    + No senior official has been found. Create one now. +
    + {% endif %} {% else %}
    {{ field.contents }}
    {% endif %} @@ -30,7 +39,7 @@ {% if field.field.name == "senior_official" %}
    - {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly %} + {% include "django/admin/includes/contact_detail_list.html" with user=original_object.senior_official no_title_top_padding=field.is_readonly hide_no_contact_info_message=True %}
    {% elif field.field.name == "display_admins" %} {% if admins|length > 0 %} @@ -40,5 +49,13 @@ {% if members|length > 0 %} {% include "django/admin/includes/portfolio_members_table.html" with members=members %} {% endif %} + {% elif field.field.name == "domains" %} + {% if domains|length > 0 %} + {% include "django/admin/includes/portfolio_domains_table.html" with domains=domains %} + {% endif %} + {% elif field.field.name == "domain_requests" %} + {% if domain_requests|length > 0 %} + {% include "django/admin/includes/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} + {% endif %} {% endif %} {% endblock after_help_text %} \ No newline at end of file From 75b34dab3b7ee1e333384dcb274772dda44d425d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 10:55:20 -0600 Subject: [PATCH 36/79] Cleanup --- src/registrar/admin.py | 9 --- .../models/user_portfolio_permission.py | 14 ----- src/registrar/registrar_middleware.py | 2 + .../django/admin/includes/details_button.html | 9 +++ .../portfolio/portfolio_admins_table.html | 48 ++++++++++++++++ .../portfolio_domain_requests_table.html | 26 +++++++++ .../portfolio/portfolio_domains_table.html | 30 ++++++++++ .../{ => portfolio}/portfolio_fieldset.html | 8 +-- .../portfolio/portfolio_members_table.html | 55 ++++++++++++++++++ .../includes/portfolio_admins_table.html | 50 ---------------- .../portfolio_domain_requests_table.html | 28 --------- .../includes/portfolio_domains_table.html | 32 ----------- .../includes/portfolio_members_table.html | 57 ------------------- .../user_portfolio_permission_fieldset.html | 26 --------- .../django/admin/portfolio_change_form.html | 2 +- 15 files changed, 175 insertions(+), 221 deletions(-) create mode 100644 src/registrar/templates/django/admin/includes/details_button.html create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html rename src/registrar/templates/django/admin/includes/{ => portfolio}/portfolio_fieldset.html (82%) create mode 100644 src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_admins_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_domains_table.html delete mode 100644 src/registrar/templates/django/admin/includes/portfolio_members_table.html delete mode 100644 src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2c1bd5a5a..f2e9a65e8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1245,7 +1245,6 @@ class UserDomainRoleResource(resources.ModelResource): class UserPortfolioPermissionAdmin(ListHeaderAdmin): form = UserPortfolioPermissionsForm - change_form_template = "django/admin/user_portfolio_permission_change_form.html" class Meta: """Contains meta information about this class""" @@ -1263,14 +1262,6 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): autocomplete_fields = ["user", "portfolio"] - def change_view(self, request, object_id, form_url="", extra_context=None): - """Adds a readonly display for roles and permissions""" - obj = self.get_object(request, object_id) - extra_context = extra_context or {} - extra_context["display_roles"] = ", ".join(obj.get_readable_roles()) - extra_context["display_permissions"] = ", ".join(obj.get_readable_additional_permissions()) - return super().change_view(request, object_id, form_url, extra_context) - class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom user domain role admin class.""" diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 112d93009..5479b6f3d 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -82,20 +82,6 @@ class UserPortfolioPermission(TimeStampedModel): ) return f"{self.user}" f" " if self.roles else "" - def get_readable_roles(self): - """Returns a list of labels of each role in self.roles""" - role_labels = [] - for role in self.roles: - role_labels.append(UserPortfolioRoleChoices.get_user_portfolio_role_label(role)) - return role_labels - - def get_readable_additional_permissions(self): - """Returns a list of labels of each additional_permission in self.additional_permissions""" - perm_labels = [] - for perm in self.additional_permissions: - perm_labels.append(UserPortfolioPermissionChoices.get_user_portfolio_permission_label(perm)) - return perm_labels - def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. diff --git a/src/registrar/registrar_middleware.py b/src/registrar/registrar_middleware.py index 6346ed4fd..2ccea9321 100644 --- a/src/registrar/registrar_middleware.py +++ b/src/registrar/registrar_middleware.py @@ -49,6 +49,8 @@ class CheckUserProfileMiddleware: self.setup_page, self.logout_page, "/admin", + # These are here as there is a bug with this middleware that breaks djangos built in debug console. + # The debug console uses this directory, but since this overrides that, it throws errors. "/__debug__", ] self.other_excluded_pages = [ diff --git a/src/registrar/templates/django/admin/includes/details_button.html b/src/registrar/templates/django/admin/includes/details_button.html new file mode 100644 index 000000000..9ae039b04 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/details_button.html @@ -0,0 +1,9 @@ + +{% comment %} This view provides a detail button that can be used to show/hide content {% endcomment %} +
    + Details +
    + {% block detail_content %} + {% endblock detail_content%} +
    +
    \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html new file mode 100644 index 000000000..4ea9225da --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html @@ -0,0 +1,48 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + + + {% for admin in admins %} + {% url 'admin:registrar_userportfoliopermission_change' admin.pk as url %} + + + + + + + + {% endfor %} + +
    NameTitleEmailPhone
    {{ admin.user.get_formatted_name}}{{ admin.user.title }} + {% if admin.user.email %} + {{ admin.user.email }} + {% else %} + None + {% endif %} + {{ admin.user.phone }} + {% if admin.user.email %} + + + {% endif %} +
    +{% endblock detail_content %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html new file mode 100644 index 000000000..5086721f7 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html @@ -0,0 +1,26 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + {% for domain_request in domain_requests %} + {% url 'admin:registrar_domainrequest_change' domain_request.pk as url %} + + + {% if domain_request.get_status_display %} + + {% else %} + + {% endif %} + + {% endfor %} + +
    NameStatus
    {{ domain_request }}{{ domain_request.get_status_display }}None
    +{% endblock detail_content %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html new file mode 100644 index 000000000..8b2aa018c --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html @@ -0,0 +1,30 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + {% for domain_info in domains %} + {% if domain_info.domain %} + {% with domain=domain_info.domain %} + {% url 'admin:registrar_domain_change' domain.pk as url %} + + + {% if domain and domain.get_state_display %} + + {% else %} + + {% endif %} + + {% endwith %} + {% endif %} + {% endfor %} + +
    NameState
    {{ domain }}{{ domain.get_state_display }}None
    +{% endblock detail_content%} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html similarity index 82% rename from src/registrar/templates/django/admin/includes/portfolio_fieldset.html rename to src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html index c63964d80..26c84800f 100644 --- a/src/registrar/templates/django/admin/includes/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html @@ -43,19 +43,19 @@ {% elif field.field.name == "display_admins" %} {% if admins|length > 0 %} - {% include "django/admin/includes/portfolio_admins_table.html" with admins=admins %} + {% include "django/admin/includes/portfolio/portfolio_admins_table.html" with admins=admins %} {% endif %} {% elif field.field.name == "display_members" %} {% if members|length > 0 %} - {% include "django/admin/includes/portfolio_members_table.html" with members=members %} + {% include "django/admin/includes/portfolio/portfolio_members_table.html" with members=members %} {% endif %} {% elif field.field.name == "domains" %} {% if domains|length > 0 %} - {% include "django/admin/includes/portfolio_domains_table.html" with domains=domains %} + {% include "django/admin/includes/portfolio/portfolio_domains_table.html" with domains=domains %} {% endif %} {% elif field.field.name == "domain_requests" %} {% if domain_requests|length > 0 %} - {% include "django/admin/includes/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} + {% include "django/admin/includes/portfolio/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} {% endif %} {% endif %} {% endblock after_help_text %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html new file mode 100644 index 000000000..2f3389032 --- /dev/null +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html @@ -0,0 +1,55 @@ +{% extends "django/admin/includes/details_button.html" %} +{% load custom_filters %} +{% load static url_helpers %} + +{% block detail_content %} + + + + + + + + + + + + {% for member in members %} + {% url 'admin:registrar_userportfoliopermission_change' member.pk as url %} + + + + + + + + + {% endfor %} + +
    NameTitleEmailPhoneRoles
    {{ member.user.get_formatted_name}}{{ member.user.title }} + {% if member.user.email %} + {{ member.user.email }} + {% else %} + None + {% endif %} + {{ member.user.phone }} + {% for role in member.user|portfolio_role_summary:original %} + {{ role }} + {% endfor %} + + {% if member.user.email %} + + + {% endif %} +
    +{% endblock %} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_admins_table.html b/src/registrar/templates/django/admin/includes/portfolio_admins_table.html deleted file mode 100644 index 84137cb3d..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_admins_table.html +++ /dev/null @@ -1,50 +0,0 @@ -{% load static url_helpers %} - -
    - Details -
    - - - - - - - - - - - {% for admin in admins %} - {% url 'admin:registrar_userportfoliopermission_change' admin.pk as url %} - - - - - - - - {% endfor %} - -
    NameTitleEmailPhone
    {{ admin.user.get_formatted_name}}{{ admin.user.title }} - {% if admin.user.email %} - {{ admin.user.email }} - {% else %} - None - {% endif %} - {{ admin.user.phone }} - {% if admin.user.email %} - - - {% endif %} -
    -
    -
    \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html deleted file mode 100644 index 2887c2179..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_domain_requests_table.html +++ /dev/null @@ -1,28 +0,0 @@ -{% load static url_helpers %} - -
    - Details -
    - - - - - - - - - {% for domain_request in domain_requests %} - {% url 'admin:registrar_domainrequest_change' domain_request.pk as url %} - - - {% if domain_request.get_status_display %} - - {% else %} - - {% endif %} - - {% endfor %} - -
    NameStatus
    {{ domain_request }}{{ domain_request.get_status_display }}None
    -
    -
    \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio_domains_table.html deleted file mode 100644 index 8d958c8e8..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_domains_table.html +++ /dev/null @@ -1,32 +0,0 @@ -{% load static url_helpers %} - -
    - Details -
    - - - - - - - - - {% for domain_info in domains %} - {% if domain_info.domain %} - {% with domain=domain_info.domain %} - {% url 'admin:registrar_domain_change' domain.pk as url %} - - - {% if domain and domain.get_state_display %} - - {% else %} - - {% endif %} - - {% endwith %} - {% endif %} - {% endfor %} - -
    NameState
    {{ domain }}{{ domain.get_state_display }}None
    -
    -
    \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio_members_table.html deleted file mode 100644 index 3df12f06f..000000000 --- a/src/registrar/templates/django/admin/includes/portfolio_members_table.html +++ /dev/null @@ -1,57 +0,0 @@ -{% load custom_filters %} -{% load static url_helpers %} - -
    - Details -
    - - - - - - - - - - - - {% for member in members %} - {% url 'admin:registrar_userportfoliopermission_change' member.pk as url %} - - - - - - - - - {% endfor %} - -
    NameTitleEmailPhoneRoles
    {{ member.user.get_formatted_name}}{{ member.user.title }} - {% if member.user.email %} - {{ member.user.email }} - {% else %} - None - {% endif %} - {{ member.user.phone }} - {% for role in member.user|portfolio_role_summary:original %} - {{ role }} - {% endfor %} - - {% if member.user.email %} - - - {% endif %} -
    -
    -
    \ No newline at end of file diff --git a/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html b/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html deleted file mode 100644 index a7ffc889c..000000000 --- a/src/registrar/templates/django/admin/includes/user_portfolio_permission_fieldset.html +++ /dev/null @@ -1,26 +0,0 @@ -{% extends "django/admin/includes/detail_table_fieldset.html" %} -{% load custom_filters %} -{% load static url_helpers %} - - -{% block field_readonly %} - {% if field.field.name == "roles" %} -
    - {% if display_roles %} - {{ display_roles }} - {% else %} - No roles found. - {% endif %} -
    - {% elif field.field.name == "additional_permissions" %} -
    - {% if display_permissions %} - {{ display_permissions }} - {% else %} - No additional permissions found. - {% endif %} -
    - {% else %} -
    {{ field.contents }}
    - {% endif %} -{% endblock field_readonly%} \ No newline at end of file diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 38b155ce2..0a87f8e49 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -20,7 +20,7 @@ When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. {% endcomment %} - {% include "django/admin/includes/portfolio_fieldset.html" with original_object=original %} + {% include "django/admin/includes/portfolio/portfolio_fieldset.html" with original_object=original %} {% endfor %} {% endblock %} From 237d06623329129caf87789683d4ddc2620175e5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:05:58 -0600 Subject: [PATCH 37/79] Fix some tests + fix add view --- src/registrar/assets/js/get-gov-admin.js | 10 +++++++--- src/registrar/tests/test_admin.py | 13 ++----------- src/registrar/tests/test_api.py | 1 - 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index fd2d6330e..7271cf7b8 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -891,14 +891,18 @@ function initializeWidgetOnList(list, parentId) { }); function handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType) { - if (organizationType && organizationNameContainer && federalType) { + if (organizationType && organizationNameContainer) { let selectedValue = organizationType.value; if (selectedValue === "federal") { hideElement(organizationNameContainer); - showElement(federalType); + if (federalType) { + showElement(federalType); + } } else { showElement(organizationNameContainer); - hideElement(federalType); + if (federalType) { + hideElement(federalType); + } } } } diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index daf5e7086..e125baf60 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2097,17 +2097,8 @@ class TestPortfolioAdmin(TestCase): ) display_admins = self.admin.display_admins(self.portfolio) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_1.pk]) - self.assertIn( - f'Gerald Meoward meaoward@gov.gov', - display_admins, - ) - self.assertIn("Captain", display_admins) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_2.pk]) - self.assertIn(f'Arnold Poopy poopy@gov.gov', display_admins) - self.assertIn("Major", display_admins) + url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={self.portfolio.id}" + self.assertIn(f'2 administrators', display_admins) display_members_summary = self.admin.display_members_summary(self.portfolio) diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index 2597e65c2..7a432bcb8 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -100,7 +100,6 @@ class GetFederalPortfolioTypeJsonTest(TestCase): self.assertEqual(response.status_code, 200) data = response.json() self.assertEqual(data["federal_type"], "Judicial") - self.assertEqual(data["portfolio_type"], "Federal - Judicial") @less_console_noise_decorator def test_get_federal_and_portfolio_types_json_authenticated_regularuser(self): From e1ed5a5ed5d59083d8f20e6240f39c7a25fa856c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:36:33 -0600 Subject: [PATCH 38/79] linting + fix tests --- src/registrar/admin.py | 53 ++++++++++--------- src/registrar/models/portfolio.py | 1 - src/registrar/models/user_group.py | 6 ++- .../django/admin/includes/details_button.html | 2 +- .../portfolio_domain_requests_table.html | 2 +- .../portfolio/portfolio_domains_table.html | 2 +- .../portfolio/portfolio_fieldset.html | 2 +- .../portfolio/portfolio_members_table.html | 2 +- .../django/admin/portfolio_change_form.html | 7 --- src/registrar/templatetags/custom_filters.py | 2 +- src/registrar/tests/test_admin.py | 30 ++--------- src/registrar/tests/test_migrations.py | 5 +- 12 files changed, 47 insertions(+), 67 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f2e9a65e8..49cf08935 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -5,7 +5,6 @@ import json from django.template.loader import get_template from django import forms from django.db.models import Value, CharField, Q -from django.template.loader import render_to_string from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect from django.conf import settings @@ -2981,28 +2980,6 @@ class PortfolioAdmin(ListHeaderAdmin): "organization_name", ] - def get_readonly_fields(self, request, obj=None): - """Set the read-only state on form elements. - We have 2 conditions that determine which fields are read-only: - admin user permissions and the creator's status, so - we'll use the baseline readonly_fields and extend it as needed. - """ - readonly_fields = list(self.readonly_fields) - - # Check if the creator is restricted - if obj and obj.creator.status == models.User.RESTRICTED: - # For fields like CharField, IntegerField, etc., the widget used is - # straightforward and the readonly_fields list can control their behavior - readonly_fields.extend([field.name for field in self.model._meta.fields]) - - if request.user.has_perm("registrar.full_access_permission"): - return readonly_fields - - # Return restrictive Read-only fields for analysts and - # users who might not belong to groups - readonly_fields.extend([field for field in self.analyst_readonly_fields]) - return readonly_fields - def get_admin_users(self, obj): # Filter UserPortfolioPermission objects related to the portfolio admin_permissions = self.get_user_portfolio_permission_admins(obj) @@ -3015,7 +2992,9 @@ class PortfolioAdmin(ListHeaderAdmin): def get_user_portfolio_permission_admins(self, obj): """Returns each admin on UserPortfolioPermission for a given portfolio.""" if obj: - return obj.portfolio_users.filter(portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]) + return obj.portfolio_users.filter( + portfolio=obj, roles__contains=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) else: return [] @@ -3177,6 +3156,28 @@ class PortfolioAdmin(ListHeaderAdmin): return self.add_fieldsets return super().get_fieldsets(request, obj) + def get_readonly_fields(self, request, obj=None): + """Set the read-only state on form elements. + We have 2 conditions that determine which fields are read-only: + admin user permissions and the creator's status, so + we'll use the baseline readonly_fields and extend it as needed. + """ + readonly_fields = list(self.readonly_fields) + + # Check if the creator is restricted + if obj and obj.creator.status == models.User.RESTRICTED: + # For fields like CharField, IntegerField, etc., the widget used is + # straightforward and the readonly_fields list can control their behavior + readonly_fields.extend([field.name for field in self.model._meta.fields]) + + if request.user.has_perm("registrar.full_access_permission"): + return readonly_fields + + # Return restrictive Read-only fields for analysts and + # users who might not belong to groups + readonly_fields.extend([field for field in self.analyst_readonly_fields]) + return readonly_fields + def change_view(self, request, object_id, form_url="", extra_context=None): """Add related suborganizations and domain groups. Add the summary for the portfolio members field (list of members that link to change_forms).""" @@ -3184,7 +3185,7 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True - # We repeat these calls twice. + # We repeat these calls twice. extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) extra_context["domains"] = obj.get_domains() @@ -3205,7 +3206,7 @@ class PortfolioAdmin(ListHeaderAdmin): is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL if is_federal and obj.organization_name is None: obj.organization_name = obj.federal_agency.agency - + # Remove this line when senior_official is no longer readonly in /admin. if obj.federal_agency and obj.federal_agency.so_federal_agency.exists(): obj.senior_official = obj.federal_agency.so_federal_agency.first() diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 55ca64da5..9acec8c64 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -2,7 +2,6 @@ from django.db import models from registrar.models.domain_request import DomainRequest from registrar.models.federal_agency import FederalAgency -from registrar.utility.constants import BranchChoices from .utility.time_stamped_model import TimeStampedModel diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index e6bcaa4f4..41ae67c06 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -79,7 +79,11 @@ class UserGroup(Group): { "app_label": "registrar", "model": "userportfoliopermission", - "permissions": ["add_userportfoliopermission", "change_userportfoliopermission", "delete_userportfoliopermission"], + "permissions": [ + "add_userportfoliopermission", + "change_userportfoliopermission", + "delete_userportfoliopermission", + ], }, ] diff --git a/src/registrar/templates/django/admin/includes/details_button.html b/src/registrar/templates/django/admin/includes/details_button.html index 9ae039b04..73748f170 100644 --- a/src/registrar/templates/django/admin/includes/details_button.html +++ b/src/registrar/templates/django/admin/includes/details_button.html @@ -6,4 +6,4 @@ {% block detail_content %} {% endblock detail_content%} - \ No newline at end of file + diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html index 5086721f7..46303efce 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domain_requests_table.html @@ -23,4 +23,4 @@ {% endfor %} -{% endblock detail_content %} \ No newline at end of file +{% endblock detail_content %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html index 8b2aa018c..56621b769 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_domains_table.html @@ -27,4 +27,4 @@ {% endfor %} -{% endblock detail_content%} \ No newline at end of file +{% endblock detail_content%} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html index 26c84800f..c5c6f510a 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html @@ -58,4 +58,4 @@ {% include "django/admin/includes/portfolio/portfolio_domain_requests_table.html" with domain_requests=domain_requests %} {% endif %} {% endif %} -{% endblock after_help_text %} \ No newline at end of file +{% endblock after_help_text %} diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html index 2f3389032..136fe3a5a 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_members_table.html @@ -52,4 +52,4 @@ {% endfor %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 0a87f8e49..fec1538d9 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -13,13 +13,6 @@ {% block field_sets %} {% for fieldset in adminform %} - {% comment %} - This is a placeholder for now. - - Disclaimer: - When extending the fieldset view consider whether you need to make a new one that extends from detail_table_fieldset. - detail_table_fieldset is used on multiple admin pages, so a change there can have unintended consequences. - {% endcomment %} {% include "django/admin/includes/portfolio/portfolio_fieldset.html" with original_object=original %} {% endfor %} {% endblock %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index de9b7bfa1..0bde40bb9 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -247,4 +247,4 @@ def portfolio_role_summary(user, portfolio): if user and portfolio: return user.portfolio_role_summary(portfolio) else: - return [] \ No newline at end of file + return [] diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e125baf60..cdc3c97de 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2051,7 +2051,7 @@ class TestPortfolioAdmin(TestCase): email="meaoward@gov.gov", ) - perm_1 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_1, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) @@ -2063,7 +2063,7 @@ class TestPortfolioAdmin(TestCase): email="poopy@gov.gov", ) - perm_2 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_2, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) @@ -2075,7 +2075,7 @@ class TestPortfolioAdmin(TestCase): email="madmax@gov.gov", ) - perm_3 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_3, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] ) @@ -2087,7 +2087,7 @@ class TestPortfolioAdmin(TestCase): email="thematrix@gov.gov", ) - perm_4 = UserPortfolioPermission.objects.all().create( + UserPortfolioPermission.objects.all().create( user=admin_user_4, portfolio=self.portfolio, additional_permissions=[ @@ -2100,28 +2100,8 @@ class TestPortfolioAdmin(TestCase): url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={self.portfolio.id}" self.assertIn(f'2 administrators', display_admins) - display_members_summary = self.admin.display_members_summary(self.portfolio) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_3.pk]) - self.assertIn( - f'Mad Max madmax@gov.gov', - display_members_summary, - ) - - url = reverse("admin:registrar_userportfoliopermission_change", args=[perm_4.pk]) - self.assertIn( - f'Agent Smith thematrix@gov.gov', - display_members_summary, - ) - display_members = self.admin.display_members(self.portfolio) - - self.assertIn("Mad Max", display_members) - self.assertIn("Member", display_members) - self.assertIn("Road warrior", display_members) - self.assertIn("Agent Smith", display_members) - self.assertIn("Domain requestor", display_members) - self.assertIn("Program", display_members) + self.assertIn(f'2 members', display_members) class TestTransferUser(WebTest): diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index 5dde4831c..d646a03de 100644 --- a/src/registrar/tests/test_migrations.py +++ b/src/registrar/tests/test_migrations.py @@ -50,7 +50,9 @@ class TestGroups(TestCase): "change_user", "delete_userdomainrole", "view_userdomainrole", - "view_userportfoliopermission", + "add_userportfoliopermission", + "change_userportfoliopermission", + "delete_userportfoliopermission", "add_verifiedbystaff", "change_verifiedbystaff", "delete_verifiedbystaff", @@ -58,6 +60,7 @@ class TestGroups(TestCase): # Get the codenames of actual permissions associated with the group actual_permissions = [p.codename for p in cisa_analysts_group.permissions.all()] + self.maxDiff = None # Assert that the actual permissions match the expected permissions self.assertListEqual(actual_permissions, expected_permissions) From 0612e1bcc5613bbd1957532b7a5c13a5ffe83dc3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:40:45 -0600 Subject: [PATCH 39/79] Update src/registrar/admin.py --- src/registrar/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 49cf08935..c5dfecde3 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3185,7 +3185,6 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True - # We repeat these calls twice. extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) extra_context["domains"] = obj.get_domains() From 73264cce96a90fd4a7687149af558168e9a0e565 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:43:40 -0600 Subject: [PATCH 40/79] Remove unused portfolio_type (thanks, Linter!) --- src/registrar/admin.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 49cf08935..920411737 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2968,7 +2968,6 @@ class PortfolioAdmin(ListHeaderAdmin): "domains", "domain_requests", "suborganizations", - "portfolio_type", "display_admins", "display_members", "creator", @@ -3029,12 +3028,6 @@ class PortfolioAdmin(ListHeaderAdmin): created_on.short_description = "Created on" # type: ignore - def portfolio_type(self, obj: models.Portfolio): - """Returns the portfolio type, or "-" if the result is empty""" - return obj.portfolio_type if obj.portfolio_type else "-" - - portfolio_type.short_description = "Portfolio type" # type: ignore - def suborganizations(self, obj: models.Portfolio): """Returns a list of links for each related suborg""" queryset = obj.get_suborganizations() From 35fec1ed894b013237ce84a861b1af9cc8ab6ec9 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 26 Sep 2024 08:02:13 -1000 Subject: [PATCH 41/79] fixed unit tests --- src/registrar/tests/test_views_portfolio.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 42d0e0490..cc1f35e85 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -703,7 +703,7 @@ class TestPortfolio(WebTest): # --- admin UserPortfolioPermission.objects.filter(user=self.user, portfolio=self.portfolio).update( - role=UserPortfolioRoleChoices.ORGANIZATION_ADMIN, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], ) # Verify that the user cannot access the members page @@ -723,6 +723,7 @@ class TestPortfolio(WebTest): UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], additional_permissions=[ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_MEMBERS, @@ -733,7 +734,7 @@ class TestPortfolio(WebTest): # This will redirect the user to the members page. response = self.app.get(reverse("members")) # Make sure the page loaded - self.assertEqual(response.status_code, 200) + self.assertContains(response, "Members") @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -747,6 +748,7 @@ class TestPortfolio(WebTest): UserPortfolioPermission.objects.get_or_create( user=self.user, portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], additional_permissions=[ UserPortfolioPermissionChoices.VIEW_PORTFOLIO, UserPortfolioPermissionChoices.VIEW_MEMBERS, @@ -760,8 +762,11 @@ class TestPortfolio(WebTest): # Verify that the user can access the members page # This will redirect the user to the members page. + self.client.force_login(self.user) response = self.client.get(reverse("members"), follow=True) - + # Make sure the page loaded + self.assertEqual(response.status_code, 200) + # Check that the manage settings appear in the DOM self.assertContains(response, '') self.assertContains(response, "Manage") @@ -790,10 +795,12 @@ class TestPortfolio(WebTest): # Verify that the user can access the members page # This will redirect the user to the members page. + self.client.force_login(self.user) response = self.client.get(reverse("members"), follow=True) # Make sure the page loaded self.assertEqual(response.status_code, 200) - + # Check that the view-only settings appear in the DOM + self.assertContains(response, '') self.assertContains(response, "View") @less_console_noise_decorator From ada24a68648bce51f63d6b61c53b96e5f21ff2a1 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 26 Sep 2024 09:16:39 -1000 Subject: [PATCH 42/79] JK on last commit -- more unit test fixes --- src/registrar/tests/test_views_portfolio.py | 64 ++++++++++++++++++--- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index cc1f35e85..164ce5187 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -1,6 +1,8 @@ +import re from django.urls import reverse from api.tests.common import less_console_noise_decorator from registrar.config import settings +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.models import Portfolio, SeniorOfficial from django_webtest import WebTest # type: ignore from registrar.models import ( @@ -733,10 +735,17 @@ class TestPortfolio(WebTest): # Verify that the user can access the members page # This will redirect the user to the members page. response = self.app.get(reverse("members")) + + # ---- Useful debugging stub to see what "assertContains" is finding + # pattern = r'Members' + # matches = re.findall(pattern, response.content.decode('utf-8')) + # for match in matches: + # TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"{match}") + # Make sure the page loaded self.assertContains(response, "Members") - @less_console_noise_decorator + @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) def test_can_manage_members(self): @@ -755,6 +764,7 @@ class TestPortfolio(WebTest): UserPortfolioPermissionChoices.EDIT_MEMBERS, ], ) + # Give user permissions to modify user objects in the DB group, _ = UserGroup.objects.get_or_create(name="full_access_group") # Add the user to the group @@ -766,9 +776,12 @@ class TestPortfolio(WebTest): response = self.client.get(reverse("members"), follow=True) # Make sure the page loaded self.assertEqual(response.status_code, 200) - # Check that the manage settings appear in the DOM - self.assertContains(response, '') - self.assertContains(response, "Manage") + + # Verify that manage settings are sent in the dynamic HTML + self.client.force_login(self.user) + response = self.client.get(reverse("get_portfolio_members_json")) + self.assertContains(response, '"action_label": "Manage"') + self.assertContains(response, '"svg_icon": "settings"') @less_console_noise_decorator @override_flag("organization_feature", active=True) @@ -799,9 +812,46 @@ class TestPortfolio(WebTest): response = self.client.get(reverse("members"), follow=True) # Make sure the page loaded self.assertEqual(response.status_code, 200) - # Check that the view-only settings appear in the DOM - self.assertContains(response, '') - self.assertContains(response, "View") + + # Verify that view-only settings are sent in the dynamic HTML + response = self.client.get(reverse("get_portfolio_members_json")) + self.assertContains(response, '"action_label": "View"') + self.assertContains(response, '"svg_icon": "visibility"') + + + @override_flag("organization_feature", active=True) + @override_flag("organization_members", active=True) + def test_members_admin_tag(self): + """Test that user with proper permission is able to manage members""" + user = self.user + self.app.set_user(user.username) + + # give user permissions to view AND manage members + UserPortfolioPermission.objects.get_or_create( + user=self.user, + portfolio=self.portfolio, + roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN], + additional_permissions=[ + UserPortfolioPermissionChoices.VIEW_PORTFOLIO, + UserPortfolioPermissionChoices.VIEW_MEMBERS, + UserPortfolioPermissionChoices.EDIT_MEMBERS, + ], + ) + + # Give user permissions to modify user objects in the DB + group, _ = UserGroup.objects.get_or_create(name="full_access_group") + # Add the user to the group + user.groups.set([group]) + + # Verify that the user can access the members page + # This will redirect the user to the members page. + self.client.force_login(self.user) + response = self.client.get(reverse("members"), follow=True) + # Make sure the page loaded + self.assertEqual(response.status_code, 200) + # Check that the manage settings appear in the DOM + self.assertContains(response, '') + @less_console_noise_decorator @override_flag("organization_feature", active=True) From 8508c99f8fd649c22e34a1a88b9db9428c2697cb Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 26 Sep 2024 09:18:56 -1000 Subject: [PATCH 43/79] added a test --- src/registrar/tests/test_views_portfolio.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 164ce5187..485d28f5d 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -821,7 +821,7 @@ class TestPortfolio(WebTest): @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) - def test_members_admin_tag(self): + def test_members_admin_detection(self): """Test that user with proper permission is able to manage members""" user = self.user self.app.set_user(user.username) @@ -849,8 +849,10 @@ class TestPortfolio(WebTest): response = self.client.get(reverse("members"), follow=True) # Make sure the page loaded self.assertEqual(response.status_code, 200) - # Check that the manage settings appear in the DOM - self.assertContains(response, '') + # Verify that admin info is sent in the dynamic HTML + response = self.client.get(reverse("get_portfolio_members_json")) + # TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"{response.content}") + self.assertContains(response, '"is_admin": true') @less_console_noise_decorator From 93fbfa3a645eff902ff6aa8a622156636d149f0e Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 26 Sep 2024 09:22:21 -1000 Subject: [PATCH 44/79] linted --- src/registrar/tests/test_views_portfolio.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index 485d28f5d..b8d8e233d 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -1,8 +1,6 @@ -import re from django.urls import reverse from api.tests.common import less_console_noise_decorator from registrar.config import settings -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.models import Portfolio, SeniorOfficial from django_webtest import WebTest # type: ignore from registrar.models import ( @@ -745,7 +743,6 @@ class TestPortfolio(WebTest): # Make sure the page loaded self.assertContains(response, "Members") - @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) def test_can_manage_members(self): @@ -776,7 +773,7 @@ class TestPortfolio(WebTest): response = self.client.get(reverse("members"), follow=True) # Make sure the page loaded self.assertEqual(response.status_code, 200) - + # Verify that manage settings are sent in the dynamic HTML self.client.force_login(self.user) response = self.client.get(reverse("get_portfolio_members_json")) @@ -812,12 +809,11 @@ class TestPortfolio(WebTest): response = self.client.get(reverse("members"), follow=True) # Make sure the page loaded self.assertEqual(response.status_code, 200) - + # Verify that view-only settings are sent in the dynamic HTML response = self.client.get(reverse("get_portfolio_members_json")) self.assertContains(response, '"action_label": "View"') self.assertContains(response, '"svg_icon": "visibility"') - @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) @@ -851,10 +847,9 @@ class TestPortfolio(WebTest): self.assertEqual(response.status_code, 200) # Verify that admin info is sent in the dynamic HTML response = self.client.get(reverse("get_portfolio_members_json")) - # TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"{response.content}") + # TerminalHelper.colorful_logger(logger.info, TerminalColors.OKCYAN, f"{response.content}") self.assertContains(response, '"is_admin": true') - @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_portfolio_domain_requests_page_when_user_has_no_permissions(self): From 6118ca1393fd9d7dad71fd3b35a108ce87c775c3 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 26 Sep 2024 09:26:34 -1000 Subject: [PATCH 45/79] fixes --- src/registrar/tests/test_views_portfolio.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index b8d8e233d..cfbf5e1b0 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -697,7 +697,8 @@ class TestPortfolio(WebTest): ) # Verify that the user cannot access the members page # This will redirect the user to the members page. - response = self.app.get(reverse("members")) + self.client.force_login(self.user) + response = self.client.get(reverse("members"), follow=True) # Assert the response is a 403 Forbidden self.assertEqual(response.status_code, 403) @@ -708,7 +709,7 @@ class TestPortfolio(WebTest): # Verify that the user cannot access the members page # This will redirect the user to the members page. - response = self.app.get(reverse("members"), follow=True) + response = self.client.get(reverse("members"), follow=True) # Assert the response is a 403 Forbidden self.assertEqual(response.status_code, 403) @@ -732,7 +733,10 @@ class TestPortfolio(WebTest): # Verify that the user can access the members page # This will redirect the user to the members page. - response = self.app.get(reverse("members")) + self.client.force_login(self.user) + response = self.client.get(reverse("members"), follow=True) + # Make sure the page loaded + self.assertEqual(response.status_code, 200) # ---- Useful debugging stub to see what "assertContains" is finding # pattern = r'Members' @@ -743,6 +747,7 @@ class TestPortfolio(WebTest): # Make sure the page loaded self.assertContains(response, "Members") + @less_console_noise_decorator @override_flag("organization_feature", active=True) @override_flag("organization_members", active=True) def test_can_manage_members(self): From f624a496ffae0c3c9218cacbfa163d26f7581bba Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 26 Sep 2024 09:33:09 -1000 Subject: [PATCH 46/79] last fix... --- src/registrar/tests/test_views_domain.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index f0776d10c..127b78a4a 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -1567,9 +1567,9 @@ class TestDomainSuborganization(TestDomainOverview): self.domain_information.refresh_from_db() # Add portfolio perms to the user object - # portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( - # user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] - # ) + portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + ) self.assertEqual(self.domain_information.sub_organization, suborg) From 6aa7546c2ee306e34d7d00e3ade59756f0099bb1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:12:20 -0600 Subject: [PATCH 47/79] Fix SO message and fix search text --- src/registrar/admin.py | 2 +- src/registrar/assets/js/get-gov-admin.js | 2 +- .../django/admin/includes/portfolio/portfolio_fieldset.html | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7bf92125f..ea84f3191 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2957,7 +2957,7 @@ class PortfolioAdmin(ListHeaderAdmin): list_display = ("organization_name", "organization_type", "federal_type", "creator") search_fields = ["organization_name"] - search_help_text = "Search by portfolio organization." + search_help_text = "Search by organization name." readonly_fields = [ # This is the created_at field "created_on", diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 7271cf7b8..d98d11437 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -978,7 +978,7 @@ function initializeWidgetOnList(list, parentId) { $seniorOfficial.val("").trigger("change"); }else { // Show the "create one now" text if this field is none in readonly mode. - readonlySeniorOfficial.innerHTML = 'No senior official has been found. Create one now.' + readonlySeniorOfficial.innerHTML = 'No senior official found. Create one now.' } console.warn("Record not found: " + data.error); }else { diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html index c5c6f510a..87b56cb60 100644 --- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html +++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html @@ -27,7 +27,7 @@ {% else %} {% url "admin:registrar_seniorofficial_add" as url %}
    - No senior official has been found. Create one now. + No senior official found. Create one now.
    {% endif %} {% else %} From d80287a428cee71b9283da408156144dedc9ff68 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:37:31 -0600 Subject: [PATCH 48/79] User portfolio suggestions --- src/registrar/admin.py | 18 +++++++++++++----- src/registrar/models/portfolio.py | 15 +++++++++++---- .../models/user_portfolio_permission.py | 14 ++++++++++---- src/registrar/tests/test_models.py | 5 ++++- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ea84f3191..ffea1fb24 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -10,8 +10,7 @@ from django.http import HttpResponseRedirect from django.conf import settings from django.shortcuts import redirect from django_fsm import get_available_FIELD_transitions, FSMField -from registrar.models.domain_information import DomainInformation -from registrar.models.user_portfolio_permission import UserPortfolioPermission +from registrar.models import DomainInformation, Portfolio, UserPortfolioPermission from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from waffle.decorators import flag_is_active from django.contrib import admin, messages @@ -1257,9 +1256,18 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): list_display = [ "user", "portfolio", + "get_roles", ] autocomplete_fields = ["user", "portfolio"] + search_fields = ["user__first_name", "user__last_name", "user__email", "portfolio__organization_name"] + search_help_text = "Search by first name, last name, email, or portfolio." + + def get_roles(self, obj): + readable_roles = obj.get_readable_roles() + return ", ".join(readable_roles) + + get_roles.short_description = "Roles" class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): @@ -3174,14 +3182,14 @@ class PortfolioAdmin(ListHeaderAdmin): def change_view(self, request, object_id, form_url="", extra_context=None): """Add related suborganizations and domain groups. Add the summary for the portfolio members field (list of members that link to change_forms).""" - obj = self.get_object(request, object_id) + obj: Portfolio = self.get_object(request, object_id) extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) - extra_context["domains"] = obj.get_domains() - extra_context["domain_requests"] = obj.get_domain_requests() + extra_context["domains"] = obj.get_domains(order_by=["domain_information__name"]) + extra_context["domain_requests"] = obj.get_domain_requests(order_by=["domain_information__name"]) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 9acec8c64..e6f7162d6 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -132,13 +132,20 @@ class Portfolio(TimeStampedModel): return federal_agency.federal_type if federal_agency else None # == Getters for domains == # - def get_domains(self): + def get_domains(self, order_by=None): """Returns all DomainInformations associated with this portfolio""" - return self.information_portfolio.all() + if not order_by: + return self.information_portfolio.all() + else: + return self.information_portfolio.all().order_by(*order_by) - def get_domain_requests(self): + def get_domain_requests(self, order_by=None): """Returns all DomainRequests associated with this portfolio""" - return self.DomainRequest_portfolio.all() + if not order_by: + return self.DomainRequest_portfolio.all() + else: + return self.DomainRequest_portfolio.all().order_by(*order_by) + # == Getters for suborganization == # def get_suborganizations(self): diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 5479b6f3d..4cf85d4fc 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -77,11 +77,16 @@ class UserPortfolioPermission(TimeStampedModel): def __str__(self): readable_roles = [] if self.roles: - readable_roles = sorted( - [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles] - ) + readable_roles = self.get_readable_roles() return f"{self.user}" f" " if self.roles else "" + def get_readable_roles(self): + """Returns a readable list of self.roles""" + readable_roles = [] + if self.roles: + readable_roles = sorted([UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles]) + return readable_roles + def _get_portfolio_permissions(self): """ Retrieve the permissions for the user's portfolio roles. @@ -108,7 +113,8 @@ class UserPortfolioPermission(TimeStampedModel): existing_permissions = UserPortfolioPermission.objects.filter(user=self.user) if not flag_is_active_for_user(self.user, "multiple_portfolios") and existing_permissions.exists(): raise ValidationError( - "Only one portfolio permission is allowed per user when multiple portfolios are disabled." + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." ) # Check if portfolio is set without accessing the related object. diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a6cac1389..015c67dab 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1332,7 +1332,10 @@ class TestUserPortfolioPermission(TestCase): self.assertEqual( cm.exception.message, - "Only one portfolio permission is allowed per user when multiple portfolios are disabled.", + ( + "This user is already assigned to a portfolio. " + "Based on current waffle flag settings, users cannot be assigned to multiple portfolios." + ), ) From 2260269e3d5e444c79b37c1fcf369907cc800541 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:39:21 -0600 Subject: [PATCH 49/79] fix order by --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ffea1fb24..de1b96e6c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3188,8 +3188,8 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) - extra_context["domains"] = obj.get_domains(order_by=["domain_information__name"]) - extra_context["domain_requests"] = obj.get_domain_requests(order_by=["domain_information__name"]) + extra_context["domains"] = obj.get_domains(order_by=["domain__name"]) + extra_context["domain_requests"] = obj.get_domain_requests(order_by=["requested_domain__name"]) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): From 81e76fe04a74800826308b68c2f378edc69240f9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:49:19 -0600 Subject: [PATCH 50/79] fix fed agency bug --- src/registrar/assets/js/get-gov-admin.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index d98d11437..c01f5d784 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -860,10 +860,13 @@ function initializeWidgetOnList(list, parentId) { let organizationType = document.getElementById("id_organization_type"); let readonlyOrganizationType = document.querySelector(".field-organization_type .readonly"); + let organizationNameContainer = document.querySelector(".field-organization_name"); + let federalType = document.querySelector(".field-federal_type"); + if ($federalAgency && (organizationType || readonlyOrganizationType)) { // Attach the change event listener $federalAgency.on("change", function() { - handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType); + handleFederalAgencyChange($federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType); }); } @@ -882,8 +885,6 @@ function initializeWidgetOnList(list, parentId) { // Handle hiding the organization name field when the organization_type is federal. // Run this first one page load, then secondly on a change event. - let organizationNameContainer = document.querySelector(".field-organization_name"); - let federalType = document.querySelector(".field-federal_type"); handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); organizationType.addEventListener("change", function() { handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); @@ -907,7 +908,7 @@ function initializeWidgetOnList(list, parentId) { } } - function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType) { + function handleFederalAgencyChange(federalAgency, organizationType, readonlyOrganizationType, organizationNameContainer, federalType) { // Don't do anything on page load if (isInitialPageLoad) { isInitialPageLoad = false; @@ -941,6 +942,8 @@ function initializeWidgetOnList(list, parentId) { } } + handleOrganizationTypeChange(organizationType, organizationNameContainer, federalType); + // Determine if any changes are necessary to the display of portfolio type or federal type // based on changes to the Federal Agency let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value; From 88b38baaa7e96b37a50406c057587d1c9eea5dbb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:50:08 -0600 Subject: [PATCH 51/79] lint --- src/registrar/models/portfolio.py | 1 - src/registrar/models/user_portfolio_permission.py | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index e6f7162d6..8d820e105 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -145,7 +145,6 @@ class Portfolio(TimeStampedModel): return self.DomainRequest_portfolio.all() else: return self.DomainRequest_portfolio.all().order_by(*order_by) - # == Getters for suborganization == # def get_suborganizations(self): diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 4cf85d4fc..f021fc6bf 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -84,7 +84,9 @@ class UserPortfolioPermission(TimeStampedModel): """Returns a readable list of self.roles""" readable_roles = [] if self.roles: - readable_roles = sorted([UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles]) + readable_roles = sorted( + [UserPortfolioRoleChoices.get_user_portfolio_role_label(role) for role in self.roles] + ) return readable_roles def _get_portfolio_permissions(self): From a7b3fc71a5d251b4f24486f095107ef9aa897c75 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 26 Sep 2024 14:57:04 -0600 Subject: [PATCH 52/79] Update admin.py --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index de1b96e6c..8406ac3a6 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1267,7 +1267,7 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin): readable_roles = obj.get_readable_roles() return ", ".join(readable_roles) - get_roles.short_description = "Roles" + get_roles.short_description = "Roles" # type: ignore class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin): From 92abc78c3d1750ae2eedcc991225e1830bf66d29 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 08:59:26 -0600 Subject: [PATCH 53/79] Cleanup --- src/registrar/assets/sass/_theme/_tooltips.scss | 3 +-- .../commands/create_federal_portfolio.py | 7 ------- .../commands/utility/terminal_helper.py | 1 + .../templates/includes/members_table.html | 17 +++-------------- src/registrar/templates/portfolio_members.html | 2 +- src/registrar/tests/test_reports.py | 5 ----- src/registrar/views/portfolio_members_json.py | 4 ---- src/registrar/views/portfolios.py | 1 - 8 files changed, 6 insertions(+), 34 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_tooltips.scss b/src/registrar/assets/sass/_theme/_tooltips.scss index 177809615..f0abf915d 100644 --- a/src/registrar/assets/sass/_theme/_tooltips.scss +++ b/src/registrar/assets/sass/_theme/_tooltips.scss @@ -1,5 +1,4 @@ @use "uswds-core" as *; -@use "cisa_colors" as *; // Only apply this custom wrapping to desktop @@ -29,4 +28,4 @@ #extended-logo .usa-tooltip__body { font-weight: 400 !important; -} \ No newline at end of file +} diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 4f715d1b7..cf9339bec 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -256,10 +256,3 @@ class Command(BaseCommand): DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - - def handle_portfolio_members(self, portfolio: Portfolio, federal_agency: FederalAgency): - """ - Associate portfolio with members for a federal agency. - Updates all relevant member records. - """ - # TODO: future ticket? diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index d5ee478f7..fa7cde683 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -7,6 +7,7 @@ from django.db.models.manager import BaseManager from typing import List from registrar.utility.enums import LogCode + logger = logging.getLogger(__name__) diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index 78f126a09..307bfcbf2 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -40,8 +40,9 @@ + {% comment %} - {% if portfolio_members_count and portfolio_members_count > 0 %} + {% if portfolio_members and portfolio_members|length > 0 %} {% endif %} + {% endcomment %} {% if portfolio %} @@ -188,19 +190,6 @@ - {% include "includes/members_table.html" with portfolio=portfolio portfolio_members_count=portfolio_members_count %} + {% include "includes/members_table.html" with portfolio=portfolio %} {% endblock %} diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index d4c0f2ab5..797592806 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -356,11 +356,6 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): self.assertIn(self.domain_3.name, csv_content) self.assertNotIn(self.domain_2.name, csv_content) - # Test the output for readonly admin - # portfolio_permission.roles = [UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY] - # portfolio_permission.save() - # portfolio_permission.refresh_from_db() - # Get the csv content csv_content = self._run_domain_data_type_user_export(request) self.assertIn(self.domain_1.name, csv_content) diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 7a06dd9fb..587caba94 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -43,7 +43,6 @@ def get_portfolio_members_json(request): for member in page_obj.object_list ] - # DEVELOPER'S NOTE (9-24-24): # If you're wondering where these JSON values are used, check out the class "MembersTable" # in get-gov.js (specifically the "loadTable" function). # @@ -68,9 +67,6 @@ def get_portfolio_members_json(request): ) else: - # This was added to handle NoneType error - # In other examples of we assume there will never be zero entries returned...which is *fine*...until - # something goes wrong. return JsonResponse( { "members": [], diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 54438964a..6d29d9006 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -61,7 +61,6 @@ class PortfolioMembersView(PortfolioMembersPermissionView, View): all_members = User.objects.filter(id__in=member_ids) context["portfolio_members"] = all_members - context["portfolio_members_count"] = all_members.count() return render(request, "portfolio_members.html", context) From a128ebcf6937a652143f5f040239d5401ee4b550 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:46:21 -0600 Subject: [PATCH 54/79] Update settings.py --- src/registrar/config/settings.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 9eb649ad8..30882cd5d 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -476,6 +476,8 @@ class JsonServerFormatter(ServerFormatter): def format(self, record): formatted_record = super().format(record) + if not hasattr(record, "server_time"): + record.server_time = self.formatTime(record, self.datefmt) log_entry = {"server_time": record.server_time, "level": record.levelname, "message": formatted_record} return json.dumps(log_entry) From 14a1bd53ba67b8b65d8879bb0a028b110d4b1bee Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 13:19:30 -0600 Subject: [PATCH 55/79] Fix url bug --- src/registrar/admin.py | 16 ++++++++++------ src/registrar/assets/js/get-gov-admin.js | 3 ++- .../django/admin/portfolio_change_form.html | 2 ++ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8406ac3a6..56f5310e0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3186,10 +3186,11 @@ class PortfolioAdmin(ListHeaderAdmin): extra_context = extra_context or {} extra_context["skip_additional_contact_info"] = True - extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) - extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) - extra_context["domains"] = obj.get_domains(order_by=["domain__name"]) - extra_context["domain_requests"] = obj.get_domain_requests(order_by=["requested_domain__name"]) + if obj: + extra_context["members"] = self.get_user_portfolio_permission_non_admins(obj) + extra_context["admins"] = self.get_user_portfolio_permission_admins(obj) + extra_context["domains"] = obj.get_domains(order_by=["domain__name"]) + extra_context["domain_requests"] = obj.get_domain_requests(order_by=["requested_domain__name"]) return super().change_view(request, object_id, form_url, extra_context) def save_model(self, request, obj, form, change): @@ -3208,8 +3209,11 @@ class PortfolioAdmin(ListHeaderAdmin): obj.organization_name = obj.federal_agency.agency # Remove this line when senior_official is no longer readonly in /admin. - if obj.federal_agency and obj.federal_agency.so_federal_agency.exists(): - obj.senior_official = obj.federal_agency.so_federal_agency.first() + if obj.federal_agency: + if obj.federal_agency.so_federal_agency.exists(): + obj.senior_official = obj.federal_agency.so_federal_agency.first() + else: + obj.senior_official = None super().save_model(request, obj, form, change) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index c01f5d784..25e35b73b 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -965,6 +965,7 @@ function initializeWidgetOnList(list, parentId) { // If we can update the contact information, it'll be shown again. hideElement(contactList.parentElement); + let seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value; let $seniorOfficial = django.jQuery("#id_senior_official"); let readonlySeniorOfficial = document.querySelector(".field-senior_official .readonly"); let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; @@ -981,7 +982,7 @@ function initializeWidgetOnList(list, parentId) { $seniorOfficial.val("").trigger("change"); }else { // Show the "create one now" text if this field is none in readonly mode. - readonlySeniorOfficial.innerHTML = 'No senior official found. Create one now.' + readonlySeniorOfficial.innerHTML = `No senior official found. Create one now.`; } console.warn("Record not found: " + data.error); }else { diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index fec1538d9..8de6cd5eb 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -8,6 +8,8 @@ {% url 'get-federal-and-portfolio-types-from-federal-agency-json' as url %} + {% url "admin:registrar_seniorofficial_add" as url %} + {{ block.super }} {% endblock content %} From 9a7a65d958af3a3278b06ad1e2e01a7a425d2011 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 13:23:31 -0600 Subject: [PATCH 56/79] fix merge conflict --- .../templates/django/admin/includes/detail_table_fieldset.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index f6ada5166..6b755724e 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -119,7 +119,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% endfor %} {% endwith %} - {% elif field.field.name == "display_admins" or field.field.name == "domain_managers" or field.field.namd == "invited_domain_managers" %} + {% elif field.field.name == "domain_managers" or field.field.name == "invited_domain_managers" %}
    {{ field.contents|safe }}
    {% elif field.field.name == "display_members" %}
    From 3290137833bc683e3c68607080ddd470fdecc1d9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:20:03 -0600 Subject: [PATCH 57/79] Federal agency changes --- src/registrar/admin.py | 2 +- ...pulate_federal_agency_initials_and_fceb.py | 2 +- ...nitials_federalagency_acronym_and_more.py} | 33 ++++++++++++++++++- src/registrar/models/federal_agency.py | 7 ++-- .../tests/test_management_scripts.py | 10 +++--- 5 files changed, 42 insertions(+), 12 deletions(-) rename src/registrar/migrations/{0130_alter_portfolio_federal_agency_and_more.py => 0130_remove_federalagency_initials_federalagency_acronym_and_more.py} (62%) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 2e81d1d4b..c93159c6b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3291,7 +3291,7 @@ class FederalAgencyResource(resources.ModelResource): class FederalAgencyAdmin(ListHeaderAdmin, ImportExportModelAdmin): list_display = ["agency"] search_fields = ["agency"] - search_help_text = "Search by agency name." + search_help_text = "Search by federal agency." ordering = ["agency"] resource_classes = [FederalAgencyResource] diff --git a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py index 506405b78..30ae08b47 100644 --- a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py +++ b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py @@ -42,7 +42,7 @@ class Command(BaseCommand, PopulateScriptTemplate): """For each record, update the initials and is_fceb field if data exists for it""" initials, agency_status = self.federal_agency_dict.get(record.agency) - record.initials = initials + record.acronym = initials if agency_status and isinstance(agency_status, str) and agency_status.strip().upper() == "FCEB": record.is_fceb = True else: diff --git a/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py similarity index 62% rename from src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py rename to src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py index f3372b405..665b2115f 100644 --- a/src/registrar/migrations/0130_alter_portfolio_federal_agency_and_more.py +++ b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-09-26 15:09 +# Generated by Django 4.2.10 on 2024-09-27 20:12 from django.db import migrations, models import django.db.models.deletion @@ -12,6 +12,37 @@ class Migration(migrations.Migration): ] operations = [ + migrations.RemoveField( + model_name="federalagency", + name="initials", + ), + migrations.AddField( + model_name="federalagency", + name="acronym", + field=models.CharField( + blank=True, + help_text="Acronym commonly used to reference the federal agency (Optional)", + max_length=10, + null=True, + ), + ), + migrations.AlterField( + model_name="federalagency", + name="federal_type", + field=models.CharField( + blank=True, + choices=[("executive", "Executive"), ("judicial", "Judicial"), ("legislative", "Legislative")], + max_length=20, + null=True, + ), + ), + migrations.AlterField( + model_name="federalagency", + name="is_fceb", + field=models.BooleanField( + blank=True, help_text="Federal Civilian Executive Branch (FCEB)", null=True, verbose_name="FCEB" + ), + ), migrations.AlterField( model_name="portfolio", name="federal_agency", diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 5cc87b38c..aeeebac8c 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -22,21 +22,20 @@ class FederalAgency(TimeStampedModel): choices=BranchChoices.choices, null=True, blank=True, - help_text="Federal agency type (executive, judicial, legislative, etc.)", ) - initials = models.CharField( + acronym = models.CharField( max_length=10, null=True, blank=True, - help_text="Agency initials", + help_text="Acronym commonly used to reference the federal agency (Optional)", ) is_fceb = models.BooleanField( null=True, blank=True, verbose_name="FCEB", - help_text="Determines if this agency is FCEB", + help_text="Federal Civilian Executive Branch (FCEB)", ) def __str__(self) -> str: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index cbdc2c034..9fcd261f7 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1387,18 +1387,18 @@ class TestPopulateFederalAgencyInitialsAndFceb(TestCase): self.agency4.refresh_from_db() # Check if FederalAgency objects were updated correctly - self.assertEqual(self.agency1.initials, "ABMC") + self.assertEqual(self.agency1.acronym, "ABMC") self.assertTrue(self.agency1.is_fceb) - self.assertEqual(self.agency2.initials, "ACHP") + self.assertEqual(self.agency2.acronym, "ACHP") self.assertTrue(self.agency2.is_fceb) # We expect that this field doesn't have any data, # as none is specified in the CSV - self.assertIsNone(self.agency3.initials) + self.assertIsNone(self.agency3.acronym) self.assertIsNone(self.agency3.is_fceb) - self.assertEqual(self.agency4.initials, "KC") + self.assertEqual(self.agency4.acronym, "KC") self.assertFalse(self.agency4.is_fceb) @less_console_noise_decorator @@ -1411,7 +1411,7 @@ class TestPopulateFederalAgencyInitialsAndFceb(TestCase): # Verify that the missing agency was not updated missing_agency.refresh_from_db() - self.assertIsNone(missing_agency.initials) + self.assertIsNone(missing_agency.acronym) self.assertIsNone(missing_agency.is_fceb) From fbd82d5e6f2d6b5c29daa5fb3a0b83bd0f2d9619 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:22:03 -0600 Subject: [PATCH 58/79] suborg changes + portfolio on user --- src/registrar/admin.py | 70 ++++--------------- ...initials_federalagency_acronym_and_more.py | 7 +- src/registrar/models/suborganization.py | 2 +- .../models/utility/generic_helper.py | 8 +++ .../django/admin/suborg_change_form.html | 40 ++++++----- .../django/admin/user_change_form.html | 20 ------ src/registrar/utility/admin_helpers.py | 51 ++++++++++++++ 7 files changed, 104 insertions(+), 94 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c93159c6b..e3a3b2b2d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -20,7 +20,7 @@ from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email +from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email, get_field_links_as_list from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes @@ -755,9 +755,10 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): }, ), ("Important dates", {"fields": ("last_login", "date_joined")}), + ("Associated portfolios", {"fields": ("portfolios",)}), ) - readonly_fields = ("verification_type",) + readonly_fields = ("verification_type", "portfolios") analyst_fieldsets = ( ( @@ -780,6 +781,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): }, ), ("Important dates", {"fields": ("last_login", "date_joined")}), + ("Associated portfolios", {"fields": ("portfolios",)}), ) # TODO: delete after we merge organization feature @@ -859,6 +861,14 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): ordering = ["first_name", "last_name", "email"] search_help_text = "Search by first name, last name, or email." + def portfolios(self, obj: models.User): + """Returns a list of links for each related suborg""" + portfolio_ids = obj.get_portfolios().values_list("portfolio", flat=True) + queryset = models.Portfolio.objects.filter(id__in=portfolio_ids) + return get_field_links_as_list(queryset, "portfolio") + + portfolios.short_description = "Portfolios" # type: ignore + def get_search_results(self, request, queryset, search_term): """ Override for get_search_results. This affects any upstream model using autocomplete_fields, @@ -3101,7 +3111,7 @@ class PortfolioAdmin(ListHeaderAdmin): def suborganizations(self, obj: models.Portfolio): """Returns a list of links for each related suborg""" queryset = obj.get_suborganizations() - return self.get_field_links_as_list(queryset, "suborganization") + return get_field_links_as_list(queryset, "suborganization") suborganizations.short_description = "Suborganizations" # type: ignore @@ -3159,59 +3169,6 @@ class PortfolioAdmin(ListHeaderAdmin): "senior_official", ] - def get_field_links_as_list( - self, queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None - ): - """ - Generate HTML links for items in a queryset, using a specified attribute for link text. - - Args: - queryset: The queryset of items to generate links for. - model_name: The model name used to construct the admin change URL. - attribute_name: The attribute or method name to use for link text. If None, the item itself is used. - link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. - separator: The separator to use between links in the resulting HTML. - If none, an unordered list is returned. - - Returns: - A formatted HTML string with links to the admin change pages for each item. - """ - links = [] - for item in queryset: - - # This allows you to pass in attribute_name="get_full_name" for instance. - if attribute_name: - item_display_value = self.value_of_attribute(item, attribute_name) - else: - item_display_value = item - - if item_display_value: - change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) - - link = f'{escape(item_display_value)}' - if link_info_attribute: - link += f" ({self.value_of_attribute(item, link_info_attribute)})" - - if separator: - links.append(link) - else: - links.append(f"
  • {link}
    • ") - - # If no separator is specified, just return an unordered list. - if separator: - return format_html(separator.join(links)) if links else "-" - else: - links = "".join(links) - return format_html(f'
      {links}
    ') if links else "-" - - def value_of_attribute(self, obj, attribute_name: str): - """Returns the value of getattr if the attribute isn't callable. - If it is, execute the underlying function and return that result instead.""" - value = getattr(obj, attribute_name) - if callable(value): - value = value() - return value - def get_fieldsets(self, request, obj=None): """Override of the default get_fieldsets definition to add an add_fieldsets view""" # This is the add view if no obj exists @@ -3348,6 +3305,7 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportModelAdmin): "portfolio", ] search_fields = ["name"] + search_help_text = "Search by suborganization." change_form_template = "django/admin/suborg_change_form.html" diff --git a/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py index 665b2115f..6e5935748 100644 --- a/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py +++ b/src/registrar/migrations/0130_remove_federalagency_initials_federalagency_acronym_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-09-27 20:12 +# Generated by Django 4.2.10 on 2024-09-27 20:20 from django.db import migrations, models import django.db.models.deletion @@ -87,4 +87,9 @@ class Migration(migrations.Migration): to="registrar.seniorofficial", ), ), + migrations.AlterField( + model_name="suborganization", + name="name", + field=models.CharField(max_length=1000, unique=True, verbose_name="Suborganization"), + ), ] diff --git a/src/registrar/models/suborganization.py b/src/registrar/models/suborganization.py index feeee0669..6ad80fdc0 100644 --- a/src/registrar/models/suborganization.py +++ b/src/registrar/models/suborganization.py @@ -10,7 +10,7 @@ class Suborganization(TimeStampedModel): name = models.CharField( unique=True, max_length=1000, - help_text="Suborganization", + verbose_name="Suborganization", ) portfolio = models.ForeignKey( diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 3cafe87c4..b02068de0 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -334,3 +334,11 @@ def get_url_name(path): except Resolver404: logger.error(f"No matching URL name found for path: {path}") return None + +def value_of_attribute(obj, attribute_name: str): + """Returns the value of getattr if the attribute isn't callable. + If it is, execute the underlying function and return that result instead.""" + value = getattr(obj, attribute_name) + if callable(value): + value = value() + return value diff --git a/src/registrar/templates/django/admin/suborg_change_form.html b/src/registrar/templates/django/admin/suborg_change_form.html index 005d67aec..25fe5700d 100644 --- a/src/registrar/templates/django/admin/suborg_change_form.html +++ b/src/registrar/templates/django/admin/suborg_change_form.html @@ -8,27 +8,35 @@

    Domain requests

    Domains

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

    Portfolio information

    -
    -
    -

    Portfolios

    - -
    -
    -
    - {% endif %} -

    Associated requests and domains

    diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 0b99bba13..32d2ad09d 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -1,5 +1,9 @@ from registrar.models.domain_request import DomainRequest from django.template.loader import get_template +from django.utils.html import format_html +from django.urls import reverse +from django.utils.html import escape +from registrar.models.utility.generic_helper import value_of_attribute def get_all_action_needed_reason_emails(request, domain_request): @@ -34,3 +38,50 @@ def get_action_needed_reason_default_email(request, domain_request, action_neede email_body_text_cleaned = email_body_text.strip().lstrip("\n") return email_body_text_cleaned + + +def get_field_links_as_list( + queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None + ): + """ + Generate HTML links for items in a queryset, using a specified attribute for link text. + + Args: + queryset: The queryset of items to generate links for. + model_name: The model name used to construct the admin change URL. + attribute_name: The attribute or method name to use for link text. If None, the item itself is used. + link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. + separator: The separator to use between links in the resulting HTML. + If none, an unordered list is returned. + + Returns: + A formatted HTML string with links to the admin change pages for each item. + """ + links = [] + for item in queryset: + + # This allows you to pass in attribute_name="get_full_name" for instance. + if attribute_name: + item_display_value = value_of_attribute(item, attribute_name) + else: + item_display_value = item + + if item_display_value: + change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) + + link = f'{escape(item_display_value)}' + if link_info_attribute: + link += f" ({value_of_attribute(item, link_info_attribute)})" + + if separator: + links.append(link) + else: + links.append(f"
  • {link}
    • ") + + # If no separator is specified, just return an unordered list. + if separator: + return format_html(separator.join(links)) if links else "-" + else: + links = "".join(links) + return format_html(f'
      {links}
    ') if links else "-" + From 4c756bba69a950763cf6ced2189aeeb0b376ccef Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:24:42 -0600 Subject: [PATCH 59/79] User changes --- src/registrar/admin.py | 2 +- src/registrar/utility/admin_helpers.py | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e3a3b2b2d..1ac081eeb 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -865,7 +865,7 @@ class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin): """Returns a list of links for each related suborg""" portfolio_ids = obj.get_portfolios().values_list("portfolio", flat=True) queryset = models.Portfolio.objects.filter(id__in=portfolio_ids) - return get_field_links_as_list(queryset, "portfolio") + return get_field_links_as_list(queryset, "portfolio", msg_for_none="No portfolios.") portfolios.short_description = "Portfolios" # type: ignore diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index 32d2ad09d..a6428f826 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -41,7 +41,7 @@ def get_action_needed_reason_default_email(request, domain_request, action_neede def get_field_links_as_list( - queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None + queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None, msg_for_none="-", ): """ Generate HTML links for items in a queryset, using a specified attribute for link text. @@ -53,6 +53,8 @@ def get_field_links_as_list( link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. separator: The separator to use between links in the resulting HTML. If none, an unordered list is returned. + msg_for_none: What to return when the field would otherwise display None. + Defaults to `-`. Returns: A formatted HTML string with links to the admin change pages for each item. @@ -80,8 +82,8 @@ def get_field_links_as_list( # If no separator is specified, just return an unordered list. if separator: - return format_html(separator.join(links)) if links else "-" + return format_html(separator.join(links)) if links else msg_for_none else: links = "".join(links) - return format_html(f'
      {links}
    ') if links else "-" + return format_html(f'
      {links}
    ') if links else msg_for_none From e798410ac4cf942a44b6a1456ba42588ba4912d6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:32:46 -0600 Subject: [PATCH 60/79] update domain info / domain request --- ...er_domaininformation_portfolio_and_more.py | 64 +++++++++++++++++++ src/registrar/models/domain_information.py | 5 +- src/registrar/models/domain_request.py | 5 +- 3 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py diff --git a/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py new file mode 100644 index 000000000..bf1513f7d --- /dev/null +++ b/src/registrar/migrations/0131_alter_domaininformation_portfolio_and_more.py @@ -0,0 +1,64 @@ +# Generated by Django 4.2.10 on 2024-09-30 14:31 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0130_remove_federalagency_initials_federalagency_acronym_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="domaininformation", + name="portfolio", + field=models.ForeignKey( + blank=True, + help_text="If blank, domain is not associated with a portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="sub_organization", + field=models.ForeignKey( + blank=True, + help_text="If blank, domain is associated with the overarching organization for this portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="information_sub_organization", + to="registrar.suborganization", + verbose_name="Suborganization", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="portfolio", + field=models.ForeignKey( + blank=True, + help_text="If blank, request is not associated with a portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="DomainRequest_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AlterField( + model_name="domainrequest", + name="sub_organization", + field=models.ForeignKey( + blank=True, + help_text="If blank, request is associated with the overarching organization for this portfolio.", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="request_sub_organization", + to="registrar.suborganization", + verbose_name="Suborganization", + ), + ), + ] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index d04f09c07..6e99dfbee 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -63,7 +63,7 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, related_name="information_portfolio", - help_text="Portfolio associated with this domain", + help_text="If blank, domain is not associated with a portfolio.", ) sub_organization = models.ForeignKey( @@ -72,7 +72,8 @@ class DomainInformation(TimeStampedModel): null=True, blank=True, related_name="information_sub_organization", - help_text="The suborganization that this domain is included under", + help_text="If blank, domain is associated with the overarching organization for this portfolio.", + verbose_name="Suborganization", ) domain_request = models.OneToOneField( diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index bb8693ac1..35c121f3e 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -327,7 +327,7 @@ class DomainRequest(TimeStampedModel): null=True, blank=True, related_name="DomainRequest_portfolio", - help_text="Portfolio associated with this domain request", + help_text="If blank, request is not associated with a portfolio.", ) sub_organization = models.ForeignKey( @@ -336,7 +336,8 @@ class DomainRequest(TimeStampedModel): null=True, blank=True, related_name="request_sub_organization", - help_text="The suborganization that this domain request is included under", + help_text="If blank, request is associated with the overarching organization for this portfolio.", + verbose_name="Suborganization", ) # This is the domain request user who created this domain request. From df3f37bf404372edc1dda26869b644cc1a2dc15c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 08:36:19 -0600 Subject: [PATCH 61/79] fix script --- docs/operations/data_migration.md | 2 +- .../commands/populate_federal_agency_initials_and_fceb.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 5e1aa688a..a234d882b 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -754,7 +754,7 @@ Example: `cf ssh getgov-za` | 1 | **emailTo** | Specifies where the email will be emailed. Defaults to help@get.gov on production. | ## Populate federal agency initials and FCEB -This script adds to the "is_fceb" and "initials" fields on the FederalAgency model. This script expects a CSV of federal CIOs to pull from, which can be sourced from [here](https://docs.google.com/spreadsheets/d/14oXHFpKyUXS5_mDWARPusghGdHCrP67jCleOknaSx38/edit?gid=479328070#gid=479328070). +This script adds to the "is_fceb" and "acronym" fields on the FederalAgency model. This script expects a CSV of federal CIOs to pull from, which can be sourced from [here](https://docs.google.com/spreadsheets/d/14oXHFpKyUXS5_mDWARPusghGdHCrP67jCleOknaSx38/edit?gid=479328070#gid=479328070). ### Running on sandboxes diff --git a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py index 30ae08b47..50b481e7f 100644 --- a/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py +++ b/src/registrar/management/commands/populate_federal_agency_initials_and_fceb.py @@ -36,7 +36,7 @@ class Command(BaseCommand, PopulateScriptTemplate): self.federal_agency_dict[agency_name.strip()] = (initials, agency_status) # Update every federal agency record - self.mass_update_records(FederalAgency, {"agency__isnull": False}, ["initials", "is_fceb"]) + self.mass_update_records(FederalAgency, {"agency__isnull": False}, ["acronym", "is_fceb"]) def update_record(self, record: FederalAgency): """For each record, update the initials and is_fceb field if data exists for it""" From 7602629dc5fade38c81aecf2444dd1e991e8938c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 09:08:01 -0600 Subject: [PATCH 62/79] linting --- src/registrar/admin.py | 6 +- .../models/utility/generic_helper.py | 1 + src/registrar/utility/admin_helpers.py | 86 ++++++++++--------- 3 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1ac081eeb..ca51e8b72 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -20,7 +20,11 @@ from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.utility.admin_helpers import get_all_action_needed_reason_emails, get_action_needed_reason_default_email, get_field_links_as_list +from registrar.utility.admin_helpers import ( + get_all_action_needed_reason_emails, + get_action_needed_reason_default_email, + get_field_links_as_list, +) from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.constants import BranchChoices from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index b02068de0..5e425f5a3 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -335,6 +335,7 @@ def get_url_name(path): logger.error(f"No matching URL name found for path: {path}") return None + def value_of_attribute(obj, attribute_name: str): """Returns the value of getattr if the attribute isn't callable. If it is, execute the underlying function and return that result instead.""" diff --git a/src/registrar/utility/admin_helpers.py b/src/registrar/utility/admin_helpers.py index a6428f826..2af9d0b3c 100644 --- a/src/registrar/utility/admin_helpers.py +++ b/src/registrar/utility/admin_helpers.py @@ -41,49 +41,53 @@ def get_action_needed_reason_default_email(request, domain_request, action_neede def get_field_links_as_list( - queryset, model_name, attribute_name=None, link_info_attribute=None, separator=None, msg_for_none="-", - ): - """ - Generate HTML links for items in a queryset, using a specified attribute for link text. + queryset, + model_name, + attribute_name=None, + link_info_attribute=None, + separator=None, + msg_for_none="-", +): + """ + Generate HTML links for items in a queryset, using a specified attribute for link text. - Args: - queryset: The queryset of items to generate links for. - model_name: The model name used to construct the admin change URL. - attribute_name: The attribute or method name to use for link text. If None, the item itself is used. - link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. - separator: The separator to use between links in the resulting HTML. - If none, an unordered list is returned. - msg_for_none: What to return when the field would otherwise display None. - Defaults to `-`. + Args: + queryset: The queryset of items to generate links for. + model_name: The model name used to construct the admin change URL. + attribute_name: The attribute or method name to use for link text. If None, the item itself is used. + link_info_attribute: Appends f"({value_of_attribute})" to the end of the link. + separator: The separator to use between links in the resulting HTML. + If none, an unordered list is returned. + msg_for_none: What to return when the field would otherwise display None. + Defaults to `-`. - Returns: - A formatted HTML string with links to the admin change pages for each item. - """ - links = [] - for item in queryset: + Returns: + A formatted HTML string with links to the admin change pages for each item. + """ + links = [] + for item in queryset: - # This allows you to pass in attribute_name="get_full_name" for instance. - if attribute_name: - item_display_value = value_of_attribute(item, attribute_name) - else: - item_display_value = item - - if item_display_value: - change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) - - link = f'{escape(item_display_value)}' - if link_info_attribute: - link += f" ({value_of_attribute(item, link_info_attribute)})" - - if separator: - links.append(link) - else: - links.append(f"
  • {link}
    • ") - - # If no separator is specified, just return an unordered list. - if separator: - return format_html(separator.join(links)) if links else msg_for_none + # This allows you to pass in attribute_name="get_full_name" for instance. + if attribute_name: + item_display_value = value_of_attribute(item, attribute_name) else: - links = "".join(links) - return format_html(f'
      {links}
    ') if links else msg_for_none + item_display_value = item + if item_display_value: + change_url = reverse(f"admin:registrar_{model_name}_change", args=[item.pk]) + + link = f'{escape(item_display_value)}' + if link_info_attribute: + link += f" ({value_of_attribute(item, link_info_attribute)})" + + if separator: + links.append(link) + else: + links.append(f"
  • {link}
    • ") + + # If no separator is specified, just return an unordered list. + if separator: + return format_html(separator.join(links)) if links else msg_for_none + else: + links = "".join(links) + return format_html(f'
      {links}
    ') if links else msg_for_none From b84beef6c4e18b27bd16d093af6dbf41a71c04a6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 09:41:50 -0600 Subject: [PATCH 63/79] Update create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index cf9339bec..d05a2911b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -68,9 +68,6 @@ class Command(BaseCommand): if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) - if parse_domains or both: - self.handle_portfolio_members(portfolio, federal_agency) - def create_or_modify_portfolio(self, federal_agency): """Creates or modifies a portfolio record based on a federal agency.""" portfolio_args = { From fe31dbdbade601d32622d9e11d9245748bc23cd7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 30 Sep 2024 09:44:05 -0600 Subject: [PATCH 64/79] Update members_table.html --- src/registrar/templates/includes/members_table.html | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/registrar/templates/includes/members_table.html b/src/registrar/templates/includes/members_table.html index 307bfcbf2..836d433eb 100644 --- a/src/registrar/templates/includes/members_table.html +++ b/src/registrar/templates/includes/members_table.html @@ -42,12 +42,9 @@
    {% comment %} + Note - the following if check will need to be done in javascript. + This is because you can dynamically delete these fields. {% if portfolio_members and portfolio_members|length > 0 %} - - - - {% endif %} +
    +
    -
    +
    - {% comment %} - - Note - the following if check will need to be done in javascript. - This is because you can dynamically delete these fields. - {% if portfolio_members and portfolio_members|length > 0 %} - - {% endif %} - {% endcomment %}
    - {% if portfolio %} - - - - {% endif %}