timetable import dialog#187
Merged
Merged
Conversation
# Conflicts: # apps/iris/src/routes/_private/admin/timetable/import.tsx
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughBackend update accepts optional timetable ChangesTimetable Management Features
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 Comment |
nemvince
previously approved these changes
May 20, 2026
![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 `@apps/iris/src/components/admin/timetable-edit-dialog.tsx`:
- Around line 73-79: The Label and Input need to be programmatically associated
for accessibility: add a stable id (e.g., "timetable-name") to the Input
component and set the Label's htmlFor prop to that same id so screen readers and
form tools can link them; update the JSX around Label and Input (the elements
currently using setName and value={name}) to include these props and keep
existing onChange/value behavior intact.
In `@apps/iris/src/components/admin/timetable-import-dialog.tsx`:
- Around line 35-143: The component TimetableImportDialog currently uses local
useState hooks (selectedFile, importName, validStartDate, validEndDate,
importStatus, errorMessage) and imperative handlers (validateAndSetFile,
handleFileSelect, handleImport, resetForm) — refactor it to TanStack Form:
create a form via useForm(...) with fields for name, file, validFrom, validTo
and move validation into the form schema; replace direct state reads with
reactive selectors using useStore(form.store, selector) for each slice instead
of the individual useState hooks; render inputs using <form.Field>{(field) =>
...}</form.Field> so the dialog UI binds to the form fields; update
importMutation to read values from form.getValues()/onSubmit and call mutate in
the form submit handler; and remove/reset local state by using form.reset() and
the form store selectors instead of resetForm/imperative fileInput manipulation.
- Around line 44-45: The form shows a date (DatePicker uses validStartDate ??
new Date()) while validation checks only validStartDate, causing submit to
remain disabled; update the validation used in the submit/required checks to use
the same effective value as the DatePicker (i.e., use (validStartDate ?? new
Date()) and (validEndDate ?? new Date()) or alternatively initialize
validStartDate/validEndDate to new Date()) so the displayed date and the
validation source match; apply the same change to the other occurrences
referenced (around the other DatePicker usages at the indicated locations).
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 9eda90b8-38ec-4feb-98cc-c8df94dafd0f
📒 Files selected for processing (6)
apps/chronos/src/routes/timetable/index.tsapps/iris/src/components/admin/sidebar.tsxapps/iris/src/components/admin/timetable-edit-dialog.tsxapps/iris/src/components/admin/timetable-import-dialog.tsxapps/iris/src/routes/_private/admin/timetable/import.tsxapps/iris/src/routes/_private/admin/timetable/manage.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/iris/src/components/admin/timetable-edit-dialog.tsx (1)
17-27: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy liftMigrate to TanStack Form pattern to match reference dialogs.
Both
card-dialog.tsxanduser-dialog.tsxuseuseForm,useStore(form.store, selector), and<form.Field>as prescribed by coding guidelines. This dialog uses plainuseState/useEffectinstead. Refactor to follow the TanStack Form pattern for consistency and to comply with the mandatory dialog structure guideline.🤖 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 `@apps/iris/src/components/admin/timetable-edit-dialog.tsx` around lines 17 - 27, The TimetableEditDialog currently uses local state/effects instead of the project's TanStack Form pattern; refactor the component that uses TimetableEditDialogProps to create a form via useForm(...) (with initial defaultValues derived from item), read form values with useStore(form.store, selector) where needed, replace existing input wiring with <form.Field> components bound to the form, and wire submission to form.handleSubmit to call the existing onSubmit(payload) with the mapped {name, validFrom, validTo} shape; ensure you remove useState/useEffect used for form fields and keep the prop names (item, isSubmitting, open, onOpenChange, onSubmit) unchanged.
♻️ Duplicate comments (1)
apps/iris/src/components/admin/timetable-import-dialog.tsx (1)
44-46:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
validStartDatedefault is still inconsistent after reset.Line 44 initializes correctly, but Line 98 resets to
undefinedwhile Line 174 still renders a fallback date and Line 292 requiresvalidStartDatetruthy. After reopen, users can see a date but cannot submit until re-selecting it.Suggested minimal fix
-const [validStartDate, setValidStartDate] = useState<Date | undefined>( - new Date() -); +const [validStartDate, setValidStartDate] = useState<Date>(new Date()); ... - setValidStartDate(undefined); + setValidStartDate(new Date()); ... - <DatePicker - date={validStartDate ?? new Date()} + <DatePicker + date={validStartDate} onDateChange={setValidStartDate} placeholder={t('timetable.validFromPlaceholder')} /> ... - if (!(selectedFile && importName.trim() && validStartDate)) { + if (!(selectedFile && importName.trim())) { return; } ... - !(selectedFile && importName.trim() && validStartDate) || + !(selectedFile && importName.trim()) || importStatus === 'uploading'🤖 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 `@apps/iris/src/components/admin/timetable-import-dialog.tsx` around lines 44 - 46, The issue is that validStartDate is initialized to new Date() but reset to undefined, causing the picker to show a fallback date while submit logic (and rendering) expects a truthy date; update the reset logic to setValidStartDate(new Date()) (instead of undefined) so the state, the date renderer (where validStartDate is used) and the submit validation (where a truthy validStartDate is required) remain consistent across open/reset/reopen; also verify any place that clears the picker uses setValidStartDate(new Date()) or explicitly handles undefined in the submit path (prefer keeping the single default behavior via setValidStartDate).
🤖 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.
Outside diff comments:
In `@apps/iris/src/components/admin/timetable-edit-dialog.tsx`:
- Around line 17-27: The TimetableEditDialog currently uses local state/effects
instead of the project's TanStack Form pattern; refactor the component that uses
TimetableEditDialogProps to create a form via useForm(...) (with initial
defaultValues derived from item), read form values with useStore(form.store,
selector) where needed, replace existing input wiring with <form.Field>
components bound to the form, and wire submission to form.handleSubmit to call
the existing onSubmit(payload) with the mapped {name, validFrom, validTo} shape;
ensure you remove useState/useEffect used for form fields and keep the prop
names (item, isSubmitting, open, onOpenChange, onSubmit) unchanged.
---
Duplicate comments:
In `@apps/iris/src/components/admin/timetable-import-dialog.tsx`:
- Around line 44-46: The issue is that validStartDate is initialized to new
Date() but reset to undefined, causing the picker to show a fallback date while
submit logic (and rendering) expects a truthy date; update the reset logic to
setValidStartDate(new Date()) (instead of undefined) so the state, the date
renderer (where validStartDate is used) and the submit validation (where a
truthy validStartDate is required) remain consistent across open/reset/reopen;
also verify any place that clears the picker uses setValidStartDate(new Date())
or explicitly handles undefined in the submit path (prefer keeping the single
default behavior via setValidStartDate).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c91d708f-40fa-4de3-8440-9a765ae482f5
📒 Files selected for processing (2)
apps/iris/src/components/admin/timetable-edit-dialog.tsxapps/iris/src/components/admin/timetable-import-dialog.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/iris/src/components/admin/timetable-import-dialog.tsx (2)
66-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid surfacing non-localized raw error text to users.
throw new Error('Failed to import timetable')can flow toAlertDescriptionviaerror.message, which bypasses localization. Use translated UI-safe text for displayed errors (and keep raw details out of user-facing strings).As per coding guidelines, "New user-facing error and success messages should go through
t(...)and the locale files, even when surfaced through toasts".🤖 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 `@apps/iris/src/components/admin/timetable-import-dialog.tsx` around lines 66 - 74, The code currently throws a raw Error('Failed to import timetable') and then surfaces error.message to the UI in the onError block; replace this flow so user-facing text always uses localization. At the throw site (where Error('Failed to import timetable') is created) either throw a non-user-facing error (with a machine code or leave the original message for logs only) or return a rejected value, and in the onError handler use t(...) for UI strings (replace setErrorMessage(error.message) and toast.error(...) with setErrorMessage(t('timetable.importError')) / toast.error(t('timetable.importError')) or a localized detailed message key), ensuring only localized strings reach AlertDescription and other UI components while raw error details are kept for logs only.
326-345:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit button types and localize the error message displayed in the alert.
The Import button without
type="submit"will default to form submission (standard HTML behavior) and theonClick={() => form.handleSubmit()}is redundant since the form'sonSubmitalready callsform.handleSubmit(). The Cancel button similarly needstype="button"to prevent unintended form submission. Additionally, line 322 displays{errorMessage}(the raw hardcoded English error string) in the Alert; this should use the localized key instead, consistent with how the toast surfacest('timetable.importError').Suggested fix pattern
<Button className="flex-1" disabled={!canImport} type="submit" > {/* ... */} </Button> <Button onClick={() => handleOpenChange(false)} type="button" variant="outline"> {t('common.cancel')} </Button>And in the error alert:
<AlertDescription>{t('timetable.importError')}</AlertDescription>🤖 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 `@apps/iris/src/components/admin/timetable-import-dialog.tsx` around lines 326 - 345, The Import and Cancel buttons in the timetable import dialog should use explicit types and the alert should use the localized error string: remove the redundant onClick that calls form.handleSubmit and set the Import <Button> to type="submit" so the form's onSubmit triggers form.handleSubmit; set the Cancel <Button> to type="button" and keep its onClick calling handleOpenChange(false); and replace the raw errorMessage shown in <AlertDescription> with the localized key via t('timetable.importError') so the Alert uses the translated message.
🤖 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 `@apps/iris/src/utils/form-schemas.ts`:
- Line 66: The schema's file field currently uses z.instanceof(File).nullable(),
allowing null to pass validation and causing the onSubmit handler's guard (if
(!file) return) to silently abort; update the schema's file field to require a
File (remove .nullable() or add a .refine(...) that enforces non-null and a
valid File) so validation fails when no file is provided, and ensure any error
messages propagate to the submit handler/UI; target the "file" schema entry in
apps/iris/src/utils/form-schemas.ts (the z.instanceof(File).nullable() symbol)
and adjust it rather than relying solely on the onSubmit check.
---
Outside diff comments:
In `@apps/iris/src/components/admin/timetable-import-dialog.tsx`:
- Around line 66-74: The code currently throws a raw Error('Failed to import
timetable') and then surfaces error.message to the UI in the onError block;
replace this flow so user-facing text always uses localization. At the throw
site (where Error('Failed to import timetable') is created) either throw a
non-user-facing error (with a machine code or leave the original message for
logs only) or return a rejected value, and in the onError handler use t(...) for
UI strings (replace setErrorMessage(error.message) and toast.error(...) with
setErrorMessage(t('timetable.importError')) /
toast.error(t('timetable.importError')) or a localized detailed message key),
ensuring only localized strings reach AlertDescription and other UI components
while raw error details are kept for logs only.
- Around line 326-345: The Import and Cancel buttons in the timetable import
dialog should use explicit types and the alert should use the localized error
string: remove the redundant onClick that calls form.handleSubmit and set the
Import <Button> to type="submit" so the form's onSubmit triggers
form.handleSubmit; set the Cancel <Button> to type="button" and keep its onClick
calling handleOpenChange(false); and replace the raw errorMessage shown in
<AlertDescription> with the localized key via t('timetable.importError') so the
Alert uses the translated message.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: ee0e0891-ebd2-4421-8f51-dc7e1aa45b49
📒 Files selected for processing (2)
apps/iris/src/components/admin/timetable-import-dialog.tsxapps/iris/src/utils/form-schemas.ts
This was referenced May 27, 2026
Merged
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
2 participants
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 by CodeRabbit
New Features
Refactor
UI