Add Consent Request API #1387
Add Consent Request API #1387
Conversation
|
Starting review.. |
| def get_cached_verification_code(self) -> Optional[str]: | ||
| """Retrieve the generated identity verification code if it exists""" | ||
| cache = get_cache() | ||
| values = cache.get_values([f"IDENTITY_VERIFICATION_CODE__{self.id}"]) or {} |
There was a problem hiding this comment.
What's the rationale for not using .get(key) here?
There was a problem hiding this comment.
Nit: can we use a constant to store, or method to return the IDENTITY_VERIFICATION_CODE__{self.id} so we can only make one update if we change it?
There was a problem hiding this comment.
This is all just a direct copy from PrivacyRequest above
There was a problem hiding this comment.
Lots of potential edge cases covered so far, but some bugs found and cleanup needed.
Main concerns are a test email being saved on the providedidentity by default, the unique constraint on consent.data_use that seems incorrect, how we don't expose the consentrequest id anywhere so the user can't make the proper request to get/update preferences, postman collection bugs and deletions of existing endpoints, identity data being required/returned in the responses for getting/saving preferences, new provided identities being created on the consentrequest endpoint create instead of correctly locating the existing one.
Secondary concerns, copy/pasted code that could be shared, minor typos..
|
Switching over to this to do a final pass! assuming it's ready |
Yes, I believe everything is ready. |
There was a problem hiding this comment.
Pass complete @sanders41, a little more clean-up requested, but it felt much more user-friendly this time around so I think it's almost there. Passing in the identity data for updating the consent request preferences still felt unintuitive, but I see we're waiting on @TheAndrewJackson's feedback there.
There was a problem hiding this comment.
nice work @sanders41
We might address a new email template in a follow-up based on @TheAndrewJackson's comment #1387 (comment) if needed
|
Thanks @pattisdr. I added an issue to follow-up with the new template. |
Purpose
Adds a consent request API
Changes
Checklist
CHANGELOG.mdfileCHANGELOG.mdfile is being appended toUnreleasedsection in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.Run Unsafe PR Checkslabel has been applied, and checks have passed, if this PR touches any external servicesTicket
Fixes #1321
Fixes #1161