guilhermearaujo-glitch/nossocrm - #38
Conversation
|
Someone is attempting to deploy a commit to the Thales Laray Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds post-mutation cache synchronization for deal items in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
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)
lib/query/hooks/useDealsQuery.ts (1)
520-531:⚠️ Potential issue | 🟠 MajorAdd generic type to
setQueryDataand remove prefix-matching invalidation in item mutations.Two issues in both
useAddDealItem(line 521) anduseRemoveDealItem(line 548):
Missing
<DealView[]>generic onsetQueryData. Every othersetQueryData(DEALS_VIEW_KEY, ...)call in this file uses the generic (lines 272, 299, 363, 378, 439, 487, 655, 668, 676). Without it,oldis untyped under TS strict mode and the.map(),.filter(), property access calls lack type safety. Add<DealView[]>to both.
onSettledinvalidation contradicts file's established pattern. This file explicitly documents why deal list invalidation is avoided: comments at lines 336, 382, and 499 state"NÃO fazer invalidateQueries para deals - Realtime gerencia a sincronização". The reason:queryKeys.deals.lists()prefix-matchesDEALS_VIEW_KEY([...queryKeys.deals.lists(), 'view']), so invalidating it refetchesDEALS_VIEW_KEYand overwrites the optimisticonSuccesspatch with stale data from the server. Remove thequeryKeys.deals.lists()invalidation from bothuseAddDealItemanduseRemoveDealItem— keep only the detail invalidation, as Realtime handles DEALS_VIEW_KEY sync.Suggested patch for both hooks
- onSuccess: (data, { dealId }) => { - queryClient.setQueryData(DEALS_VIEW_KEY, (old) => { - if (!old) return old; - return old.map((d) => - d.id === dealId ? { ...d, items: [...(d.items ?? []), data.item] } : d - ); - }); - }, - onSettled: (_data, _error, { dealId }) => { - queryClient.invalidateQueries({ queryKey: queryKeys.deals.detail(dealId) }); - queryClient.invalidateQueries({ queryKey: queryKeys.deals.lists() }); - }, + onSuccess: (data, { dealId }) => { + queryClient.setQueryData<DealView[]>(DEALS_VIEW_KEY, (old) => { + if (!old) return old; + return old.map((d) => + d.id === dealId ? { ...d, items: [...(d.items ?? []), data.item] } : d + ); + }); + }, + onSettled: (_data, _error, { dealId }) => { + // NÃO fazer invalidateQueries para deals - Realtime gerencia a sincronização + queryClient.invalidateQueries({ queryKey: queryKeys.deals.detail(dealId) }); + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/query/hooks/useDealsQuery.ts` around lines 520 - 531, In the useAddDealItem and useRemoveDealItem hooks: add the missing generic to queryClient.setQueryData as setQueryData<DealView[]>(DEALS_VIEW_KEY, ...) so the callback's old value is typed and map/filter/property access are type-safe, and update the onSettled handlers to stop invalidating the list prefix — remove the queryClient.invalidateQueries({ queryKey: queryKeys.deals.lists() }) call and keep only the detail invalidation (queryKeys.deals.detail(dealId)), because invalidating the lists prefix will refetch and overwrite the optimistic patch applied to DEALS_VIEW_KEY.
🧹 Nitpick comments (1)
lib/query/hooks/useDealsQuery.ts (1)
511-533: Consider moving the cache patch toonMutatefor true optimistic feedback.The PR description ("produto aparece imediatamente após adicionar ao deal") implies users want instant feedback. Running the patch in
onSuccessstill waits for the server round-trip — which is better than before, but not instant. Given the rest of this file already follows an optimistic-update pattern with rollback viaonError, consider doing the same here: patch the cache inonMutatewith a temp item id, replace the temp id inonSuccess, and roll back inonError. Otherwise on a slow network the product will still visibly lag.Not a blocker for this fix — flagging as a follow-up improvement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/query/hooks/useDealsQuery.ts` around lines 511 - 533, The current useAddDealItem applies the cache patch in onSuccess which waits for the server; change to a true optimistic update by moving the cache patch into onMutate inside useAddDealItem: generate a temporary id for the new item, immediately update DEALS_VIEW_KEY via queryClient.setQueryData to append the temp item to the matching deal, and return a context object containing the previous cache snapshot and the temp id for rollback; implement onError to restore the previous snapshot from that context, implement onSuccess to replace the temp id with the real item returned by dealsService.addItem (and merge any server-returned fields), and keep onSettled to invalidate queryKeys.deals.detail(dealId) and queryKeys.deals.lists(); reference functions/values: useAddDealItem, onMutate, onError, onSuccess, queryClient, dealsService.addItem, DEALS_VIEW_KEY, and queryKeys.deals.detail/lists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@vercel.json`:
- Line 13: The cron for the stage evaluations endpoint was changed to daily,
causing up to 24h latency for queued evaluations; revert the schedule for the
"/api/cron/stage-evaluations" entry back to a per-minute cadence (e.g., "*/1 * *
* *") so stage advancement runs frequently and avoids backlog; update
vercel.json's cron entry that references "/api/cron/stage-evaluations"
accordingly and ensure the change aligns with the route handler at
app/api/cron/stage-evaluations/route.ts and the evaluation logic in
lib/ai/agent/agent.service.ts.
---
Outside diff comments:
In `@lib/query/hooks/useDealsQuery.ts`:
- Around line 520-531: In the useAddDealItem and useRemoveDealItem hooks: add
the missing generic to queryClient.setQueryData as
setQueryData<DealView[]>(DEALS_VIEW_KEY, ...) so the callback's old value is
typed and map/filter/property access are type-safe, and update the onSettled
handlers to stop invalidating the list prefix — remove the
queryClient.invalidateQueries({ queryKey: queryKeys.deals.lists() }) call and
keep only the detail invalidation (queryKeys.deals.detail(dealId)), because
invalidating the lists prefix will refetch and overwrite the optimistic patch
applied to DEALS_VIEW_KEY.
---
Nitpick comments:
In `@lib/query/hooks/useDealsQuery.ts`:
- Around line 511-533: The current useAddDealItem applies the cache patch in
onSuccess which waits for the server; change to a true optimistic update by
moving the cache patch into onMutate inside useAddDealItem: generate a temporary
id for the new item, immediately update DEALS_VIEW_KEY via
queryClient.setQueryData to append the temp item to the matching deal, and
return a context object containing the previous cache snapshot and the temp id
for rollback; implement onError to restore the previous snapshot from that
context, implement onSuccess to replace the temp id with the real item returned
by dealsService.addItem (and merge any server-returned fields), and keep
onSettled to invalidate queryKeys.deals.detail(dealId) and
queryKeys.deals.lists(); reference functions/values: useAddDealItem, onMutate,
onError, onSuccess, queryClient, dealsService.addItem, DEALS_VIEW_KEY, and
queryKeys.deals.detail/lists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b482cc4b-fa75-4272-b5f1-d8b3d625e49e
📒 Files selected for processing (2)
lib/query/hooks/useDealsQuery.tsvercel.json
| { "path": "/api/cron/daily-briefing", "schedule": "0 8 * * 1-5" }, | ||
| { "path": "/api/cron/template-sync", "schedule": "0 6 * * *" }, | ||
| { "path": "/api/cron/stage-evaluations", "schedule": "* * * * *" } | ||
| { "path": "/api/cron/stage-evaluations", "schedule": "0 0 * * *" } |
There was a problem hiding this comment.
Revert cron frequency; daily scheduling delays stage advancement too much.
At Line 13, changing /api/cron/stage-evaluations from every minute to once per day introduces up to ~24h latency for queued stage evaluations, which regresses deal advancement freshness and can accumulate backlog (app/api/cron/stage-evaluations/route.ts:1-19, lib/ai/agent/agent.service.ts:669-690).
Suggested fix
- { "path": "/api/cron/stage-evaluations", "schedule": "0 0 * * *" }
+ { "path": "/api/cron/stage-evaluations", "schedule": "* * * * *" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { "path": "/api/cron/stage-evaluations", "schedule": "0 0 * * *" } | |
| { "path": "/api/cron/stage-evaluations", "schedule": "* * * * *" } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vercel.json` at line 13, The cron for the stage evaluations endpoint was
changed to daily, causing up to 24h latency for queued evaluations; revert the
schedule for the "/api/cron/stage-evaluations" entry back to a per-minute
cadence (e.g., "*/1 * * * *") so stage advancement runs frequently and avoids
backlog; update vercel.json's cron entry that references
"/api/cron/stage-evaluations" accordingly and ensure the change aligns with the
route handler at app/api/cron/stage-evaluations/route.ts and the evaluation
logic in lib/ai/agent/agent.service.ts.
Summary by CodeRabbit
Bug Fixes
Performance