Ticket #2520: Admin portfolio view #2581

zandercymatics · 2024-08-13T20:29:07Z

Ticket

Resolves #2520

Changes

Adds custom fields to the admin portfolio view
Changes the admin portfolio view to match the design
Removes help_text on some portfolio properties that do not match the figma / adds verbose_names
Added a federal type field as this did not exist before

Context for reviewers

This PR excludes the domain members and domain groups sections, as that will be implemented later. I've included functions that can be used to easily include those.

One difference from the view and this is that the federal type (executive, etc) is readonly for analysts only, but not for superusers as you can't actually add data to that field otherwise. This can be changed to be readonly for everyone, though. cc: @abroddrick - is this OK?

Setup

Create a portfolio or use an existing one (example data exists on getgov-za)
Add this portfolio record to suborganizations, domain requests, and domain information objects

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Met the acceptance criteria, or will meet them in a subsequent PR
Created/modified automated tests
Added at least 2 developers as PR reviewers (only 1 will need to approve)
Messaged on Slack or in standup to notify the team that a PR is ready for review
Changes to “how we do things” are documented in READMEs and or onboarding guide
If any model was updated to modify/add/delete columns, makemigrations was ran and the associated migrations file has been commited.

Ensured code standards are met (Original Developer)

All new functions and methods are commented using plain language
Did dependency updates in Pipfile also get changed in requirements.txt?
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Add at least 1 designer as PR reviewer

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Pulled this branch locally and tested it
Reviewed this code and left comments
Checked that all code is adequately covered by tests
Made it clear which comments need to be addressed before this work is merged
If any model was updated to modify/add/delete columns, makemigrations was ran and the associated migrations file has been commited.

Ensured code standards are met (Code reviewer)

All new functions and methods are commented using plain language
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values
(Rarely needed) Did dependency updates in Pipfile also get changed in requirements.txt?

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist

As a designer reviewer, I have

Verified that the changes match the design intention

Checked that the design translated visually
Checked behavior
Checked different states (empty, one, some, error)
Checked for landmarks, page heading structure, and links
Tried to break the intended flow

Validated user-facing changes as a designer

Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers (check off which ones were used)
- Chrome
- Microsoft Edge
- FireFox
- Safari
(Rarely needed) Tested as both an analyst and applicant user

Screenshots

Remove the content for members

github-actions · 2024-08-13T21:05:44Z

🥳 Successfully deployed to developer sandbox za.

src/registrar/admin.py

github-actions · 2024-08-13T21:28:08Z

🥳 Successfully deployed to developer sandbox za.

zandercymatics · 2024-08-14T14:25:16Z

src/registrar/templates/django/admin/includes/contact_detail_list.html

+        </br>
    {% endif %}
-    </br>



This was a bug with our existing detail list

zandercymatics · 2024-08-14T14:27:06Z

src/registrar/templates/django/admin/portfolio_change_form.html

-{% block after_related_objects %}
-    <div class="module aligned padding-3">


@CocoByte I had to remove this to match the mockup, sadly! Was this section of code going to be used elsewhere as well? I don't want to throw all of this away unless I have to

I think this is copied over from user_change_form. I don't like that we're doing the same thing in 2 different ways now, should probably bring this up to Alysia and design.

Nah its separate. This code added the following design below:

I do agree though that there is an inconsistency though. @abroddrick the design on this miro does conflict with the design on this older ticket, is this what we want?

I don't like that we're doing the same thing in 2 different ways now

@rachidatecs Can you elaborate more on what you mean here? Just want to make sure I am interpreting you correctly

Same desire for elaboration. But I will say our goals with portfolio is very different than the goal for having domains/requests show on a User.

We're using that deleted markup to display related info on user, we're currently using a collapsible fieldset on portfolio (which I understand will change again) - that's it.

zandercymatics · 2024-08-14T14:30:28Z

src/registrar/templates/django/admin/portfolio_change_form.html

+{% block field_sets %}
+    {% for fieldset in adminform %}


Left a placeholder here as the direct follow-on ticket is going to use this view. It doesn't make sense to remove then add this again

github-actions · 2024-08-14T14:35:32Z

🥳 Successfully deployed to developer sandbox za.

github-actions · 2024-08-14T15:36:08Z

🥳 Successfully deployed to developer sandbox za.

src/registrar/models/senior_official.py

zandercymatics · 2024-08-14T15:50:37Z

src/registrar/models/user.py

-        related_name="user",
+        related_name="portfolio_users",


Needed for the upcoming additions of portfolio members

github-actions · 2024-08-14T16:21:55Z

🥳 Successfully deployed to developer sandbox za.

