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.

Sign up for GitHub

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

Jump to bottom

Display person consents when merging #2376

Merged
merged 6 commits into from
Apr 24, 2023
Merged

Display person consents when merging #2376

merged 6 commits into from
Apr 24, 2023

Conversation

pbanaszkiewicz
Copy link
Contributor

This fixes #2370.

@pbanaszkiewicz pbanaszkiewicz added this to the v4.1 milestone Apr 7, 2023
@pbanaszkiewicz pbanaszkiewicz self-assigned this Apr 7, 2023
@pbanaszkiewicz pbanaszkiewicz added this to In progress in Consents via automation Apr 7, 2023
@pbanaszkiewicz pbanaszkiewicz mentioned this pull request Apr 7, 2023
Merged
6 tasks
@elichad
Copy link
Contributor

elichad commented Apr 11, 2023

I've discussed the consents merging strategy with Maneesha and Rob, and we've settled on always using the most recent consents. This shouldn't be something the user doing the merge can select. Could you update this PR to:

  • remove the choice between A and B and replace it with text such as "Use the most recent consents"
  • perhaps show the timestamps for the consents so it's clear which will be used
  • program this strategy in the backend. It looks like the "combine" option used to do the same thing, so hopefully some of that code can be used directly

@pbanaszkiewicz
Copy link
Contributor Author

@elichad sure. I will work on this later this week.

@elichad
Copy link
Contributor

elichad commented Apr 12, 2023

Great, thanks.
Another idea that came out of the discussion was that we could send an email to the merged person, letting them know that their records have been merged and that they should check everything is okay (consents and the rest of their info). I'll make a separate issue for that as I think it fits better into the email automation project.

@elichad elichad mentioned this pull request Apr 12, 2023
Open
@pbanaszkiewicz pbanaszkiewicz force-pushed the feature/2370-display-person-consents-when-merging branch from 5f6f81d to f576e49 Compare April 16, 2023 21:05
@pbanaszkiewicz
Copy link
Contributor Author

This is WIP and may require updated tests.

@pbanaszkiewicz pbanaszkiewicz marked this pull request as draft April 16, 2023 21:07
@pbanaszkiewicz pbanaszkiewicz marked this pull request as ready for review April 21, 2023 21:23
@pbanaszkiewicz
Copy link
Contributor Author

@elichad Ready for your re-review.

elichad
elichad approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@elichad elichad left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

Consents automation moved this from In progress to In Review Apr 24, 2023
@elichad elichad mentioned this pull request Apr 24, 2023
Closed
@pbanaszkiewicz pbanaszkiewicz merged commit a703553 into develop Apr 24, 2023
9 checks passed
Consents automation moved this from In Review to Done Apr 24, 2023
@pbanaszkiewicz pbanaszkiewicz deleted the feature/2370-display-person-consents-when-merging branch April 24, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elichad elichad elichad approved these changes

Assignees

@pbanaszkiewicz pbanaszkiewicz

Labels
component: forms component: user interface (UI)
Projects
Consents
Done
Milestone
v4.1
Development

Successfully merging this pull request may close these issues.

Display consents in person merge view
2 participants
@pbanaszkiewicz @elichad