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

[Consent] Group consents + Improve UI #6044

Merged
merged 4 commits into from
Aug 19, 2020

Conversation

zaliqarosli
Copy link
Contributor

@zaliqarosli zaliqarosli commented Feb 10, 2020

Brief summary of changes

PR 2 of 2. This PR includes changes from #6042 and will need to be rebased once #6042 has been merged.

This PR follows the first PR by rendering the grouping of consents and improves other aspects of the UI.

This PR:

  • Adds vertical tabs for each parent consent and renders within that tab all the children consents
  • For each consent rendering: adds a HeaderElement for the consent label, changes label for Yes/No select Element to 'Response', changes label for Date element to 'Date of Response'
  • Hides Consent history by default
  • Add Show/Hide Consent history toggling
  • Cleans up the rendering of the history string

Testing instructions (if applicable)

  1. Source all sql patches as instructed in PR [Consent] Add ParentID to consent for consent grouping #6042
  2. git checkout zaliqarosli/2020-02-10-AddParentToConsentUI
  3. Run make
  4. Got to Consent Status tab of candidate_parameters module and you will see the following changed display

Screenshot 2020-02-10 18 20 22

  1. Play around with updating consent - the editing functionality should remain the same (other than 'Update' button getting disabled on form submission)

@johnsaigle johnsaigle added the Needs Code Formatting PR requires formatting fixes to meet our coding standards (PHPCS, phan, etc.) label Feb 11, 2020
@PapillonMcGill
Copy link
Contributor

LGTM. Final verdict will wait rebase after #6042 is merged

@johnsaigle johnsaigle added Blocked PR awaiting the merge, resolution or rejection of an other Pull Request and removed Needs Code Formatting PR requires formatting fixes to meet our coding standards (PHPCS, phan, etc.) labels Feb 24, 2020
@johnsaigle
Copy link
Contributor

Adding Blocked as the other PR should be merged first (right?)

PapillonMcGill
PapillonMcGill previously approved these changes Feb 25, 2020
@PapillonMcGill PapillonMcGill added the Passed Manual Tests PR has undergone proper testing by at least one peer label Feb 25, 2020
@PapillonMcGill
Copy link
Contributor

Just one note,
I was able during the test to enter consent dated on the 25th and after, removed consent dated on the 24th.
Not sure if this is desired or a bug.

@zaliqarosli
Copy link
Contributor Author

@PapillonMcGill that has to do with data validation and is not something touched in this PR. might be a good call to add it to github issues? @ridz1208 what do you think? should consent withdrawal date be checked to be after the given date of consent?

@ridz1208
Copy link
Collaborator

@PapillonMcGill @zaliqarosli in theory it makes sense. outside the scope of this PR.

@zaliqarosli
Copy link
Contributor Author

@ridz1208 @PapillonMcGill okay i'll add it as an issue

@zaliqarosli zaliqarosli added the Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch label Apr 14, 2020
@driusan
Copy link
Collaborator

driusan commented Jun 12, 2020

This has conflicts and needs to be rebased

@zaliqarosli zaliqarosli changed the base branch from master to main July 23, 2020 15:02
@zaliqarosli zaliqarosli added the Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix label Aug 4, 2020
@zaliqarosli zaliqarosli force-pushed the 2020-02-10-AddParentToConsentUI branch from 4e3703c to 499e064 Compare August 6, 2020 20:56
@zaliqarosli zaliqarosli removed the Needs Work PR awaiting additional changes by the author or contains issues that the author needs to fix label Aug 6, 2020
@ridz1208 ridz1208 added Feature PR or issue introducing/requiring at least one new feature and removed Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch Passed Manual Tests PR has undergone proper testing by at least one peer labels Aug 6, 2020
driusan pushed a commit that referenced this pull request Aug 11, 2020
Adds ConsentGroupID to consent table (schema + patch)
  
This keeps the Consent Status tab rendering the same. Only the back-end is affected in this PR. This feature is increasingly in demand from projects who would like to split up consents via e.g. consent forms.

See also #6044 for front-end changes.
@jesscall
Copy link
Contributor

Whenever I try to add a new consent group I get the following errors on the front-end:

Screen Shot 2020-08-11 at 2 43 34 PM

@driusan driusan added Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch and removed Blocked PR awaiting the merge, resolution or rejection of an other Pull Request labels Aug 11, 2020
edit raisinbread

remove undefined index php notice

query for non parent consents to populate frontend

travis

change from consent parent to consent grouping

fix raisinbread

group raisinbread consent

fix patch

exchange parentID for groupID

lintphp

fix schema

modify sql table and add patch

edit raisinbread

remove undefined index php notice

query for non parent consents to populate frontend

improve frontend

remove unnecessary ParentID query

travis

eslint

update parentID to consentGroupID

delete old patch

php lint

parent to group
@zaliqarosli zaliqarosli force-pushed the 2020-02-10-AddParentToConsentUI branch from f054b3b to 84a03e1 Compare August 11, 2020 20:00
@zaliqarosli
Copy link
Contributor Author

zaliqarosli commented Aug 11, 2020

@jesscall do you get that error when there is a consent group that has no consents attached to it?

edit: commit 43ff938 should fix it!

@zaliqarosli zaliqarosli removed the Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch label Aug 12, 2020
@jesscall
Copy link
Contributor

@zaliqarosli yes the issue is fixed! looks good :)

jesscall
jesscall previously approved these changes Aug 12, 2020
@jesscall jesscall added the Passed Manual Tests PR has undergone proper testing by at least one peer label Aug 12, 2020
@ridz1208
Copy link
Collaborator

@zaliqarosli could you add some content to the changelog please?

@zaliqarosli
Copy link
Contributor Author

@ridz1208 re-review please!

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Dave MacFarlane <driusan@gmail.com>
@driusan driusan merged commit d504f85 into aces:main Aug 19, 2020
@ridz1208 ridz1208 added this to the 24.0.0 milestone Aug 20, 2020
zaliqarosli added a commit to zaliqarosli/Loris that referenced this pull request Sep 3, 2020
- Adds vertical tabs for each parent consent and renders within that tab all the children consents
- For each consent rendering: adds a HeaderElement for the consent label, changes label for Yes/No select Element to 'Response', changes label for Date element to 'Date of Response'
- Hides Consent history by default
- Add Show/Hide Consent history toggling
- Cleans up the rendering of the history string
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 15, 2021
Adds ConsentGroupID to consent table (schema + patch)
  
This keeps the Consent Status tab rendering the same. Only the back-end is affected in this PR. This feature is increasingly in demand from projects who would like to split up consents via e.g. consent forms.

See also aces#6044 for front-end changes.
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
Adds ConsentGroupID to consent table (schema + patch)
  
This keeps the Consent Status tab rendering the same. Only the back-end is affected in this PR. This feature is increasingly in demand from projects who would like to split up consents via e.g. consent forms.

See also aces#6044 for front-end changes.
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
- Adds vertical tabs for each parent consent and renders within that tab all the children consents
- For each consent rendering: adds a HeaderElement for the consent label, changes label for Yes/No select Element to 'Response', changes label for Date element to 'Date of Response'
- Hides Consent history by default
- Add Show/Hide Consent history toggling
- Cleans up the rendering of the history string
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature PR or issue introducing/requiring at least one new feature Passed Manual Tests PR has undergone proper testing by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants