Ticket #2484: Dynamic portfolio fields #2548

zandercymatics · 2024-08-07T17:15:57Z

Ticket

Resolves #2484

Changes

Selecting federal agency dynamically changes organization type to federal.
The Federal Agency dynamically changes the SO to be the SO for that Agency, if it exists in the table.
- You either must run the load_senior_official_table script for this to work, as no data exists otherwise - or create dummy federal agencies
The urbanization field on dynamically shows if the user selects PR, otherwise it does not show.
Adds some error handling in the action needed dropdown javascript function (was causing issues on this page)

Context for reviewers

This PR attaches some javascript logic to the Portfolio page.

It does the following:

The urbanization field is hidden when not Puerto Rico, and shown otherwise
When a federal_agency is selected, the organization_type will switch to "federal" if it is not empty or "Non-Federal Agency". Otherwise, if "Non-Federal Agency" is selected, type federal will be deselected.
When a federal_agency is selected, the senior official on the underlying federal_agency will auto-populate
- This will only work if you run the load_senior_officials script

Note:
As noted, this ticket automatically populates the senior_official and organization_type fields based off of the current federal_agency selection.

Due to the nature of the data, senior_official required an admin-only API that returns the field id based off of the federal agency name. This is accessible to analysts and superusers, though do note that the portfolio page isn't yet visible to analysts yet (as that comes down the road).

Setup

Run the load_senior_officials_table script. Instructions here.
Add a portfolio in /admin and modify the state field, and the federal_agency field.

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

github-actions · 2024-08-07T17:20:04Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-07T20:03:31Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-08T15:21:35Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-08T15:57:50Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-08T16:25:37Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-08T16:37:14Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-08T16:43:16Z

🥳 Successfully deployed to developer sandbox meoward.

src/registrar/assets/js/get-gov-admin.js

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

src/registrar/tests/test_api.py

Co-authored-by: Matt-Spence <[email protected]>

github-actions · 2024-08-12T14:25:18Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-12T14:49:13Z

🥳 Successfully deployed to developer sandbox meoward.

github-actions · 2024-08-12T15:15:46Z

🥳 Successfully deployed to developer sandbox meoward.

zandercymatics · 2024-08-13T14:04:25Z

@Matt-Spence Those changes are ready!

Matt-Spence

Nice changes

rachidatecs

Will run code and finalize review next.

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

rachidatecs · 2024-08-14T15:54:29Z

src/registrar/views/utility/api_views.py

+
+
+@login_required
+@staff_member_required


This seems redundant with the superuser and analyst checks you're doing below. That's not a bad thing, but I'm wondering why you bothered with the decorator?

This decorator redirects you to the admin login page and basically does the default django checks for admin perms. Tiny QOL thing, otherwise you do get blocked but you can technically still """access""" the endpoint

rachidatecs · 2024-08-14T15:55:29Z

src/registrar/models/portfolio.py

+        """Save override for custom properties"""
+
+        # The urbanization field is only intended for the state_territory puerto rico
+        if self.state_territory != self.StateTerritoryChoices.PUERTO_RICO and self.urbanization:


This needs a unit test

rachidatecs · 2024-08-14T15:58:01Z

src/registrar/assets/js/get-gov-admin.js


    let emailWasSent = document.getElementById("action-needed-email-sent");
+    if (!emailWasSent) {


Line 534 should be handling this and !actionNeededEmail || !actionNeededReasonDropdown cases

I added these because this script was failing on the portfolio page, causing script execution to stop (on textContent because the query returned none). That said I can simplify these checks to just check on if action-needed-emails-data exists or not before trying to do .textContent

src/registrar/assets/js/get-gov-admin.js

rachidatecs · 2024-08-14T16:00:14Z

src/registrar/assets/js/get-gov-admin.js

+        }
+
+        // $ symbolically denotes that this is using jQuery
+        let $federalAgency = django.jQuery("#id_federal_agency");


Q: Why use django.JQuery instead of getElement or querySelector?

Ah, its because everywhere else in the app federal agency is a select2 (autocomplete field). The portfolio page is still undergoing some work - so its likely that this will be added to that as well. Doing it this way gives us out of the box support so we don't have to alter this script when we do.

For context: you can't do .value on Select2 objects. You can only manipulate them through jQuery, its a quirk of that particular library. Went down that rabbit hole a few tickets ago

src/registrar/assets/js/get-gov-admin.js

github-actions · 2024-08-14T16:58:27Z

🥳 Successfully deployed to developer sandbox meoward.

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

🥳 Successfully deployed to developer sandbox meoward.

zandercymatics · 2024-08-14T17:21:19Z

@rachidatecs Got your changes in. Pushing them up to litterbox now

github-actions · 2024-08-14T17:23:20Z

🥳 Successfully deployed to developer sandbox meoward.

rachidatecs · 2024-08-14T17:39:50Z

What does that mean?

"Otherwise, if "Non-Federal Agency" is selected, type federal will be deselected."

zandercymatics · 2024-08-14T17:44:40Z

@rachidatecs Basically, the organization_type field will be set to federal automatically UNLESS its Non-Federal Agency. If it is, it'll set the field to None (assuming it isn't another value like County or something)

github-actions · 2024-08-14T18:36:49Z

🥳 Successfully deployed to developer sandbox meoward.

Update get-gov-admin.js

1d6981f

zandercymatics requested a review from abroddrick as a code owner August 7, 2024 17:15

zandercymatics changed the title ~~(Draft) Ticket #2484: Dynamic portfolio fields~~ (Draft) (on-getgov-meoward) Ticket #2484: Dynamic portfolio fields Aug 7, 2024

Merge branch 'main' into meoward/2484-dynamic-portfolio-fields

b092fc6

Add API to select SO information

a2f87c7

Add unit tests for api

e90ef9b

zandercymatics added 2 commits August 8, 2024 10:21

Fix perms

ee7b886

Linting

ac6561d

Cleanup tests

32630ad

Update test_api.py

ea47a7a

zandercymatics changed the title ~~(Draft) (on-getgov-meoward) Ticket #2484: Dynamic portfolio fields~~ (on-getgov-meoward) Ticket #2484: Dynamic portfolio fields Aug 8, 2024

zandercymatics requested review from therealslimhsiehdy, Matt-Spence, CocoByte, rachidatecs, dave-kennedy-ecs, erinysong and asaki222 August 8, 2024 17:06

Matt-Spence self-assigned this Aug 9, 2024

Matt-Spence reviewed Aug 9, 2024

View reviewed changes

src/registrar/assets/js/get-gov-admin.js Outdated Show resolved Hide resolved

Matt-Spence reviewed Aug 9, 2024

View reviewed changes

src/registrar/assets/js/get-gov-admin.js Show resolved Hide resolved

Matt-Spence reviewed Aug 9, 2024

View reviewed changes

src/registrar/templates/django/admin/portfolio_change_form.html Outdated Show resolved Hide resolved

Matt-Spence reviewed Aug 9, 2024

View reviewed changes

src/registrar/tests/test_api.py Show resolved Hide resolved

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

47779f8

Co-authored-by: Matt-Spence <[email protected]>

zandercymatics added 2 commits August 12, 2024 08:44

Add PR suggestions

44b8163

Linting and cleanup

0feb0a7

zandercymatics changed the title ~~(on-getgov-meoward) Ticket #2484: Dynamic portfolio fields~~ (on getgov-litterbox) Ticket #2484: Dynamic portfolio fields Aug 12, 2024

zandercymatics requested a review from Matt-Spence August 12, 2024 14:53

Check for api response codes when resetting values

b89ae53

Matt-Spence approved these changes Aug 13, 2024

View reviewed changes

rachidatecs self-assigned this Aug 14, 2024

rachidatecs reviewed Aug 14, 2024

View reviewed changes

PR suggestions (minus unit test)

413bb53

PR suggestions - unit test

8081e92

zandercymatics requested a review from rachidatecs August 14, 2024 17:11

Add url logic to domain requests and domain table

2c4c9dd

rachidatecs approved these changes Aug 14, 2024

View reviewed changes

zandercymatics changed the title ~~(on getgov-litterbox) Ticket #2484: Dynamic portfolio fields~~ Ticket #2484: Dynamic portfolio fields Aug 14, 2024

input => span

67cd368

zandercymatics merged commit 9a55ade into main Aug 14, 2024
10 checks passed

zandercymatics deleted the meoward/2484-dynamic-portfolio-fields branch August 14, 2024 18:53

zandercymatics mentioned this pull request Aug 16, 2024

On portfolio add/update pages show "Federal type" and "Portfolio type" dynamically #2608

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ticket #2484: Dynamic portfolio fields #2548

Ticket #2484: Dynamic portfolio fields #2548

zandercymatics commented Aug 7, 2024 •

edited

Loading

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 12, 2024

github-actions bot commented Aug 12, 2024

github-actions bot commented Aug 12, 2024

zandercymatics commented Aug 13, 2024

Matt-Spence left a comment

rachidatecs left a comment

rachidatecs Aug 14, 2024

zandercymatics Aug 14, 2024 •

edited

Loading

rachidatecs Aug 14, 2024

rachidatecs Aug 14, 2024

zandercymatics Aug 14, 2024 •

edited

Loading

rachidatecs Aug 14, 2024

zandercymatics Aug 14, 2024 •

edited

Loading

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

zandercymatics commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

rachidatecs commented Aug 14, 2024

zandercymatics commented Aug 14, 2024

github-actions bot commented Aug 14, 2024


		let emailWasSent = document.getElementById("action-needed-email-sent");
		if (!emailWasSent) {



		@login_required
		@staff_member_required

Ticket #2484: Dynamic portfolio fields #2548

Ticket #2484: Dynamic portfolio fields #2548

Conversation

zandercymatics commented Aug 7, 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 7, 2024

github-actions bot commented Aug 7, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 8, 2024

github-actions bot commented Aug 12, 2024

github-actions bot commented Aug 12, 2024

github-actions bot commented Aug 12, 2024

zandercymatics commented Aug 13, 2024

Matt-Spence left a comment

Choose a reason for hiding this comment

rachidatecs left a comment

Choose a reason for hiding this comment

rachidatecs Aug 14, 2024

Choose a reason for hiding this comment

zandercymatics Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

rachidatecs Aug 14, 2024

Choose a reason for hiding this comment

rachidatecs Aug 14, 2024

Choose a reason for hiding this comment

zandercymatics Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

rachidatecs Aug 14, 2024

Choose a reason for hiding this comment

zandercymatics Aug 14, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

zandercymatics commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

rachidatecs commented Aug 14, 2024

zandercymatics commented Aug 14, 2024

github-actions bot commented Aug 14, 2024

zandercymatics commented Aug 7, 2024 •

edited

Loading

zandercymatics Aug 14, 2024 •

edited

Loading

zandercymatics Aug 14, 2024 •

edited

Loading

zandercymatics Aug 14, 2024 •

edited

Loading