Repository navigation
fix(realtime): fix shared drive permissions not updating in sidebar - #248
Conversation
📝 WalkthroughWalkthroughAdds permission-cache invalidation calls after member invite/update/delete operations and makes the global drive socket hook join/leave user-specific channels using the authenticated user ID (tracked via a ref) instead of socket IDs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Drive API
participant DB as Database
participant Cache as Permissions Cache
participant Socket as Socket Server
rect rgba(0,128,255,0.5)
Client->>API: Invite / Update / Delete member
API->>DB: persist change (create/update/delete member)
DB-->>API: ack
API->>Socket: broadcast member change event
API->>Cache: invalidateUserPermissions(userId) & invalidateDrivePermissions(driveId)
Cache-->>API: ack invalidation
Socket-->>Client: emit member change event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![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 `@apps/web/src/hooks/useGlobalDriveSocket.ts`:
- Around line 106-124: The cleanup that leaves channels and resets refs must be
moved into the first useEffect that depends on [socket, user?.id,
handleDriveEvent]: inside that effect's cleanup, clear revalidationTimeoutRef if
present, and if (socket && hasJoinedRef.current && joinedUserIdRef.current) emit
'leave_global_drives' and emit 'leave' for
`user:${joinedUserIdRef.current}:drives`, then set hasJoinedRef.current = false
and joinedUserIdRef.current = null; after moving this logic, remove the separate
second useEffect that only depended on [socket] so channel leave/reset happens
whenever user?.id or socket changes and new users can re-join correctly.
🧹 Nitpick comments (1)
apps/web/src/app/api/drives/[driveId]/members/invite/route.ts (1)
122-126: Cache invalidation placement is acceptable, though timing could be optimized.The invalidation occurs before page permissions are created (lines 129-177). In a theoretical race condition, if the cache is re-populated by another request between invalidation and page permission creation, it might cache stale permissions. Consider moving this block after line 180 (after
Promise.all(permissionPromises)) to ensure all permissions are committed before cache invalidation.That said, the current placement is functional—the user already has drive membership at this point, and subsequent cache misses will fetch correct data.
| }, [socket, user?.id, handleDriveEvent]); | ||
|
|
||
| // Clean up timeout on unmount | ||
| useEffect(() => { | ||
| return () => { | ||
| if (revalidationTimeoutRef.current) { | ||
| clearTimeout(revalidationTimeoutRef.current); | ||
| } | ||
|
|
||
| // Leave the global drives channel and user-specific drives channel when unmounting | ||
| if (socket && hasJoinedRef.current) { | ||
| if (socket && hasJoinedRef.current && joinedUserIdRef.current) { | ||
| socket.emit('leave_global_drives'); | ||
| socket.emit('leave', `user:${socket.id}:drives`); | ||
| socket.emit('leave', `user:${joinedUserIdRef.current}:drives`); | ||
| console.log(`🌍 Left global:drives and user:${joinedUserIdRef.current}:drives channels`); | ||
| hasJoinedRef.current = false; | ||
| console.log('🌍 Left global:drives and user:drives channels'); | ||
| joinedUserIdRef.current = null; | ||
| } | ||
| }; | ||
| }, [socket]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the full useGlobalDriveSocket.ts file
cat -n apps/web/src/hooks/useGlobalDriveSocket.tsRepository: 2witstudios/PageSpace
Length of output: 5318
Consolidate channel cleanup into the first effect to properly handle user changes.
When user?.id changes (logout then login as different user), the first effect's cleanup runs and removes event listeners, but doesn't reset hasJoinedRef or leave channels. The second effect has [socket] as its only dependency, so its cleanup only runs on unmount or socket reconnection—not on user change. This means hasJoinedRef.current remains true and joinedUserIdRef.current keeps the old user ID. When the first effect runs again for the new user, the join condition on line 68 fails (if (!hasJoinedRef.current && user?.id)), and the new user never subscribes to their channel.
Move the channel cleanup and ref reset logic from the second effect's cleanup (lines 110-122) into the first effect's cleanup (after line 104), then remove the second effect entirely. This ensures channels are properly left and refs are reset whenever the user or socket changes.
Proposed fix
// Cleanup function
return () => {
console.log('🌍 useGlobalDriveSocket: Cleaning up global drive listeners');
// Remove all event listeners
events.forEach(event => {
socket.off(event, handleDriveEvent);
});
// Clear any pending debounced refetch
if (revalidationTimeoutRef.current) {
clearTimeout(revalidationTimeoutRef.current);
revalidationTimeoutRef.current = undefined;
}
+
+ // Leave channels and reset join state when user changes
+ if (hasJoinedRef.current && joinedUserIdRef.current) {
+ socket.emit('leave_global_drives');
+ socket.emit('leave', `user:${joinedUserIdRef.current}:drives`);
+ console.log(`🌍 Left global:drives and user:${joinedUserIdRef.current}:drives channels`);
+ hasJoinedRef.current = false;
+ joinedUserIdRef.current = null;
+ }
};
}, [socket, user?.id, handleDriveEvent]);
-
- // Clean up timeout on unmount
- useEffect(() => {
- return () => {
- if (revalidationTimeoutRef.current) {
- clearTimeout(revalidationTimeoutRef.current);
- }
-
- // Leave the global drives channel and user-specific drives channel when unmounting
- if (socket && hasJoinedRef.current && joinedUserIdRef.current) {
- socket.emit('leave_global_drives');
- socket.emit('leave', `user:${joinedUserIdRef.current}:drives`);
- console.log(`🌍 Left global:drives and user:${joinedUserIdRef.current}:drives channels`);
- hasJoinedRef.current = false;
- joinedUserIdRef.current = null;
- }
- };
- }, [socket]);🤖 Prompt for AI Agents
In `@apps/web/src/hooks/useGlobalDriveSocket.ts` around lines 106 - 124, The
cleanup that leaves channels and resets refs must be moved into the first
useEffect that depends on [socket, user?.id, handleDriveEvent]: inside that
effect's cleanup, clear revalidationTimeoutRef if present, and if (socket &&
hasJoinedRef.current && joinedUserIdRef.current) emit 'leave_global_drives' and
emit 'leave' for `user:${joinedUserIdRef.current}:drives`, then set
hasJoinedRef.current = false and joinedUserIdRef.current = null; after moving
this logic, remove the separate second useEffect that only depended on [socket]
so channel leave/reset happens whenever user?.id or socket changes and new users
can re-join correctly.
- Fix useGlobalDriveSocket to use user.id instead of socket.id for channel subscription (socket.id is the Socket.IO connection ID, not the user ID, so member events were never being received) - Add permission cache invalidation when members are added/updated/removed - Consolidate cleanup into single useEffect so channel leave/reset happens when user?.id or socket changes - Add mocks for invalidateUserPermissions/invalidateDrivePermissions in tests These changes ensure that when someone shares a drive with a user: 1. The realtime event is properly received (correct channel subscription) 2. Permission caches are invalidated immediately (no stale permissions) 3. The sidebar refreshes to show the newly shared drive https://claude.ai/code/session_017uoRQb8sw7PuoxVgK2hhhQ
5e5e3b6 to
6f2baa6
Compare
2 participants
(socket.id is the Socket.IO connection ID, not the user ID, so member events were
never being received)
These changes ensure that when someone shares a drive with a user:
https://claude.ai/code/session_017uoRQb8sw7PuoxVgK2hhhQ
Summary by CodeRabbit
Bug Fixes
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.