Added set current button and fixed validation issues on create campaign page#586
Added set current button and fixed validation issues on create campaign page#586aunali8812 merged 3 commits intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe PR widens the futureTimestamp boundary by 60 seconds, adds a "Set to current" start-date action and aggregated error handling in the campaign editor, and relaxes the form hook's isDisabled check to allow new campaigns to submit even when not dirty. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant Editor as "CampaignEditor"
participant Hook as "useCampaignForm"
participant DateUtil as "datetime.ts"
participant Validator as "react-hook-form"
User->>Editor: Click "Set to current"
Editor->>DateUtil: request now + 1 minute
DateUtil-->>Editor: timestamp
Editor->>Hook: form.setValue('start_ms', timestamp, { shouldDirty: true })
Hook->>Validator: trigger validation for start_ms
Validator-->>Hook: validation result
alt valid
User->>Editor: Submit form
Editor->>Validator: form.handleSubmit(values)
Validator-->>Editor: validated values
Editor->>Server: send payload (includes start_ms)
else invalid
Validator-->>Editor: fieldErrorMessages
Editor-->>User: display aggregated errors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 1
🤖 Fix all issues with AI agents
In `@src/entities/campaign/components/editor.tsx`:
- Around line 885-889: The submit Button currently sets disabled based on
isUpdate ? isDisabled : form.formState.isSubmitting which bypasses the shared
isDisabled hook in create mode; change the Button's disabled prop to always use
isDisabled (remove the ternary) so both create and update flows respect the
isDisabled logic (which already considers isValid, isSubmitting, token loading,
etc.) — update the Button element in editor.tsx (the one with props
variant="brand-filled" and type="submit") to use disabled={isDisabled}.
🧹 Nitpick comments (1)
src/common/lib/datetime.ts (1)
101-104: Reasonable grace window, but document the rationale.The 60-second tolerance aligns with the editor's
+1 minutebuffer inhandleStartNow/handleStartDateChange, preventing race conditions where a few seconds elapse between user selection and server-side validation. Consider adding a brief inline comment explaining why 60 seconds was chosen (e.g., to match the UI's 1-minute buffer), so future maintainers don't tighten or widen this arbitrarily.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/entities/campaign/components/editor.tsx`:
- Around line 156-160: handleStartNow currently updates only "start_ms" which
can leave "end_ms" stale and un-validated; after calling
form.setValue("start_ms", ...), also trigger re-validation of the end date (for
example call form.trigger("end_ms") or otherwise validate the "end_ms" field) so
any end-before-start errors surface immediately; update the handleStartNow
function to set start_ms as it does and then call form.trigger("end_ms") (or
trigger full form validation) to ensure the end date is re-checked.
🧹 Nitpick comments (1)
src/entities/campaign/components/editor.tsx (1)
96-102: Minor:correctedTimeduplicatesminTime.Both compute
Temporal.Now.instant().add({ minutes: 1 }).epochMilliseconds. You can reuseminTimedirectly.Suggested diff
if (selectedTime < minTime) { - // Auto-correct to minimum valid time (silently) - const correctedTime = Temporal.Now.instant().add({ minutes: 1 }).epochMilliseconds; - form.setValue("start_ms", correctedTime, { shouldValidate, shouldDirty: true }); + form.setValue("start_ms", minTime, { shouldValidate, shouldDirty: true }); } else {
| // "Set to current" — sets start date to right now | ||
| const handleStartNow = () => { | ||
| const startEpoch = Temporal.Now.instant().add({ minutes: 1 }).epochMilliseconds; | ||
| form.setValue("start_ms", startEpoch, { shouldDirty: true, shouldValidate: true }); | ||
| }; |
There was a problem hiding this comment.
handleStartNow doesn't trigger re-validation of end_ms.
If the user has already picked an end date and then clicks "Set to current", the new start_ms could land after end_ms. Since only start_ms is set here, end_ms won't be re-validated, potentially leaving the form in an inconsistent state with no visible error.
Consider triggering end_ms validation as well:
Suggested addition
const handleStartNow = () => {
const startEpoch = Temporal.Now.instant().add({ minutes: 1 }).epochMilliseconds;
form.setValue("start_ms", startEpoch, { shouldDirty: true, shouldValidate: true });
+ // Re-validate end date in case it's now before the new start
+ const endMs = form.getValues("end_ms");
+ if (endMs !== undefined) {
+ form.trigger("end_ms");
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // "Set to current" — sets start date to right now | |
| const handleStartNow = () => { | |
| const startEpoch = Temporal.Now.instant().add({ minutes: 1 }).epochMilliseconds; | |
| form.setValue("start_ms", startEpoch, { shouldDirty: true, shouldValidate: true }); | |
| }; | |
| // "Set to current" — sets start date to right now | |
| const handleStartNow = () => { | |
| const startEpoch = Temporal.Now.instant().add({ minutes: 1 }).epochMilliseconds; | |
| form.setValue("start_ms", startEpoch, { shouldDirty: true, shouldValidate: true }); | |
| // Re-validate end date in case it's now before the new start | |
| const endMs = form.getValues("end_ms"); | |
| if (endMs !== undefined) { | |
| form.trigger("end_ms"); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@src/entities/campaign/components/editor.tsx` around lines 156 - 160,
handleStartNow currently updates only "start_ms" which can leave "end_ms" stale
and un-validated; after calling form.setValue("start_ms", ...), also trigger
re-validation of the end date (for example call form.trigger("end_ms") or
otherwise validate the "end_ms" field) so any end-before-start errors surface
immediately; update the handleStartNow function to set start_ms as it does and
then call form.trigger("end_ms") (or trigger full form validation) to ensure the
end date is re-checked.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes / Improvements