[Candidate_parameters] Improve front-end validation for entering Consent data #4034

zaliqarosli · 2018-10-29T19:14:15Z

Brief summary of changes

This PR makes entering consent data more rigorous. Going forward, this PR implements the following workflow for data entry:

Date of consent being given is required for both answers to consent "yes" or "no"
Date of withdrawal of consent required if answer changes from "yes" to "no"
Consent status "yes" or "no" cannot be changed to an empty one
Replaces confirmation message to swal
Adds script to check errors in data

To test this change...

Make sure you have consent set up (https://github.com/aces/Loris/blob/minor/modules/candidate_parameters/setup.md)
In Candidate Parameters -> Consent Status tab:

Date of Withdrawal field should be disabled until it is needed.
For a currently empty consent status field, change status to "yes". Date of Consent should be required.
For a currently empty consent status field, change status to "no". Date of Consent should be required. Date of withdrawal still disabled.
For a consent status field with a "yes" status, change answer to "no". Date of Withdrawal should be enabled and required. Save answer.
Change the same status field back to "yes". Date of Withdrawal is disabled and the date cleared.
For a consent status field with a "no" or "yes" status, you cannot change it back to empty.

Note for reviewing: While there are plenty of pre-ES6 syntax, conversion of vars to lets/consts does not belong in this PR. All new code highlighted in green should already conform to ES6 standards.

me-evans · 2018-10-30T17:51:48Z

As discussed in today's LORIS meeting, weighing in re: proposed consent changes. This is great, just have a few minor adjustments to request.

Date of consent being given is required for both answers to consent "yes" and "no"
- Agree completely
Date of withdrawal of consent required if answer changes from "yes" to "no"
- Agree, just propose also blocking them from :
  - Entering Date of withdrawal as the first date with either Yes or No
    - Reason: To prevent errors and to enforce a proper consent history
      - Ex. If consent is withdrawn before user has recorded consent given, we should still require that an entry of 'Yes' with the 'Date of Consent' be saved before allowing a new entry with the 'No' / 'Date of Withdrawal' combo
  - Entering a 'Date of Withdrawal' if the consent is still 'Yes'
If "yes" or "no" answer to consent already exists, the current status cannot be erased to an empty answer
- Agree with @LeighMac that some users (those who can edit candidate parameters) should be able to remove entries made by mistake.
- Could remove blank option in status menu once a 'yes' or 'no' has been saved and instead have a separate, explicit button for clearing the last consent update in case of error.
  - Button triggers pop-up with warning to only proceed if entry was made by mistake (ex. wrong candidate) and instructions to update consent instead if not erasing an error
  - User must select either 'Proceed' to clear last status/date or 'Dismiss' to return to main

zaliqarosli · 2018-11-01T18:48:29Z

Changes made :

Date of Withdrawal field is disabled by default. It is enabled when either :

consent is being withdrawn (status changes from "yes" to "no") OR
date of withdrawal already exists (allows user to remove/change date)

Date of Withdrawal field is required when:

consent is being withdrawn (status changes from "yes" to "no")

Removed disallowing users to erase a valid status as the ideal workflow drawn out above requires more work to be done. A GitHub issue can be made to address this @me-evans . Please review code to double check the logic for points 1 and 2. I added comments to make the code human readable.

edit: latest workflow updated in PR description

davidblader · 2018-11-05T18:26:06Z

modules/candidate_parameters/jsx/ConsentStatus.js

-            withdrawalRequired[i] = false;
+          // If answer to consent is "no", require date of consent
+          if (this.state.formData[consent] === "no") {
+            dateRequired[i] = true;


should this have a default value set to it as well?

may as well!

@davidblader done

ridz1208 · 2018-11-26T17:06:18Z

@zaliqarosli just tested this, minor changes needed.

I cant seem to be able to clear the date of withdrawal and save
whenthe consent changes from yes/no to empty, the date fields should all be cleared.

zaliqarosli · 2018-11-27T18:16:55Z

PR description updated to address Rida's comments above. Issue created to follow up on solving how to clear consent

ridz1208 · 2018-12-07T15:37:49Z

@zaliqarosli still not working

ridz1208 · 2018-12-07T15:38:37Z

@zaliqarosli new rules need to be applied on the backend through a release script

zaliqarosli · 2018-12-10T18:52:47Z

@ridz1208 please re-review!

tools/single_use/Cleanup_Consent_Data.php

zaliqarosli · 2019-01-02T21:40:28Z

@ridz1208 done. please re-review

stale

ridz1208

EVERYTHING ELSE LOOKS GOOD

tools/single_use/Cleanup_Consent_Data.php

travis add disabled withdraw revamp logic travis add clarity travis ridas request cleanup cleanup backend validation dont give users the option to throw an error travis travis travis travis travis travis validation for dirty data cleanup add cleanup script lint php travis travis change DB

…ent data (aces#4034) This makes entering consent data more rigorous. Implements the following workflow for data entry: - Date of consent being given is required for both answers to consent "yes" or "no" - Date of withdrawal of consent required if answer changes from "yes" to "no" - Consent status "yes" or "no" cannot be changed to an empty one - Replaces confirmation message to swal - Adds script to check errors in data

zaliqarosli added Area: UI PR or issue related to the user Interface Cleanup PR or issue introducing/requiring at least one clean-up operation [branch] minor labels Oct 29, 2018

zaliqarosli changed the title ~~[Candidate_parameters] Improve validation for entering Consent data~~ [Candidate_parameters] Improve front-end validation for entering Consent data Oct 29, 2018

davidblader reviewed Nov 5, 2018

View reviewed changes

zaliqarosli added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 12, 2018

zaliqarosli force-pushed the 2018-10-29-ImproveConsentValidation branch from 08a0106 to b386b5f Compare November 12, 2018 18:05

zaliqarosli removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 12, 2018

zaliqarosli mentioned this pull request Nov 27, 2018

[ConsentStatus] Add explicit button/action to clear consent #4149

Closed

zaliqarosli force-pushed the 2018-10-29-ImproveConsentValidation branch from d648b23 to e133af4 Compare November 29, 2018 20:01

ridz1208 added the State: Needs work PR awaiting additional work by the author to proceed label Dec 7, 2018

zaliqarosli force-pushed the 2018-10-29-ImproveConsentValidation branch from 7011aab to b83f140 Compare December 7, 2018 23:40

zaliqarosli added "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help and removed "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help State: Needs work PR awaiting additional work by the author to proceed labels Dec 7, 2018

ridz1208 previously requested changes Dec 28, 2018

View reviewed changes

tools/single_use/Cleanup_Consent_Data.php Outdated Show resolved Hide resolved

ridz1208 self-assigned this Jan 25, 2019

ridz1208 requested changes Feb 1, 2019

View reviewed changes

tools/single_use/Cleanup_Consent_Data.php Outdated Show resolved Hide resolved

zaliqarosli added 2 commits February 4, 2019 12:10

add if errors empty

6a0876c

zaliqarosli force-pushed the 2018-10-29-ImproveConsentValidation branch from 5b3519d to 6a0876c Compare February 4, 2019 17:23

ridz1208 approved these changes Feb 5, 2019

View reviewed changes

ridz1208 added the Passed manual tests PR has been successfully tested by at least one peer label Feb 5, 2019

driusan merged commit 9ef6e11 into aces:minor Feb 11, 2019

ridz1208 added this to the 20.3.0 milestone Mar 14, 2019

zaliqarosli mentioned this pull request Jun 14, 2019

[Candidate Parameters] Outdated Test Plan (Consent Status Tab) #4876

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Candidate_parameters] Improve front-end validation for entering Consent data #4034

[Candidate_parameters] Improve front-end validation for entering Consent data #4034

zaliqarosli commented Oct 29, 2018 •

edited

Loading

me-evans commented Oct 30, 2018 •

edited

Loading

zaliqarosli commented Nov 1, 2018 •

edited

Loading

davidblader Nov 5, 2018

zaliqarosli Nov 5, 2018

zaliqarosli Nov 5, 2018

ridz1208 commented Nov 26, 2018

zaliqarosli commented Nov 27, 2018 •

edited

Loading

ridz1208 commented Dec 7, 2018

ridz1208 commented Dec 7, 2018

zaliqarosli commented Dec 10, 2018

zaliqarosli commented Jan 2, 2019

ridz1208 left a comment

[Candidate_parameters] Improve front-end validation for entering Consent data #4034

[Candidate_parameters] Improve front-end validation for entering Consent data #4034

Conversation

zaliqarosli commented Oct 29, 2018 • edited Loading

Brief summary of changes

To test this change...

me-evans commented Oct 30, 2018 • edited Loading

zaliqarosli commented Nov 1, 2018 • edited Loading

davidblader Nov 5, 2018

Choose a reason for hiding this comment

zaliqarosli Nov 5, 2018

Choose a reason for hiding this comment

zaliqarosli Nov 5, 2018

Choose a reason for hiding this comment

ridz1208 commented Nov 26, 2018

zaliqarosli commented Nov 27, 2018 • edited Loading

ridz1208 commented Dec 7, 2018

ridz1208 commented Dec 7, 2018

zaliqarosli commented Dec 10, 2018

zaliqarosli commented Jan 2, 2019

ridz1208 left a comment

Choose a reason for hiding this comment

zaliqarosli commented Oct 29, 2018 •

edited

Loading

me-evans commented Oct 30, 2018 •

edited

Loading

zaliqarosli commented Nov 1, 2018 •

edited

Loading

zaliqarosli commented Nov 27, 2018 •

edited

Loading