Family Review + Child Profile Page Tanstack DB Migration and related fixes#93
Merged
Conversation
…page, make approve and publish default
Contributor
📝 WalkthroughWalkthroughReplace per-entity React Query mutations with a TanStack DB collections factory and provider; add claim server endpoints and query keys; refactor profile and review components/routes to use transaction-backed drafts and live queries; remove legacy mutation hooks. ChangesTanStack DB Collections Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
Member
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
app/src/lib/serverValidation.ts (1)
45-50: ⚡ Quick winRemove redundant
instanceof Errorcheck.Since
erroris now typed asError, theinstanceofcheck at line 48 will always betrueand can be removed.♻️ Proposed fix
export function getServerValidationIssues( error: Error, ): Array<ValidationIssue> { - if (!(error instanceof Error)) { - return []; - } - try { const parsed = JSON.parse(error.message);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/lib/serverValidation.ts` around lines 45 - 50, In getServerValidationIssues, remove the redundant runtime guard "if (!(error instanceof Error)) { return []; }" because the parameter is already typed as Error; delete that conditional and its early return so the function directly processes the provided error (keeping existing logic that constructs/returns Array<ValidationIssue>), and ensure no other code paths relied on that branch.app/src/components/review/GuardianInfoCard.tsx (1)
232-236: 💤 Low value"Guardian comments" shows stale data while editing.
The readonly display at line 235 reads
family.privateNotes(the original prop) rather thanformState.privateNotes. While editing, users see the old value here even though their changes are staged informState. If this is intentional (showing "saved" vs "pending"), consider adding a visual indicator; otherwise, useformState.privateNotes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/review/GuardianInfoCard.tsx` around lines 232 - 236, The readonly "Guardian comments" display is currently rendering the original prop family.privateNotes instead of the editable state; update the rendering in GuardianInfoCard (the <p> showing "Guardian comments:") to use formState.privateNotes so the UI reflects staged edits, or if you intentionally want to show saved vs pending values add a clear visual indicator (e.g., "Saved" vs "Pending") and keep family.privateNotes for the saved value while showing formState.privateNotes as the pending draft.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/collections/factory.ts`:
- Around line 471-480: The createGiftsTransaction function currently only
processes update mutations (using isUpdateMutation) so insert mutations are
ignored; update the mutation loop in createGiftsTransaction to detect insert
mutations (e.g., isInsertMutation) and call the corresponding persistence
routine for inserts (e.g., persistGiftInsert or a newly implemented persist
function) in addition to the existing persistGiftUpdate call, ensuring both
insert and update branches are awaited, and keep the subsequent
invalidateGiftDerivedCaches call as-is so derived caches are invalidated after
all mutations are persisted.
- Around line 188-203: The photo upload is committed before the child record
update, causing partial state if updateChild fails; instead, when isDataUrl is
true call uploadChildPictureStaff first to obtain photoUrl but do not call
mutation.collection.utils.writeUpdate yet; include that photoUrl in the updates
object you build (i.e. stop excluding "photoUrl" from UPDATABLE_CHILD_FIELDS
when isDataUrl and add the returned photoUrl into updates), then await
updateChild({ data: { childId, updates } }); only after updateChild succeeds
call mutation.collection.utils.writeUpdate({ id: childId, photoUrl }) (or write
the full updates) so the photo change is persisted atomically from the client
perspective; reference functions: uploadChildPictureStaff, updateChild,
mutation.collection.utils.writeUpdate, UPDATABLE_CHILD_FIELDS, modified,
isDataUrl, photoUrl, childId.
In `@app/src/components/child-profile/ConfirmUnpublishModal.tsx`:
- Around line 37-39: The DialogDescription currently always says "This will
remove the profile from the storefront" in ConfirmUnpublishModal.tsx which is
incorrect for the publish action; update the component to render a conditional
description based on the action (e.g., if publishing show "This will add the
profile to the storefront." otherwise keep "This will remove the profile from
the storefront.") by branching where DialogDescription is rendered (use the
existing prop or state that indicates the action, e.g., isPublishing /
isUnpublishing or actionType) so the displayed message matches the operation.
In `@app/src/components/review/ChildCard.tsx`:
- Around line 480-489: The price handler writes directly to the DB
(collections.gifts.update) so price edits bypass the edit transaction and aren't
rolled back on Cancel; update the onPriceChange handler to validate with
parsePriceInput/hasValidListedPrice as before but call the existing editGift
transactional API (using the gift identifier, e.g., gift.id, and setting
listedPrice) instead of collections.gifts.update, awaiting the editGift call and
preserving the invalid-value toast.warning("Invalid price!"); this ensures price
changes go through the same transaction/undo flow as title/notes edits.
- Around line 238-240: The catch block in ChildCard.tsx currently only logs the
error ("Child review save failed") and provides no user feedback; update the
catch to surface the failure to the UI by either invoking the app's
toast/notification utility (e.g., showToast or similar) or by
rethrowing/propagating the error to the collection-level error handler used by
GuardianInfoCard and ReviewActionPanel; locate the save handler inside the
ChildCard component (the function containing the try/catch) and add a
user-facing notification with a clear message (e.g., "Failed to save child
review") and include the error details or pass the error to the common error
handler so the user sees the failure.
In `@app/src/components/review/GuardianInfoCard.tsx`:
- Around line 96-98: The catch block in the save handler of GuardianInfoCard
(where it currently calls console.error("Failed to save guardian info", error))
doesn't surface failures to users; mirror ChildCard's behavior by invoking
toast.error(...) with a clear message (e.g., "Failed to save guardian info") and
optionally include short error text, while still logging the full error to
console or processLogger for debugging; update the catch in the save function
(the try/catch that wraps the save logic in GuardianInfoCard) to call
toast.error in addition to the existing console.error.
In `@app/src/components/review/ReviewActionPanel.tsx`:
- Around line 63-93: runReviewUpdate currently updates only approved, held and
notes but ignores the audit fields lastReviewedAt and reviewedBy passed in
nextStatus, so those values are never persisted; update the
collections.families.update draft inside runReviewUpdate to also set
draft.reviewStatus.lastReviewedAt = nextStatus.lastReviewedAt and
draft.reviewStatus.reviewedBy = nextStatus.reviewedBy (respecting the
held/approved branches as needed) so the audit fields from nextStatus are saved
when the transaction commits.
---
Nitpick comments:
In `@app/src/components/review/GuardianInfoCard.tsx`:
- Around line 232-236: The readonly "Guardian comments" display is currently
rendering the original prop family.privateNotes instead of the editable state;
update the rendering in GuardianInfoCard (the <p> showing "Guardian comments:")
to use formState.privateNotes so the UI reflects staged edits, or if you
intentionally want to show saved vs pending values add a clear visual indicator
(e.g., "Saved" vs "Pending") and keep family.privateNotes for the saved value
while showing formState.privateNotes as the pending draft.
In `@app/src/lib/serverValidation.ts`:
- Around line 45-50: In getServerValidationIssues, remove the redundant runtime
guard "if (!(error instanceof Error)) { return []; }" because the parameter is
already typed as Error; delete that conditional and its early return so the
function directly processes the provided error (keeping existing logic that
constructs/returns Array<ValidationIssue>), and ensure no other code paths
relied on that branch.
🪄 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: 5ca4a765-3853-4c12-9c18-9f065034a147
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
app/package.jsonapp/src/collections/context.tsxapp/src/collections/factory.tsapp/src/collections/preload.tsapp/src/components/child-profile/ChildHeader.tsxapp/src/components/child-profile/ChildInfo.tsxapp/src/components/child-profile/ChildSidebar.tsxapp/src/components/child-profile/ConfirmUnpublishModal.tsxapp/src/components/child-profile/GiftInfoCard.tsxapp/src/components/child-profile/GiftInfoSection.tsxapp/src/components/child-profile/SelectedGifts.tsxapp/src/components/review/ChildCard.tsxapp/src/components/review/GuardianInfoCard.tsxapp/src/components/review/ReviewActionPanel.tsxapp/src/hooks/mutations/useCreateGift.tsapp/src/hooks/mutations/useUpdateChild.tsapp/src/hooks/mutations/useUpdateFamily.tsapp/src/hooks/mutations/useUpdateFamilyReviewStatus.tsapp/src/hooks/mutations/useUpdateGift.tsapp/src/hooks/mutations/useUploadChildPicture.tsapp/src/integrations/tanstack-query/root-provider.tsxapp/src/lib/serverValidation.tsapp/src/queries/claim.tsapp/src/queries/index.tsapp/src/router.tsxapp/src/routes/_authenticated/staff/child/$childId.tsxapp/src/routes/_authenticated/staff/review/$familyId.tsxapp/src/server/functions/child.tsapp/src/server/functions/family.tsapp/src/server/functions/profile.tsapp/src/server/services/childPhotoService.server.ts
💤 Files with no reviewable changes (6)
- app/src/hooks/mutations/useUploadChildPicture.ts
- app/src/hooks/mutations/useUpdateFamilyReviewStatus.ts
- app/src/hooks/mutations/useUpdateGift.ts
- app/src/hooks/mutations/useUpdateChild.ts
- app/src/hooks/mutations/useUpdateFamily.ts
- app/src/hooks/mutations/useCreateGift.ts
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/server/functions/child.ts (1)
1142-1159: 💤 Low valueConsider validating claim existence before update.
The function updates the claim document directly without verifying it exists. If an invalid
claimIdis passed, Firestore'supdate()will throw an error that may not have a clear message. A quick existence check improves error clarity.Proposed enhancement
.handler(async ({ data }) => { const { claimId, trackingNumber } = data; const db = getServerDB(); + const claimDoc = await db.claims.doc(claimId).get(); + if (!claimDoc.exists) { + throw new Error("Claim not found"); + } await db.claims.doc(claimId).update({ "purchaseConfirmation.trackingNumber": trackingNumber, }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/server/functions/child.ts` around lines 1142 - 1159, In updateClaimTrackingNumber: before calling db.claims.doc(claimId).update(...), fetch the document snapshot (e.g., db.claims.doc(claimId).get()) and check snapshot.exists; if it does not exist, return/throw a clear error (bad request/not found) indicating the claimId is invalid, otherwise proceed to call .update(...) as before; ensure the error path uses the same middleware/response conventions used elsewhere in the service.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/src/server/functions/child.ts`:
- Around line 1142-1159: In updateClaimTrackingNumber: before calling
db.claims.doc(claimId).update(...), fetch the document snapshot (e.g.,
db.claims.doc(claimId).get()) and check snapshot.exists; if it does not exist,
return/throw a clear error (bad request/not found) indicating the claimId is
invalid, otherwise proceed to call .update(...) as before; ensure the error path
uses the same middleware/response conventions used elsewhere in the service.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47136726-cae2-45bc-bc62-ecf16d893382
📒 Files selected for processing (10)
app/src/collections/factory.tsapp/src/components/child-profile/ConfirmUnpublishModal.tsxapp/src/components/child-profile/GiftInfoCard.tsxapp/src/components/review/ChildCard.tsxapp/src/components/review/GuardianInfoCard.tsxapp/src/components/review/ReviewActionPanel.tsxapp/src/lib/utils.tsapp/src/routes/_authenticated/staff/child/$childId.tsxapp/src/server/functions/child.tsapp/src/server/functions/gifts.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Type of Change
Related Issues (put task name here from notion)
Closes #73
Screenshots (If it is a front end feature screenshot is required)
Additional Notes
Summary by CodeRabbit
New Features
Refactor
Bug Fixes