refactor(ack-pay): extract timestamp schemas#104
Open
EfeDurmaz16 wants to merge 2 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
WalkthroughExtracts timestamp validation/normalization into module-local ChangesTimestamp schema consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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
🤖 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 `@packages/ack-pay/src/schemas/valibot.ts`:
- Around line 7-10: timestampSchema currently transforms a string/date by
calling new Date(input).toISOString(), which throws RangeError for invalid date
strings; change the schema to first validate that the input yields a valid Date
before transforming: within the v.pipe/v.union flow (timestampSchema) add a
refinement or pre-check (e.g., v.string().refine or a custom step) that parses
the input into a Date and checks !isNaN(date.getTime()), and only then call the
transform to toISOString(); ensure invalid inputs cause a validation error (via
refine failure) instead of allowing transform to throw.
In `@packages/ack-pay/src/schemas/zod/v3.ts`:
- Around line 7-9: The current timestampSchema uses .transform((val) => new
Date(val).toISOString()) which can throw RangeError for invalid strings; update
timestampSchema (the z.union of z.date() and z.string()) to first validate
inputs with a .refine() or .superRefine() that checks date validity (e.g., for
strings use Date.parse or new Date(val).getTime() and ensure it is not NaN, and
for Date objects ensure !isNaN(val.getTime())), and only then apply the
transform toISOString(), so the schema returns Zod validation errors instead of
throwing during the transform.
In `@packages/ack-pay/src/schemas/zod/v4.ts`:
- Around line 7-9: timestampSchema currently uses .transform((val) => new
Date(val).toISOString()) which can throw a RangeError for invalid date strings;
update the schema to validate the input before transforming by adding a
refinement (e.g., on the union of z.date() | z.string()) that checks the value
can be parsed to a valid Date (using Date.parse or isNaN(new
Date(...).getTime()) logic) and return a validation error if invalid, then
perform the .transform toISOString() only after the refinement succeeds; refer
to the timestampSchema, the .transform callback, and the z.union([z.date(),
z.string()]) as the locations to add the .refine()/.superRefine() check.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04a95d74-16ec-4e29-8b25-b13fb9551f5d
📒 Files selected for processing (3)
packages/ack-pay/src/schemas/valibot.tspackages/ack-pay/src/schemas/zod/v3.tspackages/ack-pay/src/schemas/zod/v4.ts
Author
|
Addressed the timestamp validation feedback in 224c426. Changes:
Verification:
|
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.
Summary
timestampSchemahelpersContext
Split out from the review feedback on #96, where the timestamp schema extraction was identified as useful independently of the approval model proposal.
Verification
corepack pnpm install --frozen-lockfilecorepack pnpm --filter @agentcommercekit/ack-pay... buildcorepack pnpm --filter @agentcommercekit/ack-pay test -- --runcorepack pnpm --filter @agentcommercekit/ack-pay check:typescorepack pnpm exec oxfmt --check packages/ack-pay/src/schemas/valibot.ts packages/ack-pay/src/schemas/zod/v3.ts packages/ack-pay/src/schemas/zod/v4.tsgit diff --checkNotes
distoutputs. Building the ACK-Pay dependency graph fixed that, and the rerun passed.AI Usage Disclosure
This contribution was AI-assisted using Codex CLI and the Codex app. AI assistance was used for repository/docs navigation, understanding ACK/Catena context, identifying relevant issues or contribution areas, and assisting with small edits/validation. I reviewed the final diff and take responsibility for the submitted changes.
Summary by CodeRabbit
Refactor
Bug Fix
Tests