🐛 app: invalidate kyc query on version change #682
Conversation
🦋 Changeset detectedLatest commit: 68e5691 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello @franm91, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust mechanism for managing client-side data caching across application versions. By implementing version-aware deserialization and integrating a release identifier, the system can now automatically invalidate specific cached queries, such as KYC data, when the application updates. This prevents the use of stale or incompatible data, ensuring data integrity and a consistent user experience after new deployments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #682 +/- ##
==========================================
+ Coverage 61.84% 61.87% +0.02%
==========================================
Files 169 169
Lines 5289 5298 +9
Branches 1496 1497 +1
==========================================
+ Hits 3271 3278 +7
- Misses 1845 1848 +3
+ Partials 173 172 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds version-aware cache deserialization to the QueryClient persistence layer: persisted state is annotated with a buster (current release), deserialization filters KYC-related queries when the buster differs from the running release, and the persisted buster is updated during hydration and save. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cruzdanilo
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to invalidate the kyc query cache when the application version changes. It uses a version 'buster' string with the persisted query client and a custom deserialization function for selective cache invalidation. My review found a critical logical flaw in the custom deserializer that would cause the entire cache to be invalidated, instead of just the kyc queries. I've provided a code suggestion to fix this issue and ensure the intended behavior is achieved. The rest of the changes correctly support this new functionality.
| function versionAwareDeserialize(cache: string): PersistedClient { | ||
| const data: PersistedClient = deserialize(cache); | ||
| if (data.buster === release) return data; | ||
| data.clientState.queries = data.clientState.queries.filter( | ||
| (query) => !INVALIDATE_ON_UPGRADE.has(query.queryKey[0] as string), | ||
| ); | ||
| return data; | ||
| } |
There was a problem hiding this comment.
The current implementation of versionAwareDeserialize correctly filters out stale queries, but it returns the PersistedClient object with its original (old) buster string. Subsequently, persistQueryClientRestore is called with the new release version as its buster parameter. This mismatch causes persistQueryClientRestore to invalidate the entire cache, which negates the selective invalidation logic.
To fix this, versionAwareDeserialize should return a new PersistedClient object that includes the updated buster value (release) along with the filtered queries. This ensures that persistQueryClientRestore sees a matching buster and preserves the rest of the cache.
| function versionAwareDeserialize(cache: string): PersistedClient { | |
| const data: PersistedClient = deserialize(cache); | |
| if (data.buster === release) return data; | |
| data.clientState.queries = data.clientState.queries.filter( | |
| (query) => !INVALIDATE_ON_UPGRADE.has(query.queryKey[0] as string), | |
| ); | |
| return data; | |
| } | |
| function versionAwareDeserialize(cache: string): PersistedClient { | |
| const data: PersistedClient = deserialize(cache); | |
| if (data.buster === release) return data; | |
| return { | |
| timestamp: data.timestamp, | |
| buster: release, | |
| clientState: { | |
| mutations: data.clientState.mutations, | |
| queries: data.clientState.queries.filter( | |
| (query) => !INVALIDATE_ON_UPGRADE.has(query.queryKey[0] as string), | |
| ), | |
| }, | |
| }; | |
| } |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/queryClient.ts`:
- Around line 20-27: versionAwareDeserialize currently filters out upgraded
queries but leaves data.buster unchanged so TanStack Query will discard the
whole cache; after filtering data.clientState.queries (using
INVALIDATE_ON_UPGRADE and query.queryKey) set data.buster = release before
returning so the deserialized PersistedClient (constructed via deserialize) is
recognized as valid; update the function versionAwareDeserialize to assign
data.buster = release after the filter and then return data.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/queryClient.ts (1)
54-72: Passbuster: releasetopersistQueryClientSubscribeto prevent perpetual KYC invalidation.Currently,
persistQueryClientSubscribeis called withoutbuster, so cache updates persist without a buster value. On app restart,persistQueryClientRestorerestores withbuster: release, butversionAwareDeserializetreats the stored cache as having an outdated buster and filters out KYC queries—causing KYC to be cleared every time the app launches.🐛 Proposed fix
if (typeof window !== "undefined") { - const subscribe = () => persistQueryClientSubscribe({ queryClient, persister, dehydrateOptions }); + const subscribe = () => + persistQueryClientSubscribe({ queryClient, persister, dehydrateOptions, buster: release }); hydrated.then(subscribe, subscribe); }
🤖 Fix all issues with AI agents
In `@src/utils/queryClient.ts`:
- Around line 12-28: Rename the generic local variable "data" in function
versionAwareDeserialize to a descriptive name like "persistedClient": replace
the assignment const data = deserialize(cache) with const persistedClient =
deserialize(cache) and update all subsequent references (persistedClient.buster,
persistedClient.clientState.queries, persistedClient.buster = release) and the
function return to return persistedClient so the intent of the variable is clear
and consistent with the PersistedClient type.
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.