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

chore(approvals-ga): remove beta flag for MRF email notifications and approvals #7920

Merged
merged 2 commits into from
Nov 20, 2024

Conversation

kevin9foong
Copy link
Contributor

@kevin9foong kevin9foong commented Nov 19, 2024

Problem

Need to release mrf email notifications and approvals from beta to General Availability.

Solution

Remove the beta flags for mrf email notifications and approvals.

Note that the mrfEmailNotification beta flag has been retained to prevent accidental reuse of prior flags. I.e., we might have another feature that uses the same flag name of mrfEmailNotification which is not known to the developer cause it no longer exist in the records, but exist in User collection.

Breaking Changes

  • No - this PR is backwards compatible

Tests

Test set up:

  • Create an MRF form called 'finance claims approval'
  • Add 2 approval yes/no fields named 'RO approve', 'Finance dept approve'. Assert that Use for approvals badge appears beside yes/no fields in drawer.
  • Add 2 email fields
  • Assign 1 email field to step 1 to edit, Assign approval field to step 2 to edit and Assign approval field to step 3 to edit. (do not assign approval field or step 1 email notification field yet)
  • Let step 2 be static email, let step 3 be dynamic email field

TC1: Regression test - MRF still works

  • Open the form to public
  • Respond to form, ensure that all steps of mrf can be completed and result is in admin responses tab.
  • Assert no email outcomes are sent, since not yet configured in email notification settings.

TC2: Regression test - email notif works:

  • From TC1, config email notif setting to step 2 and static email of choice.
  • Complete submission, assert that workflow completed (note that it is not an outcome status email) is sent to expected emails.
  • Remove the email notifs to prepare for TC3.

TC3: Approve without email notif. Expect no email sent

  • Assign approval field to step 2 only.
  • Complete all 3 steps of mrf workflow and approve at step 2.
  • No email is sent.

TC4: Reject without email notif. Expect no email sent

  • Use same form from TC3
  • Reject at step 2 of mrf workflow.
  • No email is sent to step 3 respondent since workflow ends, no outcome email is sent.

TC5: Approve and reject with email notification works:

  • Add step 1 and step 3 to email notification and a static email to email notification.
  • Repeat TC3. Assert that only email from step 3 and static email receives approve email, since step 1 does not have email field setup.
  • Add email field to step 1 in workflow.
  • Repeat TC4. Assert step 1 and step 3 and static email receive not approved email.

TC6: Multiple approval steps - approve all

  • From TC5, add approval step for step 3 of workflow.
  • Approve all approval steps, approve email should be received.

TC7: Multiple approval steps - Reject at step 2 (middle step)

  • From TC6, make submission
  • Reject at step 2, assert R3 does not receive email and not approved email is sent to expected emails.

TC8: Multiple approval steps - Reject at last step

  • From TC7, make submission
  • Approve at step 2 but Reject at step 3, assert not approved email is sent to all expected emails.

Edge cases:
TC10: approval field set to optional, respondent did not fill

  • From TC9, set the approval field used for step 2 as optional.
  • Try and submit the steps of form, but at step 2, do not fill in the optional approval field.
  • assert that R3 no longer gets the next step email, no completion/approval outcome emails are sent.
  • Visit the responses page, the R2 submission is recorded.
  • Share the next step response link, it goes to R3 and can be resumed.
  • Try and submit R3 and approve/reject, it should send out approved/reject email to expected emails.

TC10: approval field deleted, admin did not reassign

  • From TC9, delete the approval field used for step 2.
  • Try and submit the steps of the form.
  • assert that R3 no longer gets the next step email, no completion/approval outcome emails are sent.
  • Visit the responses page, the R2 submission is recorded.
  • Share the next step response link, it goes to R3 and can be resumed.
  • Try and submit R3 and approve/reject, it should send out approved/reject email to expected emails.

@kevin9foong kevin9foong requested a review from KenLSM November 19, 2024 04:17
@kevin9foong kevin9foong self-assigned this Nov 19, 2024
@kevin9foong
Copy link
Contributor Author

Tested on local

@kevin9foong kevin9foong marked this pull request as ready for review November 19, 2024 04:32
@datadog-opengovsg
Copy link

datadog-opengovsg bot commented Nov 19, 2024

Datadog Report

Branch report: chore/remmove-approvals-beta
Commit report: d3a5fa7
Test service: formsg

✅ 0 Failed, 2300 Passed, 1 Skipped, 5m 21.2s Total duration (1m 41.89s time saved)

@kevin9foong kevin9foong requested a review from KenLSM November 19, 2024 06:11
Copy link
Contributor

@KenLSM KenLSM left a comment

Choose a reason for hiding this comment

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

LGTM! Yay for deletions.

@kevin9foong kevin9foong merged commit 06aaef7 into develop Nov 20, 2024
19 checks passed
@kevin9foong kevin9foong deleted the chore/remmove-approvals-beta branch November 20, 2024 02:08
@kevin9foong kevin9foong mentioned this pull request Nov 20, 2024
35 tasks
kevin9foong added a commit that referenced this pull request Nov 20, 2024
* chore(deps-dev): bump @typescript-eslint/eslint-plugin from 8.14.0 to 8.15.0 in /shared (#7912)

chore(deps-dev): bump @typescript-eslint/eslint-plugin in /shared

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 8.14.0 to 8.15.0.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.15.0/packages/eslint-plugin)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/eslint-plugin"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(deps): bump @aws-sdk/client-lambda from 3.654.0 to 3.693.0 (#7910)

Bumps [@aws-sdk/client-lambda](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-lambda) from 3.654.0 to 3.693.0.
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-lambda/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.693.0/clients/client-lambda)

---
updated-dependencies:
- dependency-name: "@aws-sdk/client-lambda"
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump @typescript-eslint/parser from 8.14.0 to 8.15.0 in /shared (#7911)

chore(deps-dev): bump @typescript-eslint/parser in /shared

Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 8.14.0 to 8.15.0.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.15.0/packages/parser)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(approvals-ga): remove beta flag for MRF email notifications and approvals (#7920)

* chore: remove beta flag for mrfEmailNotifications and approvals

* chore: retain flag

* chore: bump version to v6.165.0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

2 participants