refactor: consolidate mutation hooks and eliminate backend actions#445
Conversation
…ppers Phase 1: Remove ~40 thin backend action wrapper files, replace with useConvexMutation/useConvexActionMutation hooks that call Convex mutations/actions directly from the frontend. Phase 2: Consolidate all useConvexMutation hooks into mutations.ts files (previously split between actions.ts and mutations.ts). Now mutations.ts = all mutation hooks, actions.ts = only server actions. Phase 3: Convert useDeleteIntegrationAction to collection-based useDeleteIntegration and remove dead useAddMember hook. Clean up dead code: remove unused convex-server.ts (last consumer was deleted improve-message.ts action) and unused useSsoCredentials.
yannickmonney
force-pushed
the
refactor/collection-consolidation
branch
from
01d5fe8 to
0fff361
Compare
📝 WalkthroughWalkthroughThis pull request migrates the codebase from Convex action-based hook patterns to mutation-based patterns. New React hooks Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/app/features/settings/integrations/components/integration-manage-dialog.tsx (1)
587-608: 🧹 Nitpick | 🔵 Trivial
String(integration._id)is redundant but harmless.
integration._idis already astring-compatibleId<'integrations'>. TheString()call is unnecessary sinceuseDeleteIntegrationexpects{ integrationId: string }and ConvexIdtypes are string-branded. Not blocking, but could be simplified.Suggested simplification
- await deleteIntegration({ integrationId: String(integration._id) }); + await deleteIntegration({ integrationId: integration._id });
🤖 Fix all issues with AI agents
In
`@services/platform/app/features/custom-agents/components/custom-agent-active-toggle.test.tsx`:
- Around line 13-17: The mocked hooks return the wrong shape: update the vi.mock
for useActivateCustomAgentVersion and useUnpublishCustomAgent so each returns an
object with { mutateAsync, isPending } rather than a bare function — e.g. have
useActivateCustomAgentVersion: () => ({ mutateAsync: mockActivateVersion,
isPending: false }) and useUnpublishCustomAgent: () => ({ mutateAsync:
mockUnpublish, isPending: false }) so the component destructuring of {
mutateAsync: activateVersion, isPending: isActivating } works and the
mockActivateVersion/mockUnpublish calls are invoked.
In `@services/platform/app/hooks/use-convex-action-mutation.ts`:
- Around line 7-14: Add a stable mutationKey to useConvexActionMutation so each
action mutation is identifiable and filterable in React Query; update the
useMutation call in function useConvexActionMutation (which currently returns
useMutation({ mutationFn: (args: FunctionArgs<Func>) =>
convexClient.action(func, args) })) to include a deterministic key derived from
the function reference (e.g., ['convexAction', func] or another consistent
representation of Func) via the mutationKey property so DevTools and isMutating
filters can target specific action mutations.
In `@services/platform/app/hooks/use-convex-mutation.ts`:
- Around line 7-14: Add a stable mutationKey to the useConvexMutation hook for
consistency with useConvexActionMutation: in the useConvexMutation function
(which calls useConvexClient and returns useMutation), pass mutationKey: [func]
into the useMutation options alongside mutationFn so the hook uses the function
reference as the cache key.
| vi.mock('../hooks/mutations', async (importOriginal) => ({ | ||
| ...(await importOriginal()), | ||
| useActivateCustomAgentVersion: () => mockActivateVersion, | ||
| useUnpublishCustomAgent: () => mockUnpublish, | ||
| })); |
There was a problem hiding this comment.
Mock shape mismatch causes pipeline failures — hooks now return { mutateAsync, isPending }, not a bare function.
The component destructures { mutateAsync: activateVersion, isPending: isActivating } from the hook, but the mocks return the raw vi.fn(). This is the root cause of the two test failures (mockActivateVersion / mockUnpublish never called).
🐛 Fix mock return shape
vi.mock('../hooks/mutations', async (importOriginal) => ({
...(await importOriginal()),
- useActivateCustomAgentVersion: () => mockActivateVersion,
- useUnpublishCustomAgent: () => mockUnpublish,
+ useActivateCustomAgentVersion: () => ({
+ mutateAsync: mockActivateVersion,
+ isPending: false,
+ }),
+ useUnpublishCustomAgent: () => ({
+ mutateAsync: mockUnpublish,
+ isPending: false,
+ }),
}));📝 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.
| vi.mock('../hooks/mutations', async (importOriginal) => ({ | |
| ...(await importOriginal()), | |
| useActivateCustomAgentVersion: () => mockActivateVersion, | |
| useUnpublishCustomAgent: () => mockUnpublish, | |
| })); | |
| vi.mock('../hooks/mutations', async (importOriginal) => ({ | |
| ...(await importOriginal()), | |
| useActivateCustomAgentVersion: () => ({ | |
| mutateAsync: mockActivateVersion, | |
| isPending: false, | |
| }), | |
| useUnpublishCustomAgent: () => ({ | |
| mutateAsync: mockUnpublish, | |
| isPending: false, | |
| }), | |
| })); |
🤖 Prompt for AI Agents
In
`@services/platform/app/features/custom-agents/components/custom-agent-active-toggle.test.tsx`
around lines 13 - 17, The mocked hooks return the wrong shape: update the
vi.mock for useActivateCustomAgentVersion and useUnpublishCustomAgent so each
returns an object with { mutateAsync, isPending } rather than a bare function —
e.g. have useActivateCustomAgentVersion: () => ({ mutateAsync:
mockActivateVersion, isPending: false }) and useUnpublishCustomAgent: () => ({
mutateAsync: mockUnpublish, isPending: false }) so the component destructuring
of { mutateAsync: activateVersion, isPending: isActivating } works and the
mockActivateVersion/mockUnpublish calls are invoked.
| export function useConvexActionMutation< | ||
| Func extends FunctionReference<'action'>, | ||
| >(func: Func) { | ||
| const convexClient = useConvexClient(); | ||
|
|
||
| return useMutation({ | ||
| mutationFn: (args: FunctionArgs<Func>) => convexClient.action(func, args), | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a mutationKey for better React Query cache management.
Without a mutationKey, all mutations created by this hook share an implicit undefined key. This can hinder debugging in React Query DevTools and prevents targeted cache operations (e.g., isMutating({ mutationKey }) filters). A stable key derived from the function reference would improve observability.
💡 Suggested improvement
export function useConvexActionMutation<
Func extends FunctionReference<'action'>,
>(func: Func) {
const convexClient = useConvexClient();
return useMutation({
+ mutationKey: [func],
mutationFn: (args: FunctionArgs<Func>) => convexClient.action(func, args),
});
}📝 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.
| export function useConvexActionMutation< | |
| Func extends FunctionReference<'action'>, | |
| >(func: Func) { | |
| const convexClient = useConvexClient(); | |
| return useMutation({ | |
| mutationFn: (args: FunctionArgs<Func>) => convexClient.action(func, args), | |
| }); | |
| export function useConvexActionMutation< | |
| Func extends FunctionReference<'action'>, | |
| >(func: Func) { | |
| const convexClient = useConvexClient(); | |
| return useMutation({ | |
| mutationKey: [func], | |
| mutationFn: (args: FunctionArgs<Func>) => convexClient.action(func, args), | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@services/platform/app/hooks/use-convex-action-mutation.ts` around lines 7 -
14, Add a stable mutationKey to useConvexActionMutation so each action mutation
is identifiable and filterable in React Query; update the useMutation call in
function useConvexActionMutation (which currently returns useMutation({
mutationFn: (args: FunctionArgs<Func>) => convexClient.action(func, args) })) to
include a deterministic key derived from the function reference (e.g.,
['convexAction', func] or another consistent representation of Func) via the
mutationKey property so DevTools and isMutating filters can target specific
action mutations.
| export function useConvexMutation<Func extends FunctionReference<'mutation'>>( | ||
| func: Func, | ||
| ) { | ||
| const convexClient = useConvexClient(); | ||
|
|
||
| return useMutation({ | ||
| mutationFn: (args: FunctionArgs<Func>) => convexClient.mutation(func, args), | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same mutationKey suggestion as the action counterpart.
For consistency, consider adding mutationKey: [func] here as well — same reasoning as the useConvexActionMutation hook.
💡 Suggested improvement
export function useConvexMutation<Func extends FunctionReference<'mutation'>>(
func: Func,
) {
const convexClient = useConvexClient();
return useMutation({
+ mutationKey: [func],
mutationFn: (args: FunctionArgs<Func>) => convexClient.mutation(func, args),
});
}📝 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.
| export function useConvexMutation<Func extends FunctionReference<'mutation'>>( | |
| func: Func, | |
| ) { | |
| const convexClient = useConvexClient(); | |
| return useMutation({ | |
| mutationFn: (args: FunctionArgs<Func>) => convexClient.mutation(func, args), | |
| }); | |
| export function useConvexMutation<Func extends FunctionReference<'mutation'>>( | |
| func: Func, | |
| ) { | |
| const convexClient = useConvexClient(); | |
| return useMutation({ | |
| mutationKey: [func], | |
| mutationFn: (args: FunctionArgs<Func>) => convexClient.mutation(func, args), | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@services/platform/app/hooks/use-convex-mutation.ts` around lines 7 - 14, Add
a stable mutationKey to the useConvexMutation hook for consistency with
useConvexActionMutation: in the useConvexMutation function (which calls
useConvexClient and returns useMutation), pass mutationKey: [func] into the
useMutation options alongside mutationFn so the hook uses the function reference
as the cache key.
1 participant
Phase 1: Remove ~40 thin backend action wrapper files, replace with useConvexMutation/useConvexActionMutation hooks that call Convex mutations/actions directly from the frontend.
Phase 2: Consolidate all useConvexMutation hooks into mutations.ts files (previously split between actions.ts and mutations.ts). Now mutations.ts = all mutation hooks, actions.ts = only server actions.
Phase 3: Convert useDeleteIntegrationAction to collection-based useDeleteIntegration and remove dead useAddMember hook.
Clean up dead code: remove unused convex-server.ts (last consumer was deleted improve-message.ts action) and unused useSsoCredentials.
Summary by CodeRabbit