fix: Notification counter now gets updated when reading a dialog (#1398) #1404
Conversation
📝 WalkthroughWalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
packages/frontend/src/api/useDialogById.tsx(3 hunks)
🔇 Additional comments (3)
packages/frontend/src/api/useDialogById.tsx (3)
1-1: LGTM: Clean import addition
The addition of useQueryClient import is appropriate for implementing cache invalidation.
186-186: LGTM: Proper hook initialization
The useQueryClient hook is correctly initialized at the top level of the component, following React hooks rules.
Line range hint 186-198: Verify the notification counter update behavior
Let's verify that this implementation properly updates the notification counter when reading a dialog.
✅ Verification successful
Based on the gathered information, I can now generate the final response:
Cache invalidation correctly triggers dialog list refresh with unread status
The implementation is correct for updating notification counters:
- The GraphQL query
SearchDialogFieldsincludesseenSinceLastUpdatefield which tracks read status - When a dialog is fetched (
useDialogById), it invalidates the dialogs query cache - The invalidation triggers a refetch of
getAllDialogsForPartiesquery which includes the updated seen/unread status - The dialog list is automatically updated with fresh data including notification state
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the notification counter is properly updated through the dialogs query
# Check for the notification counter implementation in the dialogs query
echo "Checking dialogs query implementation..."
rg -A 5 "notification.*count|unread.*count"
# Check if the QUERY_KEYS.DIALOGS is used consistently
echo "Checking query key usage..."
rg "QUERY_KEYS\.DIALOGS"
# Look for any existing tests related to notification counting
echo "Checking related tests..."
fd -e test.tsx -e spec.tsx | xargs rg "notification.*count|unread.*count"
Length of output: 4374
Script:
#!/bin/bash
# Let's check the implementation of useDialogs hook and the dialog data structure
# Check the useDialogs implementation
echo "Checking useDialogs implementation..."
ast-grep --pattern 'export function useDialogs($$$) {
$$$
}'
# Check the dialog type/interface definition
echo "Checking dialog type definition..."
rg -A 10 "type.*Dialog.*=|interface.*Dialog.*{"
# Check if there are any unread/notification count fields in GraphQL queries
echo "Checking GraphQL queries..."
fd -e graphql | xargs rg "unread|notification"
Length of output: 528
Script:
#!/bin/bash
# Let's fix the previous queries and check additional aspects
# Check the useDialogs implementation
echo "Checking useDialogs implementation..."
rg -A 10 "export.*function useDialogs"
# Check the dialog type definition with fixed regex
echo "Checking dialog type definition..."
rg -A 10 "(type Dialog|interface Dialog)"
# Check GraphQL operations related to dialogs
echo "Checking GraphQL operations..."
fd -e ts -e tsx | xargs rg "query GetDialogs|query GetDialogById"
# Check the actual GraphQL query definitions
echo "Checking GraphQL query definitions..."
fd -e graphql | xargs cat
Length of output: 10090
| @@ -193,6 +194,8 @@ export const useDialogById = (parties: PartyFieldsFragment[], id?: string): UseD | |||
| enabled: typeof id !== 'undefined' && partyURIs.length > 0, | |||
| }); | |||
|
|
|||
| isSuccess && queryClient.invalidateQueries({ queryKey: [QUERY_KEYS.DIALOGS] }); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider optimizing the cache invalidation strategy
While the current implementation works, there are several potential improvements to consider:
- The invalidation is too broad and happens on every successful render.
- The short-circuit evaluation style reduces code readability.
Consider this more focused approach:
- isSuccess && queryClient.invalidateQueries({ queryKey: [QUERY_KEYS.DIALOGS] });
+ useEffect(() => {
+ if (isSuccess && data?.dialogById.dialog) {
+ // Only invalidate the specific dialog and the dialogs list
+ queryClient.invalidateQueries({
+ queryKey: [QUERY_KEYS.DIALOGS],
+ exact: true,
+ });
+ queryClient.invalidateQueries({
+ queryKey: [QUERY_KEYS.DIALOG_BY_ID, id],
+ exact: true,
+ });
+ }
+ }, [isSuccess, data, id, queryClient]);This approach:
- Uses a proper effect hook to handle side effects
- More precisely targets the cache invalidation
- Is more explicit about the execution conditions
- Follows React's best practices for handling side effects
Committable suggestion skipped: line range outside the PR's diff.
Hva er endret?
Related Issue(s)
Dokumentasjon / Storybook / testdekning
storiesiStorybook, eller så er ikke dette relevant.Skjermbilder eller GIFs (valgfritt)
Summary by CodeRabbit
New Features
Bug Fixes