Fix: Notifications do not load when switching wallets#3586
Merged
Conversation
mmioana
previously approved these changes
Nov 1, 2024
Contributor
mmioana
left a comment
There was a problem hiding this comment.
Awesome work on this one @iamsamgibbs 👏 🥇 💯
After connecting the wallets, the magic bell user id was created for both leela and amy


Created a simple payment from leela to amy

And switched to amy's wallet without a page refresh and the notifications were there

Also the check in auth-proxy for the getUserNotificationsHMAC query has passed ✅


rdig
previously approved these changes
Nov 4, 2024
69bc3b5 to
240b13b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.




Description
The actual fix is in this auth proxy PR: https://github.com/JoinColony/colonyCDappAuthProxy/pull/33
This PR aims to address an issue where notifications do not load when switching wallets.
This was being caused by the
useGetUserNotificationsHmacQueryin theNotificationsDataContextProviderbeing sent before the auth cookie had been set. Which resulted in thegetUserNotificationsHMAClambda returning undefined as the x-wallet-address header is not set.My proposed solution here is to change how the auth proxy handles un-auth queries. By default the auth proxy allows all un-auth queries and only blocks mutations. This change will still allow un-auth queries by default, but will restrict certain queries (currently just
getUserNotificationsHMAC) from executing when not auth.This causes the
useGetUserNotificationsHmacQueryto be reattempted when the wallet connection has properly finished and notifications will load correctly.Note: I have tried to avoid making changes to the auth proxy by preventing the
useGetUserNotificationsHmacQueryfrom being triggered until the auth headers are set, but was unable to get this working. If this is the preferred solution and anyone has any ideas on how to resolve this, I'm happy to re-investigate this.Testing
We first want to make sure we have two users with notifications enabled and setup.
createUserNotificationsDatamutation has ran.createUserNotificationsDatamutation has ran.Now that we have two users with notifications enabled we can test properly.
Diffs
New stuff ✨
Resolves #3508