-
Notifications
You must be signed in to change notification settings - Fork 1
fix: prevent blank flash when creating role in canvas #373
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR refactors optimistic cache handling in the role creation hook. Instead of immediately replacing temp roles with real roles in the cache, it now maps and swaps role IDs before render, then triggers cache invalidation after success to maintain background consistency. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Code ReviewSummaryThis is a well-identified and correctly-fixed race condition. The fix properly aligns with the established cache management patterns in the codebase. What This FixesThe race condition occurred when:
Code Quality ✅Strengths:
Architecture Consistency ✅This change follows the established pattern from CLAUDE.md: The PR correctly implements this for the create flow, matching the update flow in Performance Considerations ✅
Potential Issues / SuggestionsMinor: Comment Clarity // Update cache BEFORE node so RoleCard finds newRole.id on re-render
// Without this ordering: node updates → RoleCard re-renders → cache still has tempRoleId → blank flashQuestion: Test Coverage
Security ✅No security concerns. This is purely a client-side cache management change. Bugs ✅No new bugs introduced. The fix resolves the race condition without side effects. Verdict: LGTM ✅This is a textbook fix for a cache synchronization race condition. The change is minimal, well-reasoned, and follows established patterns in the codebase. The only suggestion is to enhance the comment for future maintainers, but this is optional. Recommend merging as-is. |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/app/teams/[teamId]/hooks/use-create-role.tsx:
- Around line 226-228: Add dashboard cache invalidation after the existing
team/role invalidations in use-create-role (where utils.team.getById.invalidate
and utils.role.getByTeamId.invalidate are called): call
utils.dashboard.getDashboardCharts.invalidate({ teamId }) so dashboard charts
refresh when a role is created with a metricId, placing it immediately after the
two existing invalidate calls to match other mutations like use-auto-save.tsx
and use-optimistic-role-update.ts.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/teams/[teamId]/hooks/use-create-role.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Use TypeScript 5.9 with strict type checking for all frontend and backend code
Files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Prefer Server Components for initial data fetching; use Client Components ('use client') only for interactivity
Files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
src/**/*/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Client Components must use
import { api } from '@/trpc/react'for standard HTTP/Hooks wrapper
Files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
**/*.tsx
📄 CodeRabbit inference engine (GEMINI.md)
Use Tailwind CSS 4 for styling with shadcn/ui and Radix UI primitive components
Files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
src/app/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/app/**/*.tsx: Use the dual tRPC API pattern: direct calls in Server Components (api.team.getById) for 10x faster performance, and React hooks in Client Components (api.team.getById.useQuery)
Use getUserDisplayName(userId, members) utility (client-side sync) from @/lib/helpers/get-user-name for displaying user names in components
Files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use @trivago/prettier-plugin-sort-imports with inline type imports for import organization
Files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/teams/[teamId]/_components/**/*.tsx : Implement cache-first node pattern for role nodes: store only roleId in node data, fetch display data from TanStack Query cache using useRoleData hook
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/hooks/use-optimistic-role-update.ts : The `use-optimistic-role-update` hook provides shared logic for all role mutations with proper cache layer handling for TanStack Query and Prisma Accelerate. Use this hook in canvas-specific wrappers to add markDirty side effects.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : React Flow nodes must store minimal data (e.g., just `roleId`); fetch full Role data from TanStack Query cache in the Node component to keep canvas and sidebars in sync
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/teams/[teamId]/**/*.{ts,tsx} : Cache-first node system: Role nodes store only `roleId` in node.data. Display data must be fetched from TanStack Query cache using the `useRoleData()` hook which queries the `role.getByTeamId` procedure.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/hooks/**/*.ts : For role-metric cache updates, optimistically update both role cache (role.getByTeamId) and dashboard cache (dashboard.getDashboardCharts) to maintain UI consistency
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/**/*role*mutation*.{ts,tsx} : For role mutations, use cache pipeline with two operations: (1) onMutate for optimistic update, (2) onSuccess with both setData(updatedRole) using server response AND invalidate() for background refresh. Always use server response data in setData before invalidate to ensure Prisma Accelerate cache consistency.
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/dashboard/[teamId]/**/*.{ts,tsx} : Dashboard cache updates for role-metric assignments must update both `role.getByTeamId` and `dashboard.getDashboardCharts` caches during mutations. Use onMutate for optimistic updates on both caches, then invalidate both on success.
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/hooks/use-optimistic-role-update.ts : The `use-optimistic-role-update` hook provides shared logic for all role mutations with proper cache layer handling for TanStack Query and Prisma Accelerate. Use this hook in canvas-specific wrappers to add markDirty side effects.
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/app/teams/[teamId]/_components/**/*.tsx : Implement cache-first node pattern for role nodes: store only roleId in node data, fetch display data from TanStack Query cache using useRoleData hook
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/**/*role*mutation*.{ts,tsx} : For role mutations, use cache pipeline with two operations: (1) onMutate for optimistic update, (2) onSuccess with both setData(updatedRole) using server response AND invalidate() for background refresh. Always use server response data in setData before invalidate to ensure Prisma Accelerate cache consistency.
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/hooks/**/*.ts : For role-metric cache updates, optimistically update both role cache (role.getByTeamId) and dashboard cache (dashboard.getDashboardCharts) to maintain UI consistency
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/dashboard/[teamId]/**/*.{ts,tsx} : Dashboard cache updates for role-metric assignments must update both `role.getByTeamId` and `dashboard.getDashboardCharts` caches during mutations. Use onMutate for optimistic updates on both caches, then invalidate both on success.
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/teams/[teamId]/**/*.{ts,tsx} : Cache-first node system: Role nodes store only `roleId` in node.data. Display data must be fetched from TanStack Query cache using the `useRoleData()` hook which queries the `role.getByTeamId` procedure.
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : React Flow nodes must store minimal data (e.g., just `roleId`); fetch full Role data from TanStack Query cache in the Node component to keep canvas and sidebars in sync
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-20T22:12:00.576Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-12-20T22:12:00.576Z
Learning: Applies to src/app/teams/[teamId]/**/*.tsx : Avoid modifying `enrichNodesWithRoleData` flow without understanding the complete canvas serialization logic for saving/loading React Flow nodes to the database
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/hooks/**/*.ts : Use optimistic updates with setData before invalidate in TanStack Query mutations to ensure correct data when Prisma Accelerate cache hasn't propagated
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T12:52:42.935Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/app/teams/[teamId]/_components/role-node.tsx,src/app/teams/[teamId]/_components/public-role-node.tsx : These role node components are 75% identical and should be consolidated. Extract shared `RoleNodeTemplate` component with `isEditable` prop to DRY up the code.
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
📚 Learning: 2025-12-29T21:27:04.176Z
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T21:27:04.176Z
Learning: Applies to src/server/api/**/*.ts : Use enrichRolesWithUserNames() for flat role arrays and enrichChartRolesWithUserNames() for nested chart roles from src/server/api/utils/organization-members.ts
Applied to files:
src/app/teams/[teamId]/hooks/use-create-role.tsx
🔇 Additional comments (1)
src/app/teams/[teamId]/hooks/use-create-role.tsx (1)
198-224: LGTM! The reordering correctly fixes the race condition.Updating the cache before the node ensures
RoleCard(which reads from the TanStack Query cache viauseRoleData) findsnewRoleby its real ID when it re-renders. This aligns with the cache-first node pattern documented in the learnings.
| // Invalidate caches for background refresh | ||
| void utils.team.getById.invalidate({ id: teamId }); | ||
| void utils.role.getByTeamId.invalidate({ teamId }); |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n src/app/teams/[teamId]/hooks/use-create-role.tsx | head -250 | tail -50Repository: drifter089/orgOS
Length of output: 1968
🏁 Script executed:
# Check the full CreateRoleVariables type definition
rg -n "type CreateRoleVariables|interface CreateRoleVariables" -A 10 src/app/teams/[teamId]/hooks/use-create-role.tsxRepository: drifter089/orgOS
Length of output: 322
🏁 Script executed:
# Search for invalidateDashboardCache utility and its usage
rg -n "invalidateDashboardCache" --type=ts --type=tsx -B 2 -A 2Repository: drifter089/orgOS
Length of output: 87
🏁 Script executed:
# Look for other role-metric mutation patterns
rg -n "metricId" --type=ts --type=tsx src/app/teams/ -B 2 -A 2 | head -100Repository: drifter089/orgOS
Length of output: 87
🏁 Script executed:
# Search for invalidateDashboardCache without strict file type
rg -n "invalidateDashboardCache" -B 2 -A 2Repository: drifter089/orgOS
Length of output: 8673
🏁 Script executed:
# Search for dashboard.getDashboardCharts invalidation
rg -n "dashboard.getDashboardCharts.invalidate" -B 2 -A 2Repository: drifter089/orgOS
Length of output: 5361
🏁 Script executed:
# Check other role mutation hooks
fd -e ts -e tsx "use-.*role" src/app/teams/ | head -20Repository: drifter089/orgOS
Length of output: 259
🏁 Script executed:
# Look for metric assignment patterns in mutations
rg -n "metricId" src/app/teams/[teamId]/hooks/ -B 3 -A 3Repository: drifter089/orgOS
Length of output: 9557
Add dashboard cache invalidation to maintain UI consistency with role creation.
The cache invalidation is missing utils.dashboard.getDashboardCharts.invalidate({ teamId }). This is required to ensure dashboard charts reflect metric assignments when a role is created with a metricId. Other role mutations in the codebase follow this pattern (see use-auto-save.tsx, use-optimistic-role-update.ts, and role-card.tsx).
Add the following line after the existing invalidations:
void utils.dashboard.getDashboardCharts.invalidate({ teamId });🤖 Prompt for AI Agents
In @src/app/teams/[teamId]/hooks/use-create-role.tsx around lines 226 - 228, Add
dashboard cache invalidation after the existing team/role invalidations in
use-create-role (where utils.team.getById.invalidate and
utils.role.getByTeamId.invalidate are called): call
utils.dashboard.getDashboardCharts.invalidate({ teamId }) so dashboard charts
refresh when a role is created with a metricId, placing it immediately after the
two existing invalidate calls to match other mutations like use-auto-save.tsx
and use-optimistic-role-update.ts.
Summary
Fixes a race condition in role creation that caused the role node to briefly show blank/default data after closing the dialog.
Changes
onSuccesshandler to update TanStack Query cache before updating the canvas nodeSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.