#2854 - Org Member Invitation - [NL] #3080

CocoByte · 2024-11-14T17:16:16Z

Ticket 2854

Resolves #2854

Changes

Added confirmation modal to the Invite Member form
Added handlers for submitting form information and writing to the database
Added basic unit tests to verify form submission (these tests will need to be updated once e-mail sending function is implemented)

Context for reviewers

Setup

Make sure org model and member waffle flags are on

Code Review Verification Steps

Go to the members table, click "Add a new member". Fill out the form and click "Invite Member". Verify that:

A confirmation modal shows that contains all the info you filled out on the form
Submitting the modal (clicking "Yes, invite member") submits the form data, redirects to the member table, and writes to the database accordingly. There are three scenarios to test...
- [ ] Inviting a new user creates a new corresponding Portfolio Invite (that you can view in /admin)
- [ ] Inviting an already invited user does not write to the database and a message saying the user "has already been invited to this portfolio" displays above the members table after redirect
- [ ] Inviting a user who is already a portfolio member does not write to the database and a message saying the user "User is already a member of this portfolio." displays above the members table after redirect

Also double-check that:

Trying to submit the new member invitation form with errors (blanks or invalid data) does NOT display a modal and instead displays corresponding error messages

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
Update documentation in READMEs and/or onboarding guide

Ensured code standards are met (Original Developer)

If any updated dependencies on Pipfile, also update dependencies 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)

Tag @dotgov-designers in this PR's Reviewers for design review. If code is not user-facing, delete design reviewer checklist
Verify 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)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Pulled this branch locally and tested it
Verified code meets all checks above. Address any checks that are not satisfied
Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
Checked that all code is adequately covered by tests
Verify migrations are valid and do not conflict with existing migrations

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Meets all designs and user flows provided by design/product
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
(Rarely needed) Tested as both an analyst and applicant user

As a designer reviewer, I have

Verified that the changes match the design intention

Checked that the design translated visually
Checked behavior. Comment any found issues or broken flows.
Checked different states (empty, one, some, error)
Checked for landmarks, page heading structure, and links

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

References

Code review best practices

Screenshots

…vitation

…sing built in functionality

github-actions · 2024-11-18T18:37:29Z

🥳 Successfully deployed to developer sandbox nl.

github-actions · 2024-11-20T04:58:18Z

🥳 Successfully deployed to developer sandbox nl.

github-actions · 2024-11-20T05:14:20Z

🥳 Successfully deployed to developer sandbox nl.

github-actions · 2024-11-20T05:54:28Z

🥳 Successfully deployed to developer sandbox nl.

zandercymatics

UI wise, looks great. I like how you are handling things in javascript. One thing I'll note is that Member Access seems to be using the value rather than the display value.

I will note though that submit_new_member can be simplified + cleaned a bit (I've denoted it). Its fine for now since we aren't sending any emails out, but the current structure will cause some side affects once we do

src/registrar/views/portfolios.py

src/registrar/templates/portfolio_members_add_new.html

zandercymatics · 2024-11-21T18:14:42Z

src/registrar/templates/portfolio_members_add_new.html

+                <p class="margin-top-0" id="modalEmail"></p>
+
+                <h4 class="text-primary">Member Access</h4>
+                <p class="margin-top-0" id="modalAccessLevel"></p>


(nitpick/optional) Maybe better described as modalMemberAccessLevel

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

reinstated js portion of add new member page

github-actions · 2024-11-30T01:28:57Z

🥳 Successfully deployed to developer sandbox nl.

…vitation

CocoByte · 2024-12-02T23:53:36Z

messages.success(self.request, f"{email} has been invited.")

The UI uses the value because that is what matches the design ACs according to the Figma (they do not want "Basic Access" or "Admin Access"...just "Basic" and "Admin". It's a bit of a shortcut, but I think better than stripping out substrings)

github-actions · 2024-12-03T02:26:36Z

🥳 Successfully deployed to developer sandbox nl.

github-actions · 2024-12-03T02:54:19Z

🥳 Successfully deployed to developer sandbox nl.

zandercymatics

See blocking comments before merging but otherwise LGTM on getgov-NL

src/registrar/assets/src/js/getgov/portfolio-member-page.js

zandercymatics · 2024-12-03T18:49:17Z

src/registrar/views/portfolios.py

+        if not requestor.is_staff and requestor.email is not None and requestor.email.strip() != "":
+            requestor_email = requestor.email
+        elif not requestor.is_staff:


(Change before merging) this shouldn't be checking on is_staff since we don't use this anymore, it should instead check on the usergroup

We should definitely create a follow-up ticket for this because we are still using .is_staff in alot more places than just this. @abroddrick --tagging you for awareness

src/registrar/views/portfolios.py

zandercymatics · 2024-12-03T18:57:59Z

src/registrar/views/portfolios.py

+        except Exception:
+            logger.error("An error occured")
+
+        try:


Suggested change

try:

if not add_success:

return None

try:

missing an early return on not add_success. The above suggestion may have some faulty spacing though just fyi

zandercymatics · 2024-12-03T18:58:58Z

src/registrar/views/portfolios.py

+            if add_success:
+                messages.success(self.request, f"{email} has been invited.")


Suggested change

if add_success:

messages.success(self.request, f"{email} has been invited.")

messages.success(self.request, f"{email} has been invited.")

Not needed if we're doing an early return

src/registrar/views/portfolios.py

Co-authored-by: zandercymatics <[email protected]>

github-actions · 2024-12-04T08:24:35Z

🥳 Successfully deployed to developer sandbox nl.

CocoByte added 2 commits November 14, 2024 10:11

modal work...

f1b898f

Merge remote-tracking branch 'origin/main' into nl/2854-org-member-in…

00c7b90

…vitation

CocoByte requested a review from abroddrick as a code owner November 14, 2024 17:16

CocoByte added 3 commits November 14, 2024 14:42

validation working. Just need to clean things up and open the modal u…

eeb5727

…sing built in functionality

modal logic complete. Needs css updates and need to finish backend logic

a94332f

Validation complete, but it is firing twice. Need to fix that

6dc7d6f

CocoByte added 4 commits November 18, 2024 14:18

frontend updates. Code cleanup

cc4a52c

linted

bc8df41

linting, cleanup, initial unit tests

c75b604

resolve merge conflicts

a151f7d

CocoByte changed the title ~~[DRAFT] #2854 - Org Member Invitation~~ #2854 - Org Member Invitation - [NL] Nov 20, 2024

linted again

84bb88a

CocoByte requested review from therealslimhsiehdy, Matt-Spence, rachidatecs, erinysong, asaki222 and zandercymatics November 20, 2024 05:10

cleanup

a64a85b

zandercymatics self-assigned this Nov 21, 2024

zandercymatics requested changes Nov 21, 2024

View reviewed changes

CocoByte changed the title ~~#2854 - Org Member Invitation - [NL]~~ #2854 - Org Member Invitation - [AB] Nov 26, 2024

CocoByte changed the title ~~#2854 - Org Member Invitation - [AB]~~ #2854 - Org Member Invitation - [NL] Nov 26, 2024

CocoByte added 2 commits November 29, 2024 12:03

Merging main (need to reinstate js)

495b8f3

WIP on nl/2854-org-member-invitation

dcdc878

reinstated js portion of add new member page

Merge remote-tracking branch 'origin/main' into nl/2854-org-member-in…

a5499dc

…vitation

Addressed feedback

e0223e1

CocoByte requested a review from zandercymatics December 3, 2024 00:32

remove stray char (keyboard error?)

38a3d38

zandercymatics approved these changes Dec 3, 2024

View reviewed changes

CocoByte and others added 2 commits December 4, 2024 01:20

Update src/registrar/assets/src/js/getgov/portfolio-member-page.js

5293771

Co-authored-by: zandercymatics <[email protected]>

Update src/registrar/views/portfolios.py

2c2f1ab

Co-authored-by: zandercymatics <[email protected]>

CocoByte merged commit ee4a584 into main Dec 4, 2024
9 of 10 checks passed

CocoByte deleted the nl/2854-org-member-invitation branch December 4, 2024 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#2854 - Org Member Invitation - [NL] #3080

#2854 - Org Member Invitation - [NL] #3080

CocoByte commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 18, 2024

github-actions bot commented Nov 20, 2024

github-actions bot commented Nov 20, 2024

github-actions bot commented Nov 20, 2024

zandercymatics left a comment

zandercymatics Nov 21, 2024

github-actions bot commented Nov 30, 2024

CocoByte commented Dec 2, 2024

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

zandercymatics left a comment

zandercymatics Dec 3, 2024

CocoByte Dec 4, 2024

zandercymatics Dec 3, 2024

zandercymatics Dec 3, 2024

github-actions bot commented Dec 4, 2024

-        try:
+        if not add_success:
+            return None
+        try:

		if add_success:
		messages.success(self.request, f"{email} has been invited.")

#2854 - Org Member Invitation - [NL] #3080

#2854 - Org Member Invitation - [NL] #3080

Conversation

CocoByte commented Nov 14, 2024 • edited Loading

Ticket 2854

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

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

References

Screenshots

github-actions bot commented Nov 18, 2024

github-actions bot commented Nov 20, 2024

github-actions bot commented Nov 20, 2024

github-actions bot commented Nov 20, 2024

zandercymatics left a comment

Choose a reason for hiding this comment

zandercymatics Nov 21, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 30, 2024

CocoByte commented Dec 2, 2024

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

zandercymatics left a comment

Choose a reason for hiding this comment

zandercymatics Dec 3, 2024

Choose a reason for hiding this comment

CocoByte Dec 4, 2024

Choose a reason for hiding this comment

zandercymatics Dec 3, 2024

Choose a reason for hiding this comment

zandercymatics Dec 3, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 4, 2024

CocoByte commented Nov 14, 2024 •

edited

Loading