Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Easi 4580/email notifications #2774

Merged
merged 18 commits into from
Aug 23, 2024

Conversation

samoddball
Copy link
Contributor

@samoddball samoddball commented Aug 21, 2024

EASI-4580

Description

  • add Yes/No selection for admins to choose to send notifications when uploading docs

How to test this change

  • nav to admin view of Supporting Documents on a system intake
  • upload a doc
  • confirm you see the below
  • NOTE: the below will never show when initially submitting the system intake as the user creating the intake will always have the requester role (even if the user is an admin)
Screenshot 2024-08-21 at 12 31 20

PR Author Checklist

  • I have provided a detailed description of the changes in this PR.
  • I have provided clear instructions on how to test the changes in this PR.
  • I have updated tests or written new tests as appropriate in this PR.

PR Reviewer Guidelines

  • It's best to pull the branch locally and test it, rather than just looking at the code online!
  • When approving a PR, provide a reason why you're approving it
    • e.g. "Approving because I tested it locally and all functionality works as expected"
    • e.g. "Approving because the change is simple and matches the Figma design"
  • Don't be afraid to leave comments or ask questions, especially if you don't understand why something was done! (This is often a great time to suggest code comments or documentation updates)
  • Check that all code is adequately covered by tests - if it isn't feel free to suggest the addition of tests.

@@ -184,7 +184,7 @@ exports[`Governance Overview Content > renders default version without crashing
<p
class="margin-bottom-1"
>
If your Business Case is approved, you will receive a unique Life Cycle ID. If it is not approved, you will recieve documented next steps or concerns to address in order to proceed.
If your Business Case is approved, you will receive a unique Life Cycle ID. If it is not approved, you will receive documented next steps or concerns to address in order to proceed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

noticed one of these so fixed everywhere

@samoddball samoddball marked this pull request as ready for review August 21, 2024 19:32
@samoddball samoddball requested review from a team as code owners August 21, 2024 19:32
@samoddball samoddball requested review from mynar7 and aterstriep and removed request for a team August 21, 2024 19:32
mynar7
mynar7 previously requested changes Aug 23, 2024
Copy link
Contributor

@mynar7 mynar7 left a comment

Choose a reason for hiding this comment

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

Might be worth adding some test cases for when to send v not send emails

Copy link
Contributor

@aterstriep aterstriep left a comment

Choose a reason for hiding this comment

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

The field needs a few minor changes on the frontend, but overall nice job!

Based on the Figma, the field should be required. It needs an asterisk next to the legend and the validation schema needs to be updated to include this field.

src/views/SystemIntake/Documents/UploadForm.tsx Outdated Show resolved Hide resolved
src/views/SystemIntake/Documents/UploadForm.tsx Outdated Show resolved Hide resolved
src/views/SystemIntake/Documents/UploadForm.tsx Outdated Show resolved Hide resolved
src/views/SystemIntake/Documents/UploadForm.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@mynar7 mynar7 left a comment

Choose a reason for hiding this comment

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

BE changes look good!

@samoddball samoddball merged commit a1f65c8 into feature/EASI-4485 Aug 23, 2024
12 checks passed
@samoddball samoddball deleted the EASI-4580/email_notifications branch August 23, 2024 20:06
mynar7 added a commit that referenced this pull request Sep 6, 2024
* [EASI-4488] Add new document types to intake form (#2748)

* add new document types, adjust form and table, fix minio scripts

* fix cypress, remove commented code

* use translation to enforce enum options

* [EASI-4488] Add permissions to GQL schema for intake documents (#2756)

add permissions to GQL schema for intake documents

* [EASI-4489] System intake document upload form (#2760)

* Add version help text

* Update form to match Figma

* Add type prop to UploadForm

* Admin upload form

* Conditional return link

* Validation schema

* Easi 4580/email notifications (#2774)

* UI for selecting if notification should go out

* add bool check to BE

* fix bool radio

* typo

* add condition

* fix imports

* refs

* use `field.onChange` instead of `setValue`

* trying to fix snapshot

* put snap back

* added UI tests for UploadForm notification display

* add some tests for BE

* fix ref, error condition, text

* updates to validation

* [EASI-4488] GRB Review supporting docs table (#2776)

* update queries to include permissions for docs

* add documents table

* address feedback, update docs table in other places

* update snapshot

* fix failing cypress test

* fix biz case solution rendering and alert padding

* rename migration number

* [EASI-4591] IT Gov admin route updates (#2781)

* governance-review-team -> it-governance

* Update subNavItems + context

* Route updates

* Remove hidden nav links from array

* Unit test fixes

* Unit tests for action buttons

* Redirect for old admin links

* Update url in e2e tests

* Upload form route

* Test file routes

* [EASI-4597] Add software BOM to doc types (#2787)

add software BOM to doc types

* [EASI-4583] GRB review sub navigation links (#2792)

* SideNavigation component

* RequestOverview side nav

- Updated to use SideNavigation component
- Added Supporting Documents and Participants links

* Child nav link functionality

* AccordionNavigation updates

* Remove unused CSS

* Snapshot

Accordion navigation button no longer renders on desktop

* I definitely didn't forget this before

* e2e test fix

* e2e test fix - actions link selector

* e2e test fix - lcid link selector

* e2e test fix - dates link selector

* NOREF - change email link URLs to reference new path (#2795)

change email link URLs to reference new path

* Update pre-signed S3 url timeout to 90 minutes

---------

Co-authored-by: Ashley Terstriep <60187543+aterstriep@users.noreply.github.com>
Co-authored-by: samoddball <156127704+samoddball@users.noreply.github.com>
Co-authored-by: ClayBenson94 <clay.benson@oddball.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants