Use specific rotation table selection state#435
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
WalkthroughThis change introduces rotation-specific state management by creating a factory pattern for filter stores, adding a new persistent rotation selection store with SSR-safe storage, and refactoring the rotation table component to use these new rotation-specific stores with proper TypeScript typing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/2026-01 #435 +/- ##
================================================
Coverage 88.07% 88.07%
================================================
Files 91 91
Lines 4621 4621
Branches 1492 1492
================================================
Hits 4070 4070
Misses 551 551
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BoraIneviNMI
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 `@fdm-app/app/components/blocks/rotation/table.tsx`:
- Around line 107-123: The handleSelection function currently builds
newSelection only from table.getFilteredRowModel().rows and calls
setSelection(newSelection), which drops selection state for filtered-out rows;
fix this by merging the existing selection state with the computed newSelection
instead of replacing it (use the existing selection object available in scope or
read current state) so hidden rows are preserved when calling setSelection;
ensure you create a new object (immutable merge) rather than mutating state
directly and keep references to RowSelectionState and the fields derived from
(row.original as CropRow).b_lu_catalogue and (fieldRow.original as
FieldRow).b_id to locate where to merge.
🧹 Nitpick comments (2)
fdm-app/app/store/rotation-selection.ts (1)
5-10: Consider renaming the interface to match its purpose.The interface is named
FieldSelectionStatebut it's used foruseRotationSelectionStore. For clarity and consistency with the store name, consider renaming it toRotationSelectionState.Suggested rename
-interface FieldSelectionState { +interface RotationSelectionState { farmId: string | null selection: Record<string, Record<string, boolean>> setSelection: (selection: Record<string, Record<string, boolean>>) => void syncFarm: (farmId: string) => void } -export const useRotationSelectionStore = create<FieldSelectionState>()( +export const useRotationSelectionStore = create<RotationSelectionState>()(fdm-app/app/components/blocks/rotation/table.tsx (1)
96-101: Consider extractingfieldFilter.syncFarmfor stable dependency reference.While the current code works, directly referencing
fieldFilter.syncFarmin the dependency array is safer when extracted likesyncFarmwas for the selection store.Suggested change
+ const syncFilterFarm = useRotationFilterStore((state) => state.syncFarm) + useEffect(() => { if (b_id_farm) { syncFarm(b_id_farm) - fieldFilter.syncFarm(b_id_farm) + syncFilterFarm(b_id_farm) } - }, [b_id_farm, syncFarm, fieldFilter.syncFarm]) + }, [b_id_farm, syncFarm, syncFilterFarm])
| function handleSelection(rowSelection: RowSelectionState) { | ||
| // Sync to store | ||
| const newSelection = Object.fromEntries( | ||
| table | ||
| .getFilteredRowModel() | ||
| .rows.map((row) => [ | ||
| (row.original as CropRow).b_lu_catalogue, | ||
| Object.fromEntries( | ||
| row.subRows.map((fieldRow) => [ | ||
| (fieldRow.original as FieldRow).b_id, | ||
| rowSelection[fieldRow.id], | ||
| ]), | ||
| ), | ||
| ]), | ||
| ) | ||
| setSelection(newSelection) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n fdm-app/app/components/blocks/rotation/table.tsx | head -150Repository: SvenVw/fdm
Length of output: 6107
🏁 Script executed:
sed -n '150,250p' fdm-app/app/components/blocks/rotation/table.tsxRepository: SvenVw/fdm
Length of output: 4004
🏁 Script executed:
sed -n '219,300p' fdm-app/app/components/blocks/rotation/table.tsxRepository: SvenVw/fdm
Length of output: 3400
🏁 Script executed:
rg -n "setSelection|selection\[" fdm-app/app/components/blocks/rotation/table.tsx -A 2 -B 2Repository: SvenVw/fdm
Length of output: 823
🏁 Script executed:
sed -n '107,130p' fdm-app/app/components/blocks/rotation/table.tsx | cat -nRepository: SvenVw/fdm
Length of output: 1111
Confirm: selection state of filtered-out rows is indeed lost.
The handleSelection function builds newSelection only from table.getFilteredRowModel().rows, then calls setSelection(newSelection), which completely replaces the stored selection. Any rows currently filtered out are excluded from this new selection object, causing their selection state to be discarded.
To preserve selection for hidden rows, merge the new selection with the existing one instead of replacing it entirely:
setSelection({ ...selection, ...newSelection })🤖 Prompt for AI Agents
In `@fdm-app/app/components/blocks/rotation/table.tsx` around lines 107 - 123, The
handleSelection function currently builds newSelection only from
table.getFilteredRowModel().rows and calls setSelection(newSelection), which
drops selection state for filtered-out rows; fix this by merging the existing
selection state with the computed newSelection instead of replacing it (use the
existing selection object available in scope or read current state) so hidden
rows are preserved when calling setSelection; ensure you create a new object
(immutable merge) rather than mutating state directly and keep references to
RowSelectionState and the fields derived from (row.original as
CropRow).b_lu_catalogue and (fieldRow.original as FieldRow).b_id to locate where
to merge.
2 participants
Enhancements
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.