github-actions · 2024-08-14T20:48:18Z

🥳 Successfully deployed to developer sandbox za.

github-actions · 2024-08-14T20:52:21Z

🥳 Successfully deployed to developer sandbox za.

allly-b · 2024-08-14T21:41:04Z

@zandercymatics no one should change the federal branch by changing it on a portfolio, that changes it for every portfolio in that agency. I wouldn't even want superusers to do that while on this page. They should go to the federal agency page, OR change the federal agency

allly-b · 2024-08-14T21:42:55Z

@zandercymatics we don't need all this info under creator:
Verified by Login.gov
Approved domains: 1
Active requests: 4
Rejected requests: 1

The creator will always be a CISA/Fed employee or our System. All that other info should be empty for most cases

allly-b · 2024-08-14T21:46:28Z

Hey for the show hide on domains, we should have the show/hide under the "domains:" and then another under "requests" not have it all together. Also above the show hide, but below the title can you show the total number of domains and then show the total number of requests above the show hide on domain requests?

Nevermind on the above, I just found an old note that we want to change this view to be a link to a filtered view of the domains table and the domain request table respectively. Ignore that for this ticket and don't worry about making any more changes for the domains/domain requests sections.

github-actions · 2024-08-15T15:03:26Z

🥳 Successfully deployed to developer sandbox za.

github-actions · 2024-08-15T15:38:35Z

🥳 Successfully deployed to developer sandbox za.

github-actions · 2024-08-15T16:17:26Z

🥳 Successfully deployed to developer sandbox za.

github-actions · 2024-08-15T16:21:12Z

🥳 Successfully deployed to developer sandbox za.

github-actions · 2024-08-15T16:39:35Z

🥳 Successfully deployed to developer sandbox za.

zandercymatics · 2024-08-15T16:41:52Z

@rachidatecs / @abroddrick Got those changes in

src/registrar/admin.py

github-actions · 2024-08-15T17:14:31Z

🥳 Successfully deployed to developer sandbox za.

rachidatecs · 2024-08-15T17:27:00Z

src/registrar/templatetags/custom_filters.py

@@ -159,3 +159,13 @@ def and_filter(value, arg):
    Usage: {{ value|and:arg }}
    """
    return bool(value and arg)
+
+
+@register.filter(name="has_contact_info")


Why filter?

This comment by Alysia

zandercymatics · 2024-08-16T14:05:27Z

Addressed!

rachidatecs · 2024-08-16T15:18:58Z

src/registrar/admin.py

+
+    created_on.short_description = "Created on"  # type: ignore
+
+    def portfolio_type(self, obj: models.Portfolio):


Blocking question: I don't understand the difference / inter-relationships between portfolio type, organization type, federal type, federal agency

I do think we need to clamp down on these concepts at some point. I've listed descriptions of each. This may be information you already know, though, but hopefully it helps to have it written out like this? Lmk if you wanted something different.

Basically, portfolio_type and federal_type are needed to fulfill the AC and design of this

portfolio_type

Combination of Portfolio.organization_type (Federal, city) and Portfolio.federal_agency.federal_type (Executive, legislative)

The FederalAgency table now contains all definitions for all federal_agency values

Note: The AC specifically prohibits making
portfolio_type a DB field and notes that this should be a property. Personally, I think just representing it at the admin.py level though would make sense but that depends on @abroddrick

organization type

This is the same as generic_org_type. Basically, it can be Federal, City, Tribal, etc

federal_type

This is self.federal_agency.federal_type (executive, legislative)

Only describes org types of Federal, basically.

Note: As mentioned by Alysia, this should only be changeable from the FederalAgency table. Do you think this should be a db field anyway (w/ custom save logic)? Right now its just a property

federal_agency

This is the FederalAgency model. See screenshots

rachidatecs · 2024-08-16T16:55:36Z

src/registrar/assets/sass/_theme/_admin.scss

@@ -847,3 +847,8 @@ div.dja__model-description{
        }
    }
 }
+
+ul.add-list-reset {


May not be needed, look at add-list-reset: https://designsystem.digital.gov/utilities/list-reset/

zandercymatics added 5 commits August 13, 2024 12:29

stub

4243d19

Stub pt 2

ceabc16

Cleanup

9733fde

Further cleanup

5f0b342

Remove items not part of ACs

ba1392e

Remove the content for members

zandercymatics requested a review from allly-b as a code owner August 13, 2024 20:29

zandercymatics added 2 commits August 13, 2024 14:35

cleanup

fa28412

Linting

78354b6

zandercymatics commented Aug 13, 2024

View reviewed changes

src/registrar/admin.py Outdated Show resolved Hide resolved

Update src/registrar/admin.py

40c52c4

zandercymatics commented Aug 14, 2024

View reviewed changes

Add comments

ffb7e14

Finishing touches

fdc5236

rachidatecs self-assigned this Aug 14, 2024

zandercymatics commented Aug 14, 2024

View reviewed changes

src/registrar/models/senior_official.py Outdated Show resolved Hide resolved

zandercymatics commented Aug 14, 2024

View reviewed changes

Add simple unit tests

a847b07

Add type ignore

21ee9ad

zandercymatics changed the title ~~(Draft) Ticket #2520: Admin portfolio view~~ (on getgov-bob) Ticket #2520: Admin portfolio view Aug 14, 2024

zandercymatics requested review from therealslimhsiehdy, Matt-Spence, CocoByte and rachidatecs August 14, 2024 18:23

zandercymatics added 2 commits August 14, 2024 14:44

fix tests

a2e3841

css fix

7f37bf5

lint

6a34363

zandercymatics requested a review from rachidatecs August 14, 2024 20:51

Suggestions + QOL changes

9059ce3

Make federal_type readonly

06b66f4

zandercymatics added 2 commits August 15, 2024 10:12

use filter on address

cc1dd66

linting

52e3fb8

Update admin.py

e92b0c5

rachidatecs reviewed Aug 15, 2024

View reviewed changes

src/registrar/admin.py Outdated Show resolved Hide resolved

Use built in uswds class

fc138b5

rachidatecs reviewed Aug 15, 2024

View reviewed changes

rachidatecs reviewed Aug 16, 2024

View reviewed changes

rachidatecs approved these changes Aug 16, 2024

View reviewed changes

zandercymatics changed the title ~~(on getgov-bob) Ticket #2520: Admin portfolio view - [migrations]~~ Ticket #2520: Admin portfolio view Aug 16, 2024

zandercymatics merged commit 4f7414f into main Aug 16, 2024
12 checks passed

zandercymatics deleted the za/2520-admin-portfolio-view branch August 16, 2024 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ticket #2520: Admin portfolio view #2581

Ticket #2520: Admin portfolio view #2581

zandercymatics commented Aug 13, 2024 •

edited

Loading

github-actions bot commented Aug 13, 2024

github-actions bot commented Aug 13, 2024

zandercymatics Aug 14, 2024

zandercymatics Aug 14, 2024

rachidatecs Aug 14, 2024

zandercymatics Aug 14, 2024

zandercymatics Aug 14, 2024

allly-b Aug 14, 2024

rachidatecs Aug 15, 2024

zandercymatics Aug 14, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

zandercymatics Aug 14, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

allly-b commented Aug 14, 2024 •

edited

Loading

allly-b commented Aug 14, 2024

allly-b commented Aug 14, 2024 •

edited

Loading

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

zandercymatics commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

rachidatecs Aug 15, 2024

zandercymatics Aug 15, 2024

zandercymatics commented Aug 16, 2024 •

edited

Loading

rachidatecs Aug 16, 2024

zandercymatics Aug 16, 2024 •

edited

Loading

rachidatecs Aug 16, 2024

		{% block after_related_objects %}
		<div class="module aligned padding-3">


		created_on.short_description = "Created on" # type: ignore

		def portfolio_type(self, obj: models.Portfolio):

Ticket #2520: Admin portfolio view #2581

Ticket #2520: Admin portfolio view #2581

Conversation

zandercymatics commented Aug 13, 2024 • edited Loading

Ticket

Changes

Context for reviewers

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Ensured code standards are met (Original Developer)

Validated user-facing changes (if applicable)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Ensured code standards are met (Code reviewer)

Validated user-facing changes as a developer

As a designer reviewer, I have

Verified that the changes match the design intention

Validated user-facing changes as a designer

Screenshots

github-actions bot commented Aug 13, 2024

github-actions bot commented Aug 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

Choose a reason for hiding this comment

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

allly-b commented Aug 14, 2024 • edited Loading

allly-b commented Aug 14, 2024

allly-b commented Aug 14, 2024 • edited Loading

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

zandercymatics commented Aug 15, 2024

github-actions bot commented Aug 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zandercymatics commented Aug 16, 2024 • edited Loading

Choose a reason for hiding this comment

zandercymatics Aug 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zandercymatics commented Aug 13, 2024 •

edited

Loading

allly-b commented Aug 14, 2024 •

edited

Loading

allly-b commented Aug 14, 2024 •

edited

Loading

zandercymatics commented Aug 16, 2024 •

edited

Loading

zandercymatics Aug 16, 2024 •

edited

Loading