Add Zod schemas to common; infer types from schemas; compose schemas in app#80
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR converts many common TypeScript types to Zod schemas (adding zod dependency), updates app/server validation to derive from those schemas, tightens lint rules, and makes Firestore reads, Map accumulation, and UI non-null usage null-safe. ChangesZod Schema Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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 docstrings
🧪 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 |
rk234
changed the title
![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: 3
🧹 Nitpick comments (1)
common/src/types/invite.ts (1)
8-8: ⚡ Quick winValidate invite email format in the shared schema.
z.string()accepts malformed emails; using an email schema catches bad data earlier in the common contract.Proposed patch
- email: z.string(), + email: z.email(),🤖 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 `@common/src/types/invite.ts` at line 8, The invite schema currently defines the email property as a plain string (email: z.string()), which allows invalid emails; update the invite Zod schema's email field (the 'email' property in the invite schema object, e.g., InviteSchema or similar) to use Zod's email validator (z.string().email(...)) so malformed addresses are rejected and include an optional helpful message for failures.
🤖 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/server/functions/family.ts`:
- Around line 42-45: The current refine predicate only ensures the top-level
update object isn't empty but allows updates like { address: {} } to pass;
update the refine on the schema that extends AddressSchema.partial() so it also
checks that if data.address is present it has at least one key (e.g., change the
predicate to require Object.keys(data).length > 0 && (!data.address ||
Object.keys(data.address).length > 0)), keeping the existing error message or
adding a specific one for empty address; reference AddressSchema and the refine
call on the update schema to locate where to apply this change.
In `@app/src/server/functions/profile.ts`:
- Around line 76-80: The current updates schema uses UserProfileSchema.pick({
name: true, phone: true }).partial().refine(...) which silently strips unknown
keys; change it to enforce rejecting unknown fields by adding .strict() on the
picked/partial schema (e.g., call .strict() on the result of
UserProfileSchema.pick(...).partial()) so unknown update fields trigger
validation errors while preserving the existing .partial() and .refine()
behavior.
In `@common/src/types/profile-update.ts`:
- Around line 16-18: ChildProfileUpdateDataSchema currently uses
ChildSchema.omit({ id: true }).partial(), which still allows system and
relationship fields; replace it with a schema that explicitly picks only
editable profile fields from ChildSchema (e.g., name, nickname, birthDate,
gender, avatarUrl, medicalNotes — whatever set of editable attributes your
domain allows) and then apply .partial() for patch updates, ensuring you
explicitly exclude system/relationship fields such as id, createdAt, updatedAt,
parentId, familyId and any relation objects; update the declaration of
ChildProfileUpdateDataSchema to use ChildSchema.pick({...}).partial() (or an
equivalent approach) so only safe editable fields are accepted.
---
Nitpick comments:
In `@common/src/types/invite.ts`:
- Line 8: The invite schema currently defines the email property as a plain
string (email: z.string()), which allows invalid emails; update the invite Zod
schema's email field (the 'email' property in the invite schema object, e.g.,
InviteSchema or similar) to use Zod's email validator (z.string().email(...)) so
malformed addresses are rejected and include an optional helpful message for
failures.
🪄 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: 96b22cbf-d7e3-48f3-995f-50fd32a38623
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
app/src/lib/formSchemas.tsapp/src/server/functions/child.tsapp/src/server/functions/family.tsapp/src/server/functions/familyForm.tsapp/src/server/functions/profile.tscommon/package.jsoncommon/src/types/child.tscommon/src/types/claim.tscommon/src/types/family-link.tscommon/src/types/family.tscommon/src/types/gift-drive.tscommon/src/types/gift.tscommon/src/types/invite.tscommon/src/types/profile-update.tscommon/src/types/user.ts
Comment on lines
+16
to
+18
| export const ChildProfileUpdateDataSchema = ChildSchema.omit({ | ||
| id: true, | ||
| }).partial(); |
Contributor
There was a problem hiding this comment.
Narrow ChildProfileUpdateDataSchema to editable fields only.
Current shape allows system and relationship fields in profile update payloads, which can lead to unsafe state changes if applied without extra filtering.
Suggested fix
export const ChildProfileUpdateDataSchema = ChildSchema.omit({
id: true,
+ familyId: true,
+ giftDrive: true,
+ createdAt: true,
+ published: true,
+ category: true,
+ status: true,
+ livesAtHome: true,
}).partial();🤖 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 `@common/src/types/profile-update.ts` around lines 16 - 18,
ChildProfileUpdateDataSchema currently uses ChildSchema.omit({ id: true
}).partial(), which still allows system and relationship fields; replace it with
a schema that explicitly picks only editable profile fields from ChildSchema
(e.g., name, nickname, birthDate, gender, avatarUrl, medicalNotes — whatever set
of editable attributes your domain allows) and then apply .partial() for patch
updates, ensuring you explicitly exclude system/relationship fields such as id,
createdAt, updatedAt, parentId, familyId and any relation objects; update the
declaration of ChildProfileUpdateDataSchema to use
ChildSchema.pick({...}).partial() (or an equivalent approach) so only safe
editable fields are accepted.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/routes/_authenticated/staff/gifts.tsx (1)
14-16: ⚡ Quick winConsider conditional query construction instead of placeholder value.
Passing
activeDriveId ?? ""creates a query object with an invalid empty-stringdriveId, which is only prevented from running by theenabledguard. While functionally safe due to the guard, this pattern is fragile—if theenabledlogic is later modified or removed, the query would execute with an invalid parameter.♻️ Recommended: construct the query object conditionally
const { activeDriveId } = useDrive(); - const tableRowsByDriveQuery = publishedGiftsQueries.tableRowsByDrive( - activeDriveId ?? "", - ); + const tableRowsByDriveQuery = activeDriveId + ? publishedGiftsQueries.tableRowsByDrive(activeDriveId) + : { queryKey: ["publishedGifts", "tableRowsByDrive", null], queryFn: () => Promise.resolve([]) }; const { data: tableRows = [], isLoading } = useQuery({ ...tableRowsByDriveQuery, enabled: !!activeDriveId, });This makes it explicit that no valid query exists when
activeDriveIdis falsy, and eliminates the placeholder anti-pattern.🤖 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/routes/_authenticated/staff/gifts.tsx` around lines 14 - 16, The query object is built with a placeholder driveId (activeDriveId ?? ""), which risks creating an invalid query if the enabled guard changes; instead only construct the query object when activeDriveId is truthy: call publishedGiftsQueries.tableRowsByDrive(activeDriveId) conditionally (e.g. set tableRowsByDrive to undefined/null when no activeDriveId) and keep the existing enabled guard that checks activeDriveId to control execution. Update references that expect the query object to handle the possibly undefined value.
🤖 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/routes/_authenticated/staff/gifts.tsx`:
- Around line 14-16: The query object is built with a placeholder driveId
(activeDriveId ?? ""), which risks creating an invalid query if the enabled
guard changes; instead only construct the query object when activeDriveId is
truthy: call publishedGiftsQueries.tableRowsByDrive(activeDriveId) conditionally
(e.g. set tableRowsByDrive to undefined/null when no activeDriveId) and keep the
existing enabled guard that checks activeDriveId to control execution. Update
references that expect the query object to handle the possibly undefined value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01703d77-8ddb-4b85-9511-b4778b72d409
📒 Files selected for processing (22)
app/.oxlintrc.jsonapp/src/components/auth/ReauthAlertDialog.tsxapp/src/components/donor/home/ChildBlock.tsxapp/src/components/donor/home/ChildDetailSection.tsxapp/src/queries/publishedGifts.tsapp/src/routes/_authenticated/staff/gifts.tsxapp/src/routes/_authenticated/staff/route.tsxapp/src/server/functions/cart.tsapp/src/server/functions/child.tsapp/src/server/functions/family.tsapp/src/server/functions/familyForm.tsapp/src/server/functions/profile.tsapp/src/server/functions/storefront.tsapp/src/server/services/giftDriveService.server.tscommon/.oxlintrc.jsoncommon/src/types/child.tscommon/src/types/family.tscommon/src/types/gift-drive.tscommon/src/types/gift.tscommon/src/types/invite.tscommon/src/types/user.tsfunctions/.oxlintrc.json
✅ Files skipped from review due to trivial changes (2)
- functions/.oxlintrc.json
- common/.oxlintrc.json
🚧 Files skipped from review as they are similar to previous changes (8)
- common/src/types/invite.ts
- common/src/types/gift-drive.ts
- common/src/types/user.ts
- common/src/types/gift.ts
- common/src/types/family.ts
- common/src/types/child.ts
- app/src/server/functions/profile.ts
- app/src/server/functions/family.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.
Closes #79
Summary by CodeRabbit
Refactor
Chores