fix: remove TanStack cache invalidation to prevent stale data overwrites #374
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRemoved several client-side cache invalidations from role create/delete and optimistic-update flows, eliminated a delayed 5s invalidation, and added tag-based cache invalidation for team-scoped entries in the metric delete mutation. No exported API signatures changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
PR Review: Fix TanStack Cache InvalidationSummaryThis PR addresses a critical caching issue where role data was disappearing after mutations due to race conditions between TanStack Query invalidate calls and Prisma Accelerate cache propagation. Strengths
Concerns & Questions1. Prisma Accelerate Cache Propagation Timing
2. Dashboard Cache Not Invalidated
3. Missing Background Refresh Mechanism
4. Team Role Count Update
Security Considerations
Test CoverageMissing E2E/integration tests. Recommended:
Performance ConsiderationsPositive: Removes unnecessary network requests, reduces server load RecommendationsHigh Priority:
Medium Priority: Approval RecommendationApprove with minor revisions. Core fix is sound but recommend addressing dashboard cache invalidation concern (#2) before merging to avoid confusing UX where changes don't appear on dashboard for several minutes. Great work identifying this tricky caching issue! The pattern of trusting server responses via setData over background invalidation is the right approach. |
Pull Request ReviewSummaryThis PR removes TanStack Query cache invalidations after role mutations to prevent stale data from overwriting correct data. The changes align well with the project's documented caching strategy. ✅ Strengths1. Problem Correctly Identified 2. Aligns with Documented Architecture
3. Server-Side Cache Invalidation is Robust
This means the server already handles cache consistency, making client-side invalidation redundant. 4. Consistent Pattern Application
🔍 Observations & Considerations1. Change from - onSettled: () => {
+ onSuccess: () => {Analysis: This is correct. 2. Removal of Dashboard Cache Invalidation (role-card.tsx:143) - void utils.dashboard.getDashboardCharts.invalidate({ teamId });Question: The Found: Verdict: ✅ Safe. Dashboard invalidation removal is correct here. 3. 5-Second Delayed Invalidation Removal - // Delayed invalidation for eventual consistency
- // Wait for Prisma Accelerate cache propagation before background refresh
- setTimeout(() => {
- void utils.role.getByTeamId.invalidate({ teamId });
- void utils.dashboard.getDashboardCharts.invalidate({ teamId });
- }, 5000);Analysis: This was a workaround for the exact problem being fixed. The 5-second delay was attempting to wait for Prisma cache propagation, but:
Verdict: ✅ Excellent cleanup. 🎯 Code QualityConsistency: ✅ All changes follow the same pattern 🔒 Potential Issues1. Edge Case: Rapid Successive Mutations If a user performs multiple role updates in quick succession (e.g., editing multiple fields in rapid succession), there's a theoretical risk that:
Mitigation: This is already handled by:
Verdict: ✅ Low risk, properly mitigated. 2. Multi-Tab/Multi-User Scenarios Without client-side invalidation, changes made in one tab/by another user won't appear until:
Current Behavior: The Mitigation Options:
Verdict: 🧪 Testing RecommendationsManual Testing Checklist:
Edge Cases:
📊 Performance ImpactBefore: Every mutation triggered 1-3 background refetches (role + dashboard + team) Estimated Impact:
🎬 RecommendationApprove with minor suggestion: Consider adding a comment in one of the hooks explaining why invalidation is removed, to prevent future developers from re-adding it: onSuccess: (updatedRole) => {
// Update cache with server response (authoritative data)
utils.role.getByTeamId.setData({ teamId }, (old) => {
if (!old) return [updatedRole];
return old.map((role) =>
role.id === updatedRole.id ? updatedRole : role,
);
});
// No invalidate() call here - server already invalidated Prisma cache.
// Calling invalidate() could refetch stale data before cache propagation.
// Only invalidate team for role count updates
void utils.team.getById.invalidate({ id: teamId });
},This is optional but would help maintain the fix long-term. Final Verdict: ✅ LGTM - ApproveThis PR correctly addresses a real caching issue with a clean, consistent solution. The changes are safe, well-reasoned, and improve both correctness and performance. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/app/teams/[teamId]/hooks/use-delete-role.tsx (1)
82-84: Document the intentional removal ofrole.getByTeamId.invalidate()(prevents regression vs repo guidance).This
onSuccessbehavior conflicts with existing repo guidance that says “setData(...)theninvalidate()for background refresh” (from retrieved learnings). Since this PR intentionally does not invalidate to avoid Prisma Accelerate stale overwrites, please add a short inline comment (or update the shared guidance doc) explaining the rationale so someone doesn’t reintroduceinvalidate()later.Proposed patch (inline comment)
onSuccess: () => { + // Intentionally do NOT invalidate `role.getByTeamId` here: + // immediate refetch may hit Prisma Accelerate stale cache and overwrite our optimistic `setData`. + // We rely on optimistic cache + server-side cache invalidation; keep only team refetch for role counts. void utils.team.getById.invalidate({ id: teamId }); },
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/teams/[teamId]/hooks/use-create-role.tsxsrc/app/teams/[teamId]/hooks/use-delete-role.tsxsrc/components/role/role-card.tsxsrc/hooks/use-optimistic-role-update.ts
💤 Files with no reviewable changes (2)
- src/app/teams/[teamId]/hooks/use-create-role.tsx
- src/hooks/use-optimistic-role-update.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/role/role-card.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-delete-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-delete-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-delete-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-delete-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-delete-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-delete-role.tsx
🧠 Learnings (13)
📓 Common learnings
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-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
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.
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: 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-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/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: 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-29T21:27:04.176Z
Learning: Applies to src/server/api/routers/**/*.ts : Invalidate dashboard cache after mutations using invalidateDashboardCache(ctx.db, organizationId, teamId)
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/server/api/routers/**/*.ts : After mutations, invalidate cache tags using `invalidateCacheByTags(ctx.db, [``tag_${id}``])` to trigger background cache updates with Prisma Accelerate.
📚 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-delete-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/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-delete-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-delete-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-delete-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-delete-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-delete-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-delete-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-delete-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-delete-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-delete-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} : Canvas auto-save system: Changes trigger markDirty() → Debounce 2s → serializeNodes/Edges → tRPC mutation. Include beforeunload sendBeacon fallback for unsaved changes.
Applied to files:
src/app/teams/[teamId]/hooks/use-delete-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 : Auto-save canvas changes with 2-second debounce: markDirty() → debounce → serializeNodes/Edges → tRPC mutation, with beforeunload sendBeacon fallback
Applied to files:
src/app/teams/[teamId]/hooks/use-delete-role.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
PR Review: Fix TanStack Cache Invalidation to Prevent Stale DataOverviewThis PR addresses a critical cache consistency issue where role data would disappear after mutations due to stale data from Prisma Accelerate overwriting correct data in TanStack Query cache. ✅ Strengths1. Correct Problem IdentificationThe root cause analysis is spot-on. The dual-cache architecture (TanStack Query + Prisma Accelerate) can cause race conditions when:
2. Clean SolutionRemoving client-side
3. Consistent PatternAll role mutation hooks follow the same optimistic update pattern:
4. Metric Delete EnhancementThe addition of
|
| File | Change | Impact |
|---|---|---|
| use-create-role.tsx | Remove invalidate calls | ✅ Correct |
| use-delete-role.tsx | Remove onSettled invalidate | |
| role-card.tsx | Remove onSettled invalidate | |
| use-optimistic-role-update.ts | Remove 5s delayed invalidate | ✅ Correct |
| metric.ts | Add team cache invalidation | ✅ Excellent |
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/server/api/routers/metric.ts (1)
252-282: Blocker: delete mutation is missing org/access verification before deleting.
ctx.db.metric.findUnique({ where: { id } })+metric.delete({ where: { id } })allows deleting any metric by ID without ensuring it belongs toctx.workspace.organizationId(and, by extension, the current user/workspace). As per coding guidelines, usegetMetricAndVerifyAccess(or equivalent) here.Also,
metric.teamIdcan be null;invalidateCacheByTags(ctx.db, [\team_${metric.teamId}`])will emitteam_null` and may waste invalidation budget—guard it.Proposed fix
delete: workspaceProcedure .input(z.object({ id: z.string() })) .mutation(async ({ ctx, input }) => { - // Get metric info before deletion for cache invalidation - const metric = await ctx.db.metric.findUnique({ - where: { id: input.id }, - select: { teamId: true, organizationId: true }, - }); - - if (!metric) { - throw new TRPCError({ - code: "NOT_FOUND", - message: "Metric not found", - }); - } + // Verify metric belongs to user's organization (and fetch for cache invalidation) + const metric = await getMetricAndVerifyAccess( + ctx.db, + input.id, + ctx.workspace.organizationId, + ); // Roles with this metric will have metricId set to null (onDelete: SetNull) await ctx.db.metric.delete({ where: { id: input.id }, }); // Invalidate Prisma cache for dashboard and role queries await invalidateDashboardCache( ctx.db, metric.organizationId, metric.teamId, ); - await invalidateCacheByTags(ctx.db, [`team_${metric.teamId}`]); + if (metric.teamId) { + await invalidateCacheByTags(ctx.db, [`team_${metric.teamId}`]); + } return { success: true }; }),
🤖 Fix all issues with AI agents
In @src/server/api/routers/metric.ts:
- Around line 12-15: The delete mutation in src/server/api/routers/metric.ts
must perform the same authorization as the update path: call
getMetricAndVerifyAccess(...) to verify the current user/team can delete the
metric before proceeding (mirror the update mutation's check and error
handling). After successful deletion, change the cache invalidation so it
doesn't emit a tag like team_null: only include the team tag when metric.teamId
is non-null (e.g., push/invalidate `team_${teamId}` only if teamId exists) and
always include org-level tags as done elsewhere; update the calls to
invalidateCacheByTags and invalidateDashboardCache accordingly so org-level
metrics do not generate a team_null tag.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/teams/[teamId]/hooks/use-create-role.tsxsrc/app/teams/[teamId]/hooks/use-delete-role.tsxsrc/components/role/role-card.tsxsrc/hooks/use-optimistic-role-update.tssrc/server/api/routers/metric.ts
💤 Files with no reviewable changes (4)
- src/components/role/role-card.tsx
- src/app/teams/[teamId]/hooks/use-delete-role.tsx
- src/app/teams/[teamId]/hooks/use-create-role.tsx
- src/hooks/use-optimistic-role-update.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Use TypeScript 5.9 with strict type checking for all frontend and backend code
Files:
src/server/api/routers/metric.ts
src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (GEMINI.md)
src/server/api/routers/**/*.ts: In tRPC procedures, never assume authentication; always use helpers likegetTeamAndVerifyAccessto verify access before operations
Be careful when modifying team save logic related toEditSessionmodel, which handles locking for concurrent team editing
src/server/api/routers/**/*.ts: Use tRPC protectedProcedure or workspaceProcedure for API routes; avoid manual auth checks in components as WorkOS middleware validates ctx.user
Always verify resources belong to user's organization using authorization helpers (getMetricAndVerifyAccess, getRoleAndVerifyAccess, getTeamAndVerifyAccess) in tRPC procedures
Invalidate dashboard cache after mutations using invalidateDashboardCache(ctx.db, organizationId, teamId)
Files:
src/server/api/routers/metric.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use @trivago/prettier-plugin-sort-imports with inline type imports for import organization
Files:
src/server/api/routers/metric.ts
src/server/api/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/server/api/**/*.ts: Use fetchUserDisplayName(userId) utility (server-side async WorkOS) from @/server/api/utils/get-user-display-name for server-side user name retrieval
Use enrichRolesWithUserNames() for flat role arrays and enrichChartRolesWithUserNames() for nested chart roles from src/server/api/utils/organization-members.ts
Files:
src/server/api/routers/metric.ts
🧠 Learnings (14)
📓 Common learnings
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-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
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/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.
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: 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: 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-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-29T21:27:04.176Z
Learning: Applies to src/server/api/routers/**/*.ts : Invalidate dashboard cache after mutations using invalidateDashboardCache(ctx.db, organizationId, teamId)
Learnt from: CR
Repo: drifter089/orgOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-29T12:52:42.935Z
Learning: Applies to src/server/api/routers/**/*.ts : After mutations, invalidate cache tags using `invalidateCacheByTags(ctx.db, [``tag_${id}``])` to trigger background cache updates with Prisma Accelerate.
📚 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/server/api/routers/metric.ts
📚 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/routers/**/*.ts : Invalidate dashboard cache after mutations using invalidateDashboardCache(ctx.db, organizationId, teamId)
Applied to files:
src/server/api/routers/metric.ts
📚 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/server/api/routers/**/*.ts : After mutations, invalidate cache tags using `invalidateCacheByTags(ctx.db, [``tag_${id}``])` to trigger background cache updates with Prisma Accelerate.
Applied to files:
src/server/api/routers/metric.ts
📚 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/server/api/routers/metric.ts
📚 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/server/api/routers/metric.ts
📚 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/server/api/routers/metric.ts
📚 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/components/dashboard-metric-card.tsx,src/app/dashboard/[teamId]/_components/public-dashboard-metric-card.tsx : Dashboard metric cards are duplicated with public variant. Consolidate into single component with `readOnly` mode prop instead of maintaining separate components.
Applied to files:
src/server/api/routers/metric.ts
📚 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/(metric|dashboard)/**/*.tsx : Use three-stage metrics transformation: API → DataPoints (DataIngestionTransformer), DataPoints → ChartConfig (ChartTransformer), ChartConfig → UI (DashboardMetricChart)
Applied to files:
src/server/api/routers/metric.ts
📚 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/server/api/routers/metric.ts
📚 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/lib/metrics/**/*.ts : Metric transformation logic should be organized in `src/lib/metrics/` to support the 3-stage pipeline: Ingestion → Aggregation → Visualization
Applied to files:
src/server/api/routers/metric.ts
📚 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/routers/**/*.ts : Always verify resources belong to user's organization using authorization helpers (getMetricAndVerifyAccess, getRoleAndVerifyAccess, getTeamAndVerifyAccess) in tRPC procedures
Applied to files:
src/server/api/routers/metric.ts
📚 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/server/api/routers/**/*.ts : When adding new tRPC routes, use `protectedProcedure` or `workspaceProcedure` and always verify resource belongs to user's organization using authorization helpers (getMetricAndVerifyAccess, getRoleAndVerifyAccess, getTeamAndVerifyAccess) from '@/server/api/utils/authorization'.
Applied to files:
src/server/api/routers/metric.ts
📚 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/server/api/routers/metric.ts
🧬 Code graph analysis (1)
src/server/api/routers/metric.ts (1)
src/server/api/utils/cache-strategy.ts (2)
invalidateDashboardCache(197-206)invalidateCacheByTags(167-183)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: claude-review
| import { | ||
| invalidateCacheByTags, | ||
| invalidateDashboardCache, | ||
| } from "@/server/api/utils/cache-strategy"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the metric.ts file to see the imports and delete mutation
head -30 src/server/api/routers/metric.tsRepository: drifter089/orgOS
Length of output: 1140
🏁 Script executed:
# Check the delete mutation around lines 273-280
sed -n '260,290p' src/server/api/routers/metric.tsRepository: drifter089/orgOS
Length of output: 988
🏁 Script executed:
# Look for the metric create input schema to check teamId nullability
rg -A 20 "z\.object.*metric" src/server/api/routers/metric.ts | head -50Repository: drifter089/orgOS
Length of output: 1229
🏁 Script executed:
# Search for cache tag patterns used in the codebase
rg "invalidateCacheByTags.*team_" src/server/api/ --context 2Repository: drifter089/orgOS
Length of output: 1960
🏁 Script executed:
# Check the metric model/schema definition for teamId
fd "metric" src/server --type f -name "*.ts" | xargs grep -l "teamId.*z\." | head -5Repository: drifter089/orgOS
Length of output: 290
🏁 Script executed:
# Get a larger window around the delete mutation to see if there's authz check
sed -n '250,285p' src/server/api/routers/metric.tsRepository: drifter089/orgOS
Length of output: 1147
🏁 Script executed:
# Search for the full delete mutation definition
rg -B 10 -A 15 "\.delete.*workspaceProcedure" src/server/api/routers/metric.tsRepository: drifter089/orgOS
Length of output: 42
🏁 Script executed:
# Check the metric schema for teamId optionality
rg -A 30 "export.*metric.*z\.object" src/server/db/schema.ts 2>/dev/null || rg -r "prisma" -A 5 "teamId" src/server/db/schema.prisma 2>/dev/null | head -20Repository: drifter089/orgOS
Length of output: 42
🏁 Script executed:
# Look for metric create input schema
rg -B 5 -A 20 "input.*z\.object.*metricId.*z" src/server/api/routers/metric.tsRepository: drifter089/orgOS
Length of output: 1396
🏁 Script executed:
# Check if org-level metrics (teamId=null) exist in schema
rg "teamId.*@db\.String" src/server/db/schema.prisma || rg "teamId.*String" src/server/db/schema.prismaRepository: drifter089/orgOS
Length of output: 195
🏁 Script executed:
# Find Prisma schema file
find . -name "*.prisma" -type fRepository: drifter089/orgOS
Length of output: 82
🏁 Script executed:
# Check if there's authorization verification for metrics elsewhere
rg "getMetricAndVerifyAccess" src/server/api/routers/metric.ts -B 5 -A 5Repository: drifter089/orgOS
Length of output: 1019
🏁 Script executed:
# Look for update/create mutations in metric.ts to see if they use authz helpers
rg -B 5 -A 15 "create.*workspaceProcedure\|update.*workspaceProcedure" src/server/api/routers/metric.ts | head -60Repository: drifter089/orgOS
Length of output: 42
🏁 Script executed:
# Check the Metric model definition in Prisma
rg -A 20 "^model Metric" prisma/schema.prisma 2>/dev/null || find . -path "*/prisma/*.prisma" -exec rg "model Metric" {} \;Repository: drifter089/orgOS
Length of output: 3188
🏁 Script executed:
# Verify delete mutation is missing authz by comparing with update mutation
sed -n '230,280p' src/server/api/routers/metric.tsRepository: drifter089/orgOS
Length of output: 1384
🏁 Script executed:
# Check how invalidateDashboardCache handles null teamId
rg -A 20 "function invalidateDashboardCache\|export.*invalidateDashboardCache" src/server/api/utils/cache-strategy.tsRepository: drifter089/orgOS
Length of output: 42
🏁 Script executed:
# Check how invalidateCacheByTags handles the tags
rg -B 5 -A 25 "function invalidateCacheByTags\|export.*invalidateCacheByTags" src/server/api/utils/cache-strategy.tsRepository: drifter089/orgOS
Length of output: 42
🏁 Script executed:
# Verify workspaceProcedure adds organization-level filtering
rg -A 10 "workspaceProcedure" src/server/api/trpc.ts | head -30Repository: drifter089/orgOS
Length of output: 190
🏁 Script executed:
# Search for cache-strategy file and its functions
cat src/server/api/utils/cache-strategy.tsRepository: drifter089/orgOS
Length of output: 6332
🏁 Script executed:
# Check if there are any other delete mutations in routers to see the pattern
rg -B 5 -A 20 "\.delete.*workspaceProcedure" src/server/api/routers/ | head -100Repository: drifter089/orgOS
Length of output: 42
🏁 Script executed:
# Verify the exact issue: delete doesn't verify org access like update does
rg -B 20 "delete.*workspaceProcedure" src/server/api/routers/metric.ts | grep -E "getMetricAndVerifyAccess|workspaceProcedure|delete:"Repository: drifter089/orgOS
Length of output: 143
Add authorization check and guard cache tag invalidation in delete mutation.
The delete mutation is missing the getMetricAndVerifyAccess check that the update mutation uses. This allows any authenticated workspace user to delete any metric in the organization, regardless of team access. Additionally, when teamId is null (org-level metrics), the cache tag becomes team_null, wasting rate limits.
Suggested fix
delete: workspaceProcedure
.input(z.object({ id: z.string() }))
.mutation(async ({ ctx, input }) => {
- // Get metric info before deletion for cache invalidation
- const metric = await ctx.db.metric.findUnique({
- where: { id: input.id },
- select: { teamId: true, organizationId: true },
- });
-
- if (!metric) {
- throw new TRPCError({
- code: "NOT_FOUND",
- message: "Metric not found",
- });
- }
+ // Verify metric belongs to user's organization
+ const metric = await getMetricAndVerifyAccess(
+ ctx.db,
+ input.id,
+ ctx.workspace.organizationId,
+ );
// Roles with this metric will have metricId set to null (onDelete: SetNull)
await ctx.db.metric.delete({
where: { id: input.id },
});
// Invalidate Prisma cache for dashboard and role queries
await invalidateDashboardCache(
ctx.db,
metric.organizationId,
metric.teamId,
);
- await invalidateCacheByTags(ctx.db, [`team_${metric.teamId}`]);
+ if (metric.teamId) {
+ await invalidateCacheByTags(ctx.db, [`team_${metric.teamId}`]);
+ }
return { success: true };
}),🤖 Prompt for AI Agents
In @src/server/api/routers/metric.ts around lines 12 - 15, The delete mutation
in src/server/api/routers/metric.ts must perform the same authorization as the
update path: call getMetricAndVerifyAccess(...) to verify the current user/team
can delete the metric before proceeding (mirror the update mutation's check and
error handling). After successful deletion, change the cache invalidation so it
doesn't emit a tag like team_null: only include the team tag when metric.teamId
is non-null (e.g., push/invalidate `team_${teamId}` only if teamId exists) and
always include org-level tags as done elsewhere; update the calls to
invalidateCacheByTags and invalidateDashboardCache accordingly so org-level
metrics do not generate a team_null tag.
Summary
Fixes role data disappearing after creation/update/delete by removing unnecessary TanStack Query cache invalidations that could overwrite correct data with stale Prisma Accelerate responses.
Problem
After role mutations, calling
invalidate()triggered refetches that could return stale data from Prisma Accelerate cache, overwriting the correct data we already set viasetData().Solution
invalidate()calls forrole.getByTeamIdafter mutationsteam.getById.invalidate()for role count updatessetData()with server response as authoritative dataChanges
use-create-role.tsx- Remove role cache invalidationuse-delete-role.tsx- Change fromonSettledtoonSuccess, remove role cache invalidationrole-card.tsx- Change fromonSettledtoonSuccess, remove role/dashboard cache invalidationuse-optimistic-role-update.ts- Remove delayed (5s) cache invalidationSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.