-
-
Notifications
You must be signed in to change notification settings - Fork 191
feat: Validate event date range #406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Jean <jean@analoginterface.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 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 |
|
@tembo use isBefore from @repo/temporal instead |
Updated to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/schemas/src/events.ts">
<violation number="1" location="packages/schemas/src/events.ts:236">
Calling `isBefore(data.start, data.end)` without options will throw for mixed Temporal types/time zones allowed by `dateInputSchema`, causing validation to blow up instead of returning a proper error. Please guard these cases or provide the required compare options before calling `isBefore`.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
packages/schemas/src/events.ts
Outdated
| createdAt: z.instanceof(Temporal.Instant).optional(), | ||
| updatedAt: z.instanceof(Temporal.Instant).optional(), | ||
| }) | ||
| .refine((data) => isBefore(data.start, data.end), { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling isBefore(data.start, data.end) without options will throw for mixed Temporal types/time zones allowed by dateInputSchema, causing validation to blow up instead of returning a proper error. Please guard these cases or provide the required compare options before calling isBefore.
Prompt for AI agents
Address the following comment on packages/schemas/src/events.ts at line 236:
<comment>Calling `isBefore(data.start, data.end)` without options will throw for mixed Temporal types/time zones allowed by `dateInputSchema`, causing validation to blow up instead of returning a proper error. Please guard these cases or provide the required compare options before calling `isBefore`.</comment>
<file context>
@@ -205,33 +207,38 @@ export const recurrenceSchema = z
+ createdAt: z.instanceof(Temporal.Instant).optional(),
+ updatedAt: z.instanceof(Temporal.Instant).optional(),
+ })
+ .refine((data) => isBefore(data.start, data.end), {
+ message: "Event start time must be earlier than end time",
+ path: ["start"],
</file context>
✅ Addressed in b4d71dd
|
@tembo add a function to the temporal package to check whether two temporal objects are the same type plaindate, zoneddatetime, or instant. performed a check first in refine don't add any comments and choose the same style as we for example with it before function. |
Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed changes from recent commits (found 1 issue).
1 issue found across 2 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/schemas/src/events.ts">
<violation number="1" location="packages/schemas/src/events.ts:236">
The new refine short-circuits for mixed Temporal types, so events where the end precedes the start (e.g., PlainDate start vs Instant end) now pass validation. Please keep the comparison and fail when types differ instead of bypassing the check.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
packages/schemas/src/events.ts
Outdated
| createdAt: z.instanceof(Temporal.Instant).optional(), | ||
| updatedAt: z.instanceof(Temporal.Instant).optional(), | ||
| }) | ||
| .refine((data) => !isSameType(data.start, data.end) || isBefore(data.start, data.end), { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new refine short-circuits for mixed Temporal types, so events where the end precedes the start (e.g., PlainDate start vs Instant end) now pass validation. Please keep the comparison and fail when types differ instead of bypassing the check.
Prompt for AI agents
Address the following comment on packages/schemas/src/events.ts at line 236:
<comment>The new refine short-circuits for mixed Temporal types, so events where the end precedes the start (e.g., PlainDate start vs Instant end) now pass validation. Please keep the comparison and fail when types differ instead of bypassing the check.</comment>
<file context>
@@ -233,7 +233,7 @@ export const createEventInputSchema = z
updatedAt: z.instanceof(Temporal.Instant).optional(),
})
- .refine((data) => isBefore(data.start, data.end), {
+ .refine((data) => !isSameType(data.start, data.end) || isBefore(data.start, data.end), {
message: "Event start time must be earlier than end time",
path: ["start"],
</file context>
| .refine((data) => !isSameType(data.start, data.end) || isBefore(data.start, data.end), { | |
| .refine((data) => isSameType(data.start, data.end) && isBefore(data.start, data.end), { |
✅ Addressed in b4d71dd
|
@tembo resolve the merge conflict, fix the type errors, and run format |
Resolved the merge conflict, fixed type errors by adding explicit type guards for each Temporal type (PlainDate, Instant, ZonedDateTime) before calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 1 file
Description
Adds validation to the Events API to ensure the event's start date is before its end date using a Zod schema and
Temporal.Instant.compare().Screenshots / Recordings
Type of Change
Related Areas
Testing
Checklist
Notes
Summary by cubic
Validate event date ranges in the Events API to ensure start is before end. This blocks invalid events across create and update.
Written for commit b4d71dd. Summary will update automatically on new commits.