feat: add useAppObjects hook and record mutation hooks#221
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…/delete) - Add useAppObjects hook to use-metadata.ts for fetching all object definitions belonging to an app - Add useCreateRecord, useUpdateRecord, useDeleteRecord mutation hooks to use-records.ts - Add tests for resolveFields utility, metadata hooks exports, and record hooks exports - All 65 tests pass, TypeScript type check passes Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…Objects Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds missing frontend data hooks in apps/web to complete Phase 1 “foundation” gaps by exposing app-scoped object metadata (useAppObjects) and CRUD mutation hooks for records, plus basic test scaffolding.
Changes:
- Added
useCreateRecord,useUpdateRecord, anduseDeleteRecordTanStack Query mutation hooks with cache invalidation. - Added
useAppObjects(appId)metadata hook to resolve an app’s object definitions concurrently with mock fallback. - Added tests for
resolveFields()and “hook export exists” validations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/hooks/use-records.ts | Adds record mutation hooks and query cache invalidation logic. |
| apps/web/src/hooks/use-metadata.ts | Adds useAppObjects to fetch all object definitions for an app. |
| apps/web/src/types/metadata.ts | Adds resolveFields() utility to normalize field definitions. |
| apps/web/src/tests/types/metadata.test.ts | Adds unit tests for resolveFields(). |
| apps/web/src/tests/hooks/use-records.test.ts | Adds export/type presence tests for record hooks. |
| apps/web/src/tests/hooks/use-metadata.test.ts | Adds export/type presence tests for metadata hooks. |
| /** | ||
| * TanStack Query hooks for CRUD record operations. | ||
| * | ||
| * Uses the official @objectstack/client SDK to fetch from the server. | ||
| * Falls back to mock data when the server is unreachable. | ||
| */ | ||
|
|
||
| import { useQuery } from '@tanstack/react-query'; | ||
| import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; | ||
| import type { RecordData, RecordListResponse } from '@/types/metadata'; | ||
| import { objectStackClient } from '@/lib/api'; | ||
| import { getMockRecords, getMockRecord } from '@/lib/mock-data'; |
There was a problem hiding this comment.
The file header says record hooks "fall back to mock data when the server is unreachable", but the newly added mutation hooks (create/update/delete) do not have any mock fallback and will error if the backend is down. Please update the header comment to reflect the actual behavior, or implement a consistent fallback strategy for mutations if that’s intended.
| const settled = await Promise.allSettled( | ||
| objectNames.map((name) => | ||
| objectStackClient.meta.getObject(name).then((r) => | ||
| r ? (r as ObjectDefinition) : getMockObjectDefinition(name), | ||
| ).catch(() => getMockObjectDefinition(name)), | ||
| ), | ||
| ); | ||
| return settled | ||
| .filter((r): r is PromiseFulfilledResult<ObjectDefinition | undefined> => | ||
| r.status === 'fulfilled') | ||
| .map((r) => r.value) | ||
| .filter((v): v is ObjectDefinition => !!v); |
There was a problem hiding this comment.
useAppObjects wraps each per-object fetch with .catch(() => getMockObjectDefinition(name)), so none of the mapped promises should reject. Given that, Promise.allSettled(...) plus filtering status === 'fulfilled' is redundant and adds complexity; consider switching to Promise.all(...) (or remove the inner catch if you truly want allSettled semantics).
| const settled = await Promise.allSettled( | |
| objectNames.map((name) => | |
| objectStackClient.meta.getObject(name).then((r) => | |
| r ? (r as ObjectDefinition) : getMockObjectDefinition(name), | |
| ).catch(() => getMockObjectDefinition(name)), | |
| ), | |
| ); | |
| return settled | |
| .filter((r): r is PromiseFulfilledResult<ObjectDefinition | undefined> => | |
| r.status === 'fulfilled') | |
| .map((r) => r.value) | |
| .filter((v): v is ObjectDefinition => !!v); | |
| const results = await Promise.all( | |
| objectNames.map((name) => | |
| objectStackClient.meta | |
| .getObject(name) | |
| .then((r) => (r ? (r as ObjectDefinition) : getMockObjectDefinition(name))) | |
| .catch(() => getMockObjectDefinition(name)), | |
| ), | |
| ); | |
| return results.filter((v): v is ObjectDefinition => !!v); |
| export function useCreateRecord({ objectName }: UseCreateRecordOptions) { | ||
| const queryClient = useQueryClient(); | ||
|
|
||
| return useMutation<RecordData, Error, Partial<RecordData>>({ | ||
| mutationFn: async (data) => { | ||
| const result = await objectStackClient.data.create(objectName, data); | ||
| return (result?.record ?? data) as RecordData; | ||
| }, | ||
| onSuccess: () => { | ||
| void queryClient.invalidateQueries({ queryKey: ['records', objectName] }); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // ── Update record ─────────────────────────────────────────────── | ||
|
|
||
| interface UseUpdateRecordOptions { | ||
| objectName: string; | ||
| recordId: string; | ||
| } | ||
|
|
||
| export function useUpdateRecord({ objectName, recordId }: UseUpdateRecordOptions) { | ||
| const queryClient = useQueryClient(); | ||
|
|
||
| return useMutation<RecordData, Error, Partial<RecordData>>({ | ||
| mutationFn: async (data) => { | ||
| const result = await objectStackClient.data.update(objectName, recordId, data); | ||
| return (result?.record ?? data) as RecordData; | ||
| }, | ||
| onSuccess: () => { | ||
| void queryClient.invalidateQueries({ queryKey: ['records', objectName] }); | ||
| void queryClient.invalidateQueries({ queryKey: ['record', objectName, recordId] }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The new mutation hooks add important cache-invalidation behavior (invalidate list queries; invalidate/remove detail queries), but the current tests only validate that the hooks are exported. Please add at least one behavioral test that asserts the expected query cache interactions (e.g., using a QueryClient wrapper + a mocked objectStackClient.data.* method) so regressions in query keys/invalidation are caught.
| export function useAppObjects(appId: string | undefined) { | ||
| const appQuery = useAppDefinition(appId); | ||
|
|
||
| return useQuery<ObjectDefinition[]>({ | ||
| queryKey: ['metadata', 'appObjects', appId], | ||
| queryFn: async () => { | ||
| const objectNames = appQuery.data?.objects ?? []; | ||
| const settled = await Promise.allSettled( | ||
| objectNames.map((name) => | ||
| objectStackClient.meta.getObject(name).then((r) => | ||
| r ? (r as ObjectDefinition) : getMockObjectDefinition(name), | ||
| ).catch(() => getMockObjectDefinition(name)), | ||
| ), | ||
| ); | ||
| return settled | ||
| .filter((r): r is PromiseFulfilledResult<ObjectDefinition | undefined> => | ||
| r.status === 'fulfilled') | ||
| .map((r) => r.value) | ||
| .filter((v): v is ObjectDefinition => !!v); | ||
| }, | ||
| enabled: !!appId && !!appQuery.data, | ||
| }); |
There was a problem hiding this comment.
useAppObjects contains non-trivial behavior (dependency on useAppDefinition, concurrent fetch with fallback-to-mock per object, and a specific queryKey). Current tests only validate that the hook is exported; please add a behavioral test that validates the resolution/fallback logic (e.g., when one object fetch rejects, the hook still returns mock for that object).
Completes the Phase 1 Foundation gaps: missing
useAppObjectsmetadata hook and CRUD mutation hooks for records.use-metadata.ts—useAppObjectsObjectDefinitionentries for an app by name, with mock fallbackPromise.allSettledfor concurrent fetchinguse-records.ts— Mutation hooksuseCreateRecord/useUpdateRecord/useDeleteRecordvia@objectstack/clientSDKTests
resolveFields()utility coverage (5 tests)💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.