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

feat: approvals for mrf #7636

Open
wants to merge 44 commits into
base: develop
Choose a base branch
from
Open

feat: approvals for mrf #7636

wants to merge 44 commits into from

Conversation

kevin9foong
Copy link
Member

@kevin9foong kevin9foong commented Sep 2, 2024

Problem

From mrf workshops and user testing sessions, admins want a way for mrf to be used for their approval processes.

Closes FRM-1632, FRM-1829, FRM-1834, FRM-1835, FRM-1836, FRM-1856, FRM-1863

Solution

Implement approval support for mrf according to Approvals v8.

Breaking Changes

  • No - this PR is backwards compatible

Features:

Improvements:

  • Revamped UI improve understandability of MRF builder.
  • Radio is now vertical which follows design conventions instead of horizontal.
  • Added validation for workflow email field & approval fields on per workflow step granularity.

Before & After Screenshots

Revamped workflow builder (inactive state):

  • more obvious error message with contrast
  • addition of approval step
Before After
image image

Revamped workflow builder (Active state):

  • new radio layout
  • addition of approval fields
  • more directed field prompts
Before After
image image

New approvals outcome email:
image

Manual QA 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.

Copy link

linear bot commented Sep 2, 2024

FRM-1829 mrf approvals

@kevin9foong kevin9foong self-assigned this Sep 2, 2024
@kevin9foong
Copy link
Member Author

kevin9foong commented Sep 2, 2024

Feature Todo:

  • BE send outcome email based on approve/reject
  • BE validate if form field is valid yes/no form field (done on the save step form edit)
  • Fix FE validation error message -> focus -> need to click twice issue

Copy link

linear bot commented Sep 6, 2024

@kevin9foong kevin9foong marked this pull request as ready for review September 9, 2024 15:01
@kevin9foong
Copy link
Member Author

kevin9foong commented Sep 9, 2024

Moving to testing phase:

  • Pending manual QA tests
  • unit tests
  • chromatic visual tests

@kevin9foong
Copy link
Member Author

Add betaFlag same as email notifications before merging

@kevin9foong
Copy link
Member Author

  • Written comprehensive visual and interaction tests for FE workflow editStepBlock error states and inactive states.
  • Written UT for various approvals edge cases

Copy link

linear bot commented Sep 14, 2024

@kevin9foong kevin9foong force-pushed the feat/mrf-approvals branch 10 times, most recently from 6dc8154 to 6d4d883 Compare September 16, 2024 05:20
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.

Minor issues here and there but generally no big issue with the overall design of the approval feature. Discovered some debts, but not attributable to this PR.

Great work @kevin9foong !

Copy link

linear bot commented Sep 19, 2024

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