#2351: Org requests page - [RJM] #2720

zandercymatics · 2024-09-06T14:55:27Z

Ticket

Resolves #2351

Joint ticket between @dave-kennedy-ecs / @rachidatecs / @zandercymatics

Changes

Adds a portfolio domain requests page with the following changes:
- Adds a dropdown button for the domain requests button (toggleable depending on permissions)
- Adds a "creator" field to the table
- Replaces the delete button with an expandable "three dots" icon (see this figma)
- View / Manage logic like on the domains page
- Show/hide the "Start a new domain request" button depending on permissions
- Changed the API to return only domain requests associated with that portfolio
- Automatically added new domain requests created through the portfolio page to that portfolio
Added a "no organization requests" page (like our no domains page)
Small refactor to our portfolio permission functions on user to work with this new flow
Removed the VIEW_CREATED_REQUESTS permission and used EDIT_REQUESTS instead
Refactored our middleware to simplify adding portfolios + fix session retention bugs

Context for reviewers

In a nutshell, this PR adds a domain request page that is similar to the domain page on portfolios.
As part of it, we will eventually need to add readonly versions of domain requests for members that do not have permissions to edit created requests. However, that will be addressed in a follow-on ticket. For now, clicking view on any request that you do not currently manage will return an error.

Like the domains page, the domain request table will return all domain requests within said portfolio. If you have the EDIT_REQUESTS permission, you can: 1) create new domain requests, 2) manage your created domain requests. You cannot modify requests that were not created by your user.

If you only have the VIEW_ALL_REQUESTS permission, you should not see the start a new domain request button - and all manage links should change to view.

Likewise, if you are missing both of those requests - you will be presented with a no access page like the one we have for domains.

Important: This PR also partially resolves a session bug wherein the session would not updated when the underlying portfolio was updated. This will be refined further, but this is something to note.

Setup

You will need:

A portfolio
The following flags enabled: organization_feature, organization_requests
Your user as a member user with the additional roles EDIT_REQUESTS (Called Create and edit requests in /admin) and VIEW_ALL_REQUESTS (Called View all requests in /admin)
Some domain requests with your portfolio added to them

You will need to test:

Navigating to the domain requests page + verifying that the dropdown button links work
Starting + finishing a new request (should be automatically added to the given portfolio)
Verifying that you cannot create a new request without the role EDIT_REQUESTS
Verifying that you see the no permissions page when removing EDIT_REQUESTS and VIEW_ALL_REQUESTS
Verifying that the domain request table only displays domain requests on that portfolio

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

…sions

Not needed in this PR

zandercymatics · 2024-09-06T16:29:17Z

src/registrar/templates/includes/domain_requests_table.html

+            <th data-sortable="last_submitted_date" scope="col" role="columnheader">Submitted</th>
+            {% if portfolio %}
+              <th data-sortable="creator" scope="col" role="columnheader">Created by</th>
+            {% endif %}


@rachidatecs / @dave-kennedy-ecs do we need a test for this? I'm leaning toward no

It would be nice

github-actions · 2024-09-06T16:59:37Z

🥳 Successfully deployed to developer sandbox rjm.

github-actions · 2024-09-06T17:51:24Z

🥳 Successfully deployed to developer sandbox rjm.

github-actions · 2024-09-06T18:16:24Z

🥳 Successfully deployed to developer sandbox rjm.

github-actions · 2024-09-06T20:50:04Z

🥳 Successfully deployed to developer sandbox rjm.

github-actions · 2024-09-10T19:36:43Z

🥳 Successfully deployed to developer sandbox rjm.

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

asaki222 · 2024-09-10T20:27:47Z

src/registrar/models/user.py

-        request.user = self
-        has_organization_members_flag = flag_is_active(request, "organization_members")
-        if not has_organization_members_flag:
+        if not self.has_organization_members_flag():
            return False


This is great that the checking for flag are moved into a method! :-)

I second this :D

CocoByte

Most of my comments are truly on the minutia. Overall solid work!

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

CocoByte · 2024-09-11T02:42:59Z

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

@@ -1817,6 +1884,36 @@ document.addEventListener('DOMContentLoaded', function() {
      });
    }

+    function closeMoreActionMenu(accordionIsOpen) {
+      if (accordionIsOpen.getAttribute("aria-expanded") === "true") {
+        accordionIsOpen.click();


nitpick: "accordionIsOpen" looks like a boolean var...but it's clickable so clearly this is referring to an actual DOM element. Might want to rename to reflect its true identity. (eg. "openAccordionButton"? Like what you have later on?)

What about accordionThatIsOpen? Since it grabs an open accordion element

cc: @rachidatecs

We'll tackle renaming in the next ticket (search)

CocoByte · 2024-09-11T02:45:25Z

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

+      }
+    }
+
+    document.addEventListener('focusin', function(event) {


(non-blocking comment)--I'm surprised this isn't already built into the controls....are we over-customizing?

Just making a guess but I think its due to how the default accordion works. I know this work was done before -- @rachidatecs did you have to do some heavy customization of this element to get it to behave this way?

src/registrar/templates/domain_request_form.html

src/registrar/templates/includes/domain_requests_table.html

src/registrar/templates/includes/header_extended.html

src/registrar/views/domain_requests_json.py

Co-authored-by: CuriousX <nicolle.leclair@gmail.com>

github-actions · 2024-09-11T15:12:15Z

🥳 Successfully deployed to developer sandbox rjm.

Co-authored-by: CuriousX <nicolle.leclair@gmail.com>

github-actions · 2024-09-11T15:28:32Z

🥳 Successfully deployed to developer sandbox rjm.

github-actions · 2024-09-11T15:37:29Z

🥳 Successfully deployed to developer sandbox rjm.

…e.get.gov into rjm/2351-org-requests-page

github-actions · 2024-09-11T15:51:26Z

🥳 Successfully deployed to developer sandbox rjm.

CocoByte

LGTM so far. I'll let Rachid/Dave make final comments on the pieces they have been tagged in

CocoByte · 2024-09-11T23:12:55Z

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

+  }
+  .visible-mobile-flex {
+    display: none!important;
+  }


Ah, thanks for the context

github-actions · 2024-09-12T16:56:50Z

🥳 Successfully deployed to developer sandbox rjm.

asaki222 · 2024-09-12T17:23:57Z

This overall lgtm! I do have a question. Am I supposed to see the delete button when I log in with an account that has View all requests?
Here are my screenshots:

This is the view under my admin account with Create and edit requests,

and this is my non admin account

zandercymatics · 2024-09-12T18:35:04Z

@asaki222 This is expected! You should see the edit button with create and edit requests. However when you do not have that permission, you should not see the save button. Are you seeing that behavior?

CocoByte · 2024-09-12T19:04:43Z

@asaki222 This is expected! You should see the edit button with create and edit requests. However when you do not have that permission, you should not see the save button. Are you seeing that behavior?

I verified this behavior. Approving!

CocoByte

LGTM

asaki222 · 2024-09-12T19:07:52Z

LGTM!

github-actions · 2024-09-12T20:23:04Z

🥳 Successfully deployed to developer sandbox rjm.

rachidatecs and others added 15 commits September 5, 2024 00:13

wip

be8a618

refactored domain_requests_json

ba20e74

last row accordion placement

8f8d9a8

pull

bc2ae12

merge main:

ec7202b

fix bug with registrar and basic redirect link stuff

9bd67cf

Add created by column

d802b9a

pull

2939c89

Finishing or canceling a request redirects to domain request view

3c3fd26

CSS tweaks

9ea0f31

pull

b4dd786

filter requests based on permissions, updated actions based on permis…

faf5790

…sions

merged pulled updates

b4827ff

migrations file

6a08c1e

Comment out breadcrumb bar

ea75440

Not needed in this PR

zandercymatics requested a review from abroddrick as a code owner September 6, 2024 14:55

zandercymatics changed the title ~~(ignore)~~ (draft) #2351: Org requests page Sep 6, 2024

Merge branch 'main' into rjm/2351-org-requests-page

bb7a521

zandercymatics commented Sep 6, 2024

View reviewed changes

rachidatecs added 2 commits September 6, 2024 12:54

CSS done

cbdf763

merge

49e6560

cleanup

1e9ae7b

bug fix

7a67845

model and view unit tests

1af08a6

dave-kennedy-ecs and others added 2 commits September 6, 2024 17:27

requests_json tests and some linting

9f557fc

Fix existing tests

9019792

asaki222 reviewed Sep 10, 2024

View reviewed changes

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

asaki222 reviewed Sep 10, 2024

View reviewed changes

CocoByte self-assigned this Sep 11, 2024

CocoByte reviewed Sep 11, 2024

View reviewed changes

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

663fc99

Co-authored-by: CuriousX <nicolle.leclair@gmail.com>

zandercymatics and others added 3 commits September 11, 2024 09:24

Update src/registrar/templates/includes/header_extended.html

a0c73ca

Co-authored-by: CuriousX <nicolle.leclair@gmail.com>

Update src/registrar/views/domain_requests_json.py

87cb111

Co-authored-by: CuriousX <nicolle.leclair@gmail.com>

PR suggestions

096dce7

Merge branch 'rjm/2351-org-requests-page' of github.com:cisagov/manag…

d7b851e

…e.get.gov into rjm/2351-org-requests-page

CocoByte reviewed Sep 11, 2024

View reviewed changes

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

}

.visible-mobile-flex {

display: none!important;

}

Copy link

Contributor

CocoByte Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks for the context

rachidatecs changed the title ~~Issue #2351: Org requests page [getgov-rjm]~~ #2351: Org requests page - [RJM] Sep 12, 2024

unit tests for the extra column in the template

16fc446

CocoByte self-requested a review September 12, 2024 19:04

CocoByte approved these changes Sep 12, 2024

View reviewed changes

asaki222 approved these changes Sep 12, 2024

View reviewed changes

Merge branch 'main' into rjm/2351-org-requests-page

4124ad1

zandercymatics merged commit ccd1d0f into main Sep 12, 2024
10 checks passed

zandercymatics deleted the rjm/2351-org-requests-page branch September 12, 2024 20:27

zandercymatics mentioned this pull request Sep 13, 2024

Portfolio domain requests: Don't show delete button when the user doesn't have permissions, and fix spacing for the domain table #2754

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2351: Org requests page - [RJM] #2720

#2351: Org requests page - [RJM] #2720

zandercymatics commented Sep 6, 2024 •

edited

Loading

zandercymatics Sep 6, 2024 •

edited

Loading

rachidatecs Sep 12, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 10, 2024

asaki222 Sep 10, 2024

CocoByte Sep 11, 2024

CocoByte left a comment

CocoByte Sep 11, 2024

zandercymatics Sep 11, 2024

rachidatecs Sep 12, 2024

CocoByte Sep 11, 2024 •

edited

Loading

zandercymatics Sep 11, 2024

rachidatecs Sep 12, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

CocoByte left a comment

CocoByte Sep 11, 2024

github-actions bot commented Sep 12, 2024

asaki222 commented Sep 12, 2024 •

edited

Loading

zandercymatics commented Sep 12, 2024 •

edited

Loading

CocoByte commented Sep 12, 2024

CocoByte left a comment

asaki222 commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

#2351: Org requests page - [RJM] #2720

#2351: Org requests page - [RJM] #2720

Conversation

zandercymatics commented Sep 6, 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

zandercymatics Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 6, 2024

github-actions bot commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CocoByte left a 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

CocoByte Sep 11, 2024 • edited Loading

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 Sep 11, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

github-actions bot commented Sep 11, 2024

CocoByte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 12, 2024

asaki222 commented Sep 12, 2024 • edited Loading

zandercymatics commented Sep 12, 2024 • edited Loading

CocoByte commented Sep 12, 2024

CocoByte left a comment

Choose a reason for hiding this comment

asaki222 commented Sep 12, 2024

github-actions bot commented Sep 12, 2024

zandercymatics commented Sep 6, 2024 •

edited

Loading

zandercymatics Sep 6, 2024 •

edited

Loading

CocoByte Sep 11, 2024 •

edited

Loading

asaki222 commented Sep 12, 2024 •

edited

Loading

zandercymatics commented Sep 12, 2024 •

edited

Loading