-
Notifications
You must be signed in to change notification settings - Fork 1
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
Updates notification unsubscribe logic (Gets data from Notification table instead of JWT) #45
Conversation
pages/unsubscribe/[id].tsx
Outdated
await handleUnsubscribe(notificationType, notificationId, emailAddress); | ||
await removeUnsubscribeReference(id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between these calls? Is there a reason they're separate? From their names sounds like handleUnsubscribe does not actually unsubscribe you and we need this additional separate step. If that's the case, would suggest merging the endpoints it calls into one potentially
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleUnsubscribe
calls existing endpoints to remove notifications rows, these endpoints are accessed elsewhere in the app and they will in fact unsubscribe you. RemoveUnsubscribeReference
is a new endpoint just to cleanup the unsubscribe reference table. We probably could combine these if we think it's significantly better / want to make the existing endpoints more generic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have combined these calls with the lastest commits
Description
GAP-2152
Reads unsubscribe reference data from the unsubscribe table rather than decoding the JWT.
related prs: cabinetoffice/gap-find-backend#59, cabinetoffice/gap-user-service#135
Type of change
Please check the relevant options.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes:
Unit Test
Integration Test (if applicable)
End to End Test (if applicable)
Screenshots (if appropriate):
Please attach screenshots of the change if it is a UI change:
Checklist: