-
Notifications
You must be signed in to change notification settings - Fork 13
fix(#200): show createdBy on edit modal and remove from table #212
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: develop
Are you sure you want to change the base?
Conversation
|
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. Summary by CodeRabbit
WalkthroughAdds optional createdBy to the todo form schema and displays it read-only in the Edit Todo modal. Removes the Created By column from table views. Updates utility to populate createdBy in default form data. No API signatures changed; only returned form data shape expanded. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant TL as Todo List
participant EM as Edit Modal (CreateEditTodoForm)
participant TU as TodoUtil
U->>TL: Click "Edit" on a task
TL->>TU: getDefaultTodoFormData(todo)
TU-->>TL: { ..., createdBy: {label, value} | undefined }
TL->>EM: Open modal with initialData
EM-->>U: Display form (Created By shown read-only if present)
U->>EM: Save / Close
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
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.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
| Unnecessary read-only data in form state ▹ view | ||
| Complex Inline Tailwind Classes ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| src/lib/todo-util.ts | ✅ |
| src/modules/teams/team-tasks.tsx | ✅ |
| src/components/todos/todo-list-table.tsx | ✅ |
| src/components/todos/create-edit-todo-form.tsx | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/components/todos/create-edit-todo-form.tsx(4 hunks)src/components/todos/todo-list-table.tsx(0 hunks)src/lib/todo-util.ts(1 hunks)src/modules/teams/team-tasks.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- src/components/todos/todo-list-table.tsx
- src/modules/teams/team-tasks.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-11T20:28:23.137Z
Learnt from: shobhan-sundar-goutam
PR: Real-Dev-Squad/todo-frontend#204
File: src/lib/todo-util.ts:65-70
Timestamp: 2025-09-11T20:28:23.137Z
Learning: In todo-frontend repository, TTaskCreatedBy.id is already a string type, so no type conversion is needed when mapping it to form schema fields that expect strings.
Applied to files:
src/components/todos/create-edit-todo-form.tsx
🔇 Additional comments (4)
src/lib/todo-util.ts (1)
65-70: LGTM! Clean implementation of createdBy field mapping.The mapping logic correctly transforms
todo.createdByinto the expected form data shape withlabelandvaluefields, and properly handles the case whencreatedByis absent by returningundefined. This aligns perfectly with the optional schema definition in the form component.src/components/todos/create-edit-todo-form.tsx (3)
19-19: LGTM! Proper icon import.The
UserIconimport follows best practices for lucide-react with named imports for tree-shaking. Based on learnings.
43-48: LGTM! Schema correctly defines createdBy as optional.The schema structure matches the data shape returned by
getDefaultTodoFormDatain todo-util.ts, withlabelandvaluefields. Marking it optional is appropriate since not all contexts may provide creator information.
143-143: LGTM! Proper initialization of createdBy field.The default value initialization correctly uses the optional chaining and fallback pattern consistent with other form fields.
| {mode === 'edit' && ( | ||
| <Controller | ||
| control={control} | ||
| name="createdBy" | ||
| render={({ field }) => ( | ||
| <FormInput | ||
| label="Created By" | ||
| htmlFor="createdBy" | ||
| icon={UserIcon} | ||
| errorMessage={errors.createdBy?.message} | ||
| > | ||
| <Input | ||
| {...field} | ||
| value={field.value?.label ?? '--'} | ||
| readOnly | ||
| className="cursor-default border-0 text-gray-700 shadow-none focus:ring-0 focus:outline-none focus-visible:ring-0" | ||
| /> | ||
| </FormInput> | ||
| )} | ||
| /> | ||
| )} |
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.
🛠️ Refactor suggestion | 🟠 Major
Simplify field props for read-only input.
The {...field} spread on line 350 passes unnecessary handlers (onChange, onBlur, etc.) to a read-only Input. For a read-only field, only name and ref are needed.
Apply this diff to pass only the necessary props:
<FormInput
label="Created By"
htmlFor="createdBy"
icon={UserIcon}
errorMessage={errors.createdBy?.message}
>
<Input
- {...field}
+ name={field.name}
+ ref={field.ref}
value={field.value?.label ?? '--'}
readOnly
className="cursor-default border-0 text-gray-700 shadow-none focus:ring-0 focus:outline-none focus-visible:ring-0"
/>
</FormInput>Alternatively, if you want to keep accessibility attributes, you can also add aria-readonly="true" for better screen reader support:
<Input
- {...field}
+ name={field.name}
+ ref={field.ref}
value={field.value?.label ?? '--'}
readOnly
+ aria-readonly="true"
className="cursor-default border-0 text-gray-700 shadow-none focus:ring-0 focus:outline-none focus-visible:ring-0"
/>📝 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.
| {mode === 'edit' && ( | |
| <Controller | |
| control={control} | |
| name="createdBy" | |
| render={({ field }) => ( | |
| <FormInput | |
| label="Created By" | |
| htmlFor="createdBy" | |
| icon={UserIcon} | |
| errorMessage={errors.createdBy?.message} | |
| > | |
| <Input | |
| {...field} | |
| value={field.value?.label ?? '--'} | |
| readOnly | |
| className="cursor-default border-0 text-gray-700 shadow-none focus:ring-0 focus:outline-none focus-visible:ring-0" | |
| /> | |
| </FormInput> | |
| )} | |
| /> | |
| )} | |
| {mode === 'edit' && ( | |
| <Controller | |
| control={control} | |
| name="createdBy" | |
| render={({ field }) => ( | |
| <FormInput | |
| label="Created By" | |
| htmlFor="createdBy" | |
| icon={UserIcon} | |
| errorMessage={errors.createdBy?.message} | |
| > | |
| <Input | |
| name={field.name} | |
| ref={field.ref} | |
| value={field.value?.label ?? '--'} | |
| readOnly | |
| className="cursor-default border-0 text-gray-700 shadow-none focus:ring-0 focus:outline-none focus-visible:ring-0" | |
| /> | |
| </FormInput> | |
| )} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In src/components/todos/create-edit-todo-form.tsx around lines 338 to 358, the
Controller currently spreads {...field} into a read-only Input which passes
unnecessary event handlers; replace the spread with only the minimal props
required for a read-only field (e.g., name, ref and value) and set readOnly plus
aria-readonly="true" on the Input to preserve accessibility while avoiding
onChange/onBlur handlers being passed.
Date: 30-09-2025
Developer Name: @MayankBansal12
Issue Ticket Number
created byin the modal, not on table #200Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
working proof
Screencast.mp4
Description by Korbit AI
What change is being made?
Show the Created By value in the edit modal and remove the Created By column from the todo list table; wire Created By through the TodoUtil mapping so the field is available in the edit form.
Why are these changes being made?
To display who created each todo in the edit view without cluttering the list UI; Created By remains in the data layer for consistency and future use.