Enhance Rotation Table Component to Replace Farm Create Wizard Cultivations Page#377
Merged
Conversation
🦋 Changeset detectedLatest commit: eae802d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
WalkthroughAdds expandable crop rows with per-field child rows and inline editors for start/end dates, variety, and crop-residue; introduces DateRangeDisplay, TableDateSelector, TableVarietySelector, an active-table-form store, a rotation action to persist edits, and integrates the rotation table into farm.create flows while removing legacy farm.create route modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as RotationTable UI
participant Store as ActiveFormStore
participant Fetcher as Router Fetcher
participant Server as Rotation Action
User->>UI: Expand crop row
UI->>UI: Render child field rows
User->>UI: Open inline editor (date/variety/residue)
UI->>Store: setActiveForm(cellId)
Store-->>UI: editor visible
User->>UI: Submit value
UI->>UI: compute cultivationIds & fieldIds
UI->>Fetcher: POST /rotation?action with form payload
Fetcher->>Server: deliver request
Server->>Server: validate (RotationTableFormSchema) and persist updates
Server-->>Fetcher: success
Fetcher-->>UI: response received
UI->>Store: clearActiveForm()
UI->>UI: close editor, refresh display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (9)📓 Common learnings📚 Learning: 2025-08-11T12:24:32.200ZApplied to files:
📚 Learning: 2025-02-13T08:35:59.306ZApplied to files:
📚 Learning: 2025-02-13T09:03:11.890ZApplied to files:
📚 Learning: 2025-01-23T15:18:57.212ZApplied to files:
📚 Learning: 2025-08-11T11:55:26.053ZApplied to files:
📚 Learning: 2025-01-24T11:39:57.805ZApplied to files:
📚 Learning: 2025-12-15T12:19:47.858ZApplied to files:
📚 Learning: 2026-01-15T09:39:38.225ZApplied to files:
🪛 Biome (2.1.2)fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx[error] 81-81: Avoid using the index of an array as key property in an element. This is the source of the key value. The order of the items may change, and this also affects performances and component state. (lint/suspicious/noArrayIndexKey) [error] 94-94: Avoid using the index of an array as key property in an element. This is the source of the key value. The order of the items may change, and this also affects performances and component state. (lint/suspicious/noArrayIndexKey) 🔇 Additional comments (3)
✏️ Tip: You can disable this entire section by setting 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 @@
## development #377 +/- ##
============================================
Coverage 88.04% 88.04%
============================================
Files 91 91
Lines 4591 4591
Branches 1468 1468
============================================
Hits 4042 4042
Misses 549 549
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:
|
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx (1)
44-90: Potential empty render for field rows with multiple harvests.When
cultivation.type === "field"andb_lu_harvestable === "multiple", the code enters this branch but then accessescultivation.fields(line 48) which is always an empty array forFieldRow(per the route loader at line 344). This results in an emptyharvestsByOrderarray and renders nothing.Consider handling field rows separately or skipping this branch for field rows:
if ( b_lu_harvest_date.length > 1 && cultivation.b_lu_harvestable === "multiple" ) { + // For field rows, just show the dates as a range since we don't have per-field breakdown + if (cultivation.type === "field") { + const sorted = [...b_lu_harvest_date].sort((a, b) => a.getTime() - b.getTime()) + return ( + <div className="flex items-start flex-col space-y-2"> + {sorted.map((date, idx) => ( + <p key={idx} className="text-muted-foreground"> + {`${idx + 1}e ${cultivation.b_lu_croprotation === "grass" ? "snede" : "oogst"}: ${format(date, "PP", { locale: nl })}`} + </p> + ))} + </div> + ) + } const b_lu_harvest_date_per_field = cultivation.fields.map(fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (1)
298-345: Add missing required properties toFieldRowobject or make them optional in the type definition.The field object constructed at lines 298-345 is missing 5 required properties from the
FieldRowtype (defined in columns.tsx lines 35-63):
m_cropresidue: stringb_lu_variety: stringb_lu_croprotation: stringb_lu_harvestable: "once" | "multiple" | "none"calendar: stringThree of these are available in scope and can be populated:
calendaris already accessible from the outer scopeb_lu_croprotationandb_lu_harvestablecan be derived fromcultivationsForCatalogue(similar to the parent crop object)Two properties (
m_cropresidueandb_lu_variety) are not available from the current data sources and would need either a different approach or to be marked optional in the type.This causes a TypeScript type mismatch that should be resolved.
fdm-app/app/components/blocks/rotation/columns.tsx (1)
277-287: Sorting function doesn't useb_areafor field rows.The
sortingFnalways reduces overfieldsarray, butFieldRowhasfields: [](empty) and its ownb_areaproperty. This causes all field rows to sort with area = 0 instead of their actual area.Apply this diff to handle both row types correctly:
sortingFn: (rowA, rowB, _columnId) => { - const areaA = rowA.original.fields.reduce( - (acc, field) => acc + field.b_area, - 0, - ) - const areaB = rowB.original.fields.reduce( - (acc, field) => acc + field.b_area, - 0, - ) + const getArea = (row: RotationExtended) => + row.type === "field" + ? row.b_area + : row.fields.reduce((acc, field) => acc + field.b_area, 0) + const areaA = getArea(rowA.original) + const areaB = getArea(rowB.original) return areaA - areaB },
🧹 Nitpick comments (2)
fdm-app/app/components/blocks/rotation/table.tsx (1)
144-159: Edge case:cutoffEndmay be -1 whenendRowis a leaf row without parent.At line 149-151, if
endRow.getParentRow()is falsy (meaningendRowis a parent row) butendRow.subRowsis empty,cutoffEndbecomes-1. The loop at line 152 would then iterate fromcutoffStartto-1, which executes 0 iterations—harmless but unintended.This scenario seems unlikely given the data model (crop rows should always have field subRows), but a guard would make the logic more robust:
const cutoffEnd = endRow.getParentRow() ? endRow.index - : endRow.subRows.length - 1 + : Math.max(0, endRow.subRows.length - 1)fdm-app/app/components/blocks/rotation/columns.tsx (1)
216-220: Sorting function for "Percelen" column may not handle field rows correctly.Similar to the area column, this sorting function assumes all rows have a meaningful
fields.length. ForFieldRow(child rows),fieldsis empty, so they all sort with length = 0. If child rows participate in sorting, consider adding a type guard. If only parent rows are sorted (with children inheriting position), this is acceptable but could be documented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/nine-lands-beg.md(1 hunks)fdm-app/app/components/blocks/rotation/columns.tsx(5 hunks)fdm-app/app/components/blocks/rotation/date-range-display.tsx(1 hunks)fdm-app/app/components/blocks/rotation/fertilizer-display.tsx(1 hunks)fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx(2 hunks)fdm-app/app/components/blocks/rotation/table.tsx(10 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/date-range-display.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/harvest-dates-display.tsxfdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/harvest-dates-display.tsxfdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/fertilizer-display.tsx.changeset/nine-lands-beg.md
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/harvest-dates-display.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/harvest-dates-display.tsxfdm-app/app/components/blocks/rotation/fertilizer-display.tsx
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsx
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
.changeset/nine-lands-beg.md
🧬 Code graph analysis (3)
fdm-app/app/components/blocks/rotation/table.tsx (2)
fdm-app/app/components/blocks/rotation/columns.tsx (2)
CropRow(22-36)FieldRow(38-65)fdm-core/src/db/schema.ts (1)
fields(92-108)
fdm-app/app/components/blocks/rotation/fertilizer-display.tsx (1)
fdm-core/src/db/schema.ts (2)
fields(92-108)fertilizers(136-144)
fdm-app/app/components/blocks/rotation/columns.tsx (3)
fdm-app/app/components/blocks/rotation/column-header.tsx (1)
DataTableColumnHeader(19-71)fdm-app/app/components/custom/cultivation-colors.ts (1)
getCultivationColor(20-28)fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
DateRangeDisplay(5-22)
🔇 Additional comments (11)
fdm-app/app/components/blocks/rotation/fertilizer-display.tsx (1)
49-56: Field-level fertilizer resolution looks correct.The pattern aligns with other display components. One subtle note: for
FieldRow,cultivation.fieldsis empty, sofields.every(...)at line 64-68 returnstrue(vacuous truth), resulting in full-shade icons. This is acceptable behavior for single-field display since the fertilizer is applied to "all" (one) field in that context.fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
5-22: Clean utility component with proper memoization.The component handles all date range cases correctly: empty/undefined returns
emptyContent, single date formats directly, and multiple dates sort before displaying as a range. Dependencies are correctly specified..changeset/nine-lands-beg.md (1)
1-5: Changeset accurately describes the feature.The minor version bump is appropriate for this new functionality, and the description clearly explains the expandable rows and field selection improvements.
fdm-app/app/components/blocks/rotation/table.tsx (4)
212-244: Search target construction is well-structured.The memoization correctly builds hierarchical search targets: field-level targets include field name and dates, while crop-level targets aggregate unique dates across all fields. The use of Sets for deduplication is appropriate.
280-294: Table expansion and filtering configuration is correct.
getSubRowsproperly returns fields for crop rows and undefined for field rowsfilterFromLeafRows: trueenables proper hierarchical filteringgetColumnCanGlobalFilteroptimization is sensible since fuzzy search uses the customsearchTargetfield
304-331: Selection memos correctly track hierarchical selection state.The
biome-ignorecomments appropriately document whyrowSelectionis needed in dependencies despite not being directly referenced—it triggers recomputation when selection changes. The separation ofselectedCultivations(crops with any selection) andselectedFields(explicitly selected field rows) aligns with the mixed selection model.
511-524: Row rendering correctly supports hierarchical selection states.The
data-stateattribute properly distinguishes between fully selected rows and partially selected crop rows (indeterminate). The CSS classes provide appropriate visual feedback for both states.fdm-app/app/components/blocks/rotation/columns.tsx (4)
70-92: LGTM! Expand/collapse column implementation is correct.The chevron rotation animation and conditional rendering based on
row.getCanExpand()is well implemented. Minor suggestion: returningnullinstead of""is slightly more idiomatic in React for empty renders, andclassName="cursor-pointer"could replace the inline style for consistency.
167-186: Verifydatetimesorting works correctly withDate[]arrays.Both
b_lu_startandb_lu_endusesortingFn: "datetime", but the data type isDate[](an array), not a singleDate. The built-in datetime sorting function may not handle arrays as expected—it might compare array references or fail to produce meaningful sort order.Consider implementing a custom sorting function that compares the first (or min/max) date in each array, similar to how you implemented custom sorting for
b_nameandb_areacolumns.
213-273: LGTM! Conditional rendering for crop-only dropdown is well implemented.The
cultivation.type === "crop"guard correctly prevents the dropdown from rendering on field rows. The sorting function and useMemo dependencies are appropriate for the data structure.
107-132: No changes needed—parent-to-child cascade is correctly implemented.The code properly cascades selection from parent to children via TanStack Table's default behavior. When
row.toggleSelected(!!value)is called without options, it defaults toselectChildren: true, automatically propagating the selection to all subrows. The explicitselectChildren: falsewhen updating the parent from a child (lines 127-129) prevents a double-cascade loop, which is the correct pattern for hierarchical selection.Likely an incorrect or invalid review comment.
Comment on lines
+38
to
65
| export type FieldRow = { | ||
| type: "field" | ||
| b_id: string | ||
| b_name: string | ||
| b_area: number | ||
| b_isproductive: boolean | ||
| a_som_loi: number | ||
| b_soiltype_agr: string | ||
| m_cropresidue: string | ||
| b_lu_variety: string | ||
| b_lu_harvest_date: Date[] | ||
| b_lu_croprotation: string | ||
| b_lu_harvestable: "once" | "multiple" | "none" | ||
| calendar: string | ||
| fields: { | ||
| b_id: string | ||
| b_name: string | ||
| b_area: number | ||
| b_isproductive: boolean | ||
| a_som_loi: number | ||
| b_soiltype_agr: string | ||
| b_lu_harvest_date: Date[] | ||
| fertilizerApplications: { | ||
| p_name_nl: string | ||
| p_id: string | ||
| p_type: string | ||
| }[] | ||
| fertilizers: { | ||
| p_name_nl: string | ||
| p_id: string | ||
| p_type: string | ||
| }[] | ||
| b_lu_start: Date[] | ||
| b_lu_end: Date[] | ||
| fertilizerApplications: { | ||
| p_name_nl: string | ||
| p_id: string | ||
| p_type: string | ||
| }[] | ||
| fertilizers: { | ||
| p_name_nl: string | ||
| p_id: string | ||
| p_type: string | ||
| }[] | ||
| fields: FieldRow[] | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how FieldRow.fields is used in the table component
rg -n "getSubRows|\.fields" fdm-app/app/components/blocks/rotation/table.tsx -A 3 -B 3Repository: SvenVw/fdm
Length of output: 1125
FieldRow type should not include fields: FieldRow[].
The getSubRows configuration in the table shows that only crop rows expand to display child field rows. Field rows are leaf nodes in the hierarchy and should not have sub-rows. The fields property on FieldRow is dead code and should be removed from the type definition to match the actual table behavior.
🤖 Prompt for AI Agents
In fdm-app/app/components/blocks/rotation/columns.tsx around lines 38 to 65, the
FieldRow type incorrectly includes a 'fields: FieldRow[]' property even though
field rows are leaf nodes and never have sub-rows; remove the 'fields' property
from the FieldRow type declaration so it matches the table's getSubRows behavior
and eliminate the dead code.
…iously hidden by it
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
fdm-app/app/components/blocks/rotation/columns.tsx (1)
64-64:FieldRow.fields: undefinedproperty is redundant in discriminated union.The
fields: undefinedproperty onFieldRowis unnecessary for the discriminated union to work. TypeScript can narrow the type based solely on thetypeproperty. While this pattern ensures type safety with optional chaining, it adds conceptual overhead and makes the type definition less clean.Consider removing the
fieldsproperty fromFieldRowentirely and using type guards where field access is needed:export type FieldRow = { type: "field" // ... other properties ... // Remove: fields: undefined }Then use type narrowing at access sites:
if (cultivation.type === "crop") { const fields = cultivation.fields // TypeScript knows this exists }
🧹 Nitpick comments (2)
fdm-app/app/components/blocks/rotation/table.tsx (1)
421-421: Consider encoding fieldIds for URL consistency.The
cultivationIdsare properly encoded usingencodeURIComponent, butfieldIdsare joined directly. While field IDs are typically safe strings, encoding them would provide consistency and defense-in-depth against potential special characters.Apply this diff:
- to={`/farm/${b_id_farm}/${calendar}/rotation/fertilizer?cultivationIds=${selectedCultivationIds.map(encodeURIComponent).join(",")}&fieldIds=${selectedFieldIds.join(",")}`} + to={`/farm/${b_id_farm}/${calendar}/rotation/fertilizer?cultivationIds=${selectedCultivationIds.map(encodeURIComponent).join(",")}&fieldIds=${selectedFieldIds.map(encodeURIComponent).join(",")}`}Apply the same pattern to Line 462 for the harvest link.
fdm-app/app/components/blocks/rotation/columns.tsx (1)
70-92: Consider using className instead of inline style for cursor.On Line 77, the cursor style is applied inline. For consistency with the rest of the codebase that uses Tailwind CSS classes, consider using
className="cursor-pointer"instead.Apply this diff:
<button type="button" onClick={row.getToggleExpandedHandler()} - style={{ cursor: "pointer" }} + className="cursor-pointer" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
fdm-app/app/components/blocks/rotation/columns.tsx(8 hunks)fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx(3 hunks)fdm-app/app/components/blocks/rotation/table.tsx(10 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- fdm-app/app/components/blocks/rotation/harvest-dates-display.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
🧬 Code graph analysis (2)
fdm-app/app/components/blocks/rotation/table.tsx (1)
fdm-app/app/components/blocks/rotation/columns.tsx (2)
CropRow(22-36)FieldRow(38-65)
fdm-app/app/components/blocks/rotation/columns.tsx (4)
fdm-app/app/components/blocks/fields/columns.tsx (1)
columns(43-279)fdm-app/app/components/blocks/rotation/column-header.tsx (1)
DataTableColumnHeader(19-71)fdm-app/app/components/custom/cultivation-colors.ts (1)
getCultivationColor(20-28)fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
DateRangeDisplay(5-22)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
Comment on lines
+104
to
+191
| if (event.shiftKey && lastSelectedRowIndex.current !== null) { | ||
| const currentIndex = row.index | ||
| const start = Math.min(currentIndex, lastSelectedRowIndex.current) | ||
| const end = Math.max(currentIndex, lastSelectedRowIndex.current) | ||
| document.getSelection()?.removeAllRanges() | ||
|
|
||
| const rowsToSelect = table | ||
| .getRowModel() | ||
| .rows.slice(start, end + 1) | ||
| .map((r) => r.id) | ||
| const parentRowModel = table.getFilteredRowModel() | ||
| const lastSelectedRowId = lastSelectedRowIndex.current | ||
| lastSelectedRowIndex.current = null | ||
| const lastSelectedRow = table.getRow(lastSelectedRowId) | ||
| if (lastSelectedRow) { | ||
| const mode = lastSelectedRow.getIsSelected() | ||
| const currentSelectedParentIndex = | ||
| row.getParentRow()?.index ?? row.index | ||
| const lastSelectedParentIndex = | ||
| lastSelectedRow.getParentRow()?.index ?? | ||
| lastSelectedRow.index | ||
| const startRow = | ||
| currentSelectedParentIndex < lastSelectedParentIndex | ||
| ? row | ||
| : currentSelectedParentIndex > lastSelectedParentIndex | ||
| ? lastSelectedRow | ||
| : row.index < lastSelectedRow.index | ||
| ? row | ||
| : lastSelectedRow | ||
| const endRow = | ||
| lastSelectedRow.id !== startRow.id ? lastSelectedRow : row | ||
| const parentRowStart = startRow.getParentRow() ?? startRow | ||
| const parentRowEnd = endRow.getParentRow() ?? endRow | ||
|
|
||
| const newRowSelection = { ...rowSelection } | ||
| rowsToSelect.forEach((id) => { | ||
| newRowSelection[id] = true | ||
| }) | ||
| setRowSelection(newRowSelection) | ||
| const newRowSelection = { ...rowSelection } | ||
| // Select-deselect all rows in between | ||
| for ( | ||
| let i = parentRowStart.index + 1; | ||
| i < parentRowEnd.index; | ||
| i++ | ||
| ) { | ||
| const row = parentRowModel.rows[i] | ||
| newRowSelection[row.id] = mode | ||
| row.subRows.forEach((subRow) => { | ||
| newRowSelection[subRow.id] = mode | ||
| }) | ||
| } | ||
| if (parentRowStart.id === parentRowEnd.id) { | ||
| // Select within one parent row | ||
| const cutoffStart = startRow.getParentRow() | ||
| ? startRow.index | ||
| : 0 | ||
| const cutoffEnd = endRow.getParentRow() | ||
| ? endRow.index | ||
| : endRow.subRows.length - 1 | ||
| for (let i = cutoffStart; i <= cutoffEnd; i++) { | ||
| const row = parentRowStart.subRows[i] | ||
| newRowSelection[row.id] = mode | ||
| } | ||
| newRowSelection[parentRowStart.id] = | ||
| parentRowStart.subRows.every( | ||
| (row) => newRowSelection[row.id], | ||
| ) | ||
| } else { | ||
| // For the start row, select only ones greater than the index | ||
| const cutoffStart = startRow.getParentRow() | ||
| ? startRow.index | ||
| : 0 | ||
| for ( | ||
| let i = cutoffStart; | ||
| i < parentRowStart.subRows.length; | ||
| i++ | ||
| ) { | ||
| const row = parentRowStart.subRows[i] | ||
| newRowSelection[row.id] = mode | ||
| } | ||
| newRowSelection[parentRowStart.id] = | ||
| parentRowStart.subRows.every( | ||
| (row) => newRowSelection[row.id], | ||
| ) | ||
| // For the end row, select only ones greater than the index | ||
| const cutoffEnd = endRow.getParentRow() | ||
| ? endRow.index | ||
| : endRow.subRows.length - 1 | ||
| for (let i = 0; i <= cutoffEnd; i++) { | ||
| const row = parentRowEnd.subRows[i] | ||
| newRowSelection[row.id] = mode | ||
| } | ||
| newRowSelection[parentRowEnd.id] = | ||
| parentRowEnd.subRows.every( | ||
| (row) => newRowSelection[row.id], | ||
| ) | ||
| } | ||
| setRowSelection(newRowSelection) | ||
| } |
Contributor
There was a problem hiding this comment.
Shift-click selection resets anchor, preventing consecutive shift-selections.
On Line 109, lastSelectedRowIndex.current is set to null immediately after reading it during a shift-click operation. This means subsequent shift-clicks won't enter the shift-selection logic (due to the null check on Line 104), preventing users from extending the selection multiple times with consecutive shift-clicks.
Typically, shift-click should update the anchor point to enable multiple consecutive range selections. Consider moving the reset to only occur when a new non-shift click is made, or updating the anchor to the end of the current selection range.
Example fix approach:
if (event.shiftKey && lastSelectedRowIndex.current !== null) {
document.getSelection()?.removeAllRanges()
const parentRowModel = table.getFilteredRowModel()
const lastSelectedRowId = lastSelectedRowIndex.current
- lastSelectedRowIndex.current = null
const lastSelectedRow = table.getRow(lastSelectedRowId)
if (lastSelectedRow) {
// ... selection logic ...
setRowSelection(newRowSelection)
+ // Update anchor to current row for next shift-click
+ lastSelectedRowIndex.current = row.id
}
} else {
// ... regular click logic ...
lastSelectedRowIndex.current = row.id
}📝 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.
Suggested change
| if (event.shiftKey && lastSelectedRowIndex.current !== null) { | |
| const currentIndex = row.index | |
| const start = Math.min(currentIndex, lastSelectedRowIndex.current) | |
| const end = Math.max(currentIndex, lastSelectedRowIndex.current) | |
| document.getSelection()?.removeAllRanges() | |
| const rowsToSelect = table | |
| .getRowModel() | |
| .rows.slice(start, end + 1) | |
| .map((r) => r.id) | |
| const parentRowModel = table.getFilteredRowModel() | |
| const lastSelectedRowId = lastSelectedRowIndex.current | |
| lastSelectedRowIndex.current = null | |
| const lastSelectedRow = table.getRow(lastSelectedRowId) | |
| if (lastSelectedRow) { | |
| const mode = lastSelectedRow.getIsSelected() | |
| const currentSelectedParentIndex = | |
| row.getParentRow()?.index ?? row.index | |
| const lastSelectedParentIndex = | |
| lastSelectedRow.getParentRow()?.index ?? | |
| lastSelectedRow.index | |
| const startRow = | |
| currentSelectedParentIndex < lastSelectedParentIndex | |
| ? row | |
| : currentSelectedParentIndex > lastSelectedParentIndex | |
| ? lastSelectedRow | |
| : row.index < lastSelectedRow.index | |
| ? row | |
| : lastSelectedRow | |
| const endRow = | |
| lastSelectedRow.id !== startRow.id ? lastSelectedRow : row | |
| const parentRowStart = startRow.getParentRow() ?? startRow | |
| const parentRowEnd = endRow.getParentRow() ?? endRow | |
| const newRowSelection = { ...rowSelection } | |
| rowsToSelect.forEach((id) => { | |
| newRowSelection[id] = true | |
| }) | |
| setRowSelection(newRowSelection) | |
| const newRowSelection = { ...rowSelection } | |
| // Select-deselect all rows in between | |
| for ( | |
| let i = parentRowStart.index + 1; | |
| i < parentRowEnd.index; | |
| i++ | |
| ) { | |
| const row = parentRowModel.rows[i] | |
| newRowSelection[row.id] = mode | |
| row.subRows.forEach((subRow) => { | |
| newRowSelection[subRow.id] = mode | |
| }) | |
| } | |
| if (parentRowStart.id === parentRowEnd.id) { | |
| // Select within one parent row | |
| const cutoffStart = startRow.getParentRow() | |
| ? startRow.index | |
| : 0 | |
| const cutoffEnd = endRow.getParentRow() | |
| ? endRow.index | |
| : endRow.subRows.length - 1 | |
| for (let i = cutoffStart; i <= cutoffEnd; i++) { | |
| const row = parentRowStart.subRows[i] | |
| newRowSelection[row.id] = mode | |
| } | |
| newRowSelection[parentRowStart.id] = | |
| parentRowStart.subRows.every( | |
| (row) => newRowSelection[row.id], | |
| ) | |
| } else { | |
| // For the start row, select only ones greater than the index | |
| const cutoffStart = startRow.getParentRow() | |
| ? startRow.index | |
| : 0 | |
| for ( | |
| let i = cutoffStart; | |
| i < parentRowStart.subRows.length; | |
| i++ | |
| ) { | |
| const row = parentRowStart.subRows[i] | |
| newRowSelection[row.id] = mode | |
| } | |
| newRowSelection[parentRowStart.id] = | |
| parentRowStart.subRows.every( | |
| (row) => newRowSelection[row.id], | |
| ) | |
| // For the end row, select only ones greater than the index | |
| const cutoffEnd = endRow.getParentRow() | |
| ? endRow.index | |
| : endRow.subRows.length - 1 | |
| for (let i = 0; i <= cutoffEnd; i++) { | |
| const row = parentRowEnd.subRows[i] | |
| newRowSelection[row.id] = mode | |
| } | |
| newRowSelection[parentRowEnd.id] = | |
| parentRowEnd.subRows.every( | |
| (row) => newRowSelection[row.id], | |
| ) | |
| } | |
| setRowSelection(newRowSelection) | |
| } | |
| if (event.shiftKey && lastSelectedRowIndex.current !== null) { | |
| document.getSelection()?.removeAllRanges() | |
| const parentRowModel = table.getFilteredRowModel() | |
| const lastSelectedRowId = lastSelectedRowIndex.current | |
| const lastSelectedRow = table.getRow(lastSelectedRowId) | |
| if (lastSelectedRow) { | |
| const mode = lastSelectedRow.getIsSelected() | |
| const currentSelectedParentIndex = | |
| row.getParentRow()?.index ?? row.index | |
| const lastSelectedParentIndex = | |
| lastSelectedRow.getParentRow()?.index ?? | |
| lastSelectedRow.index | |
| const startRow = | |
| currentSelectedParentIndex < lastSelectedParentIndex | |
| ? row | |
| : currentSelectedParentIndex > lastSelectedParentIndex | |
| ? lastSelectedRow | |
| : row.index < lastSelectedRow.index | |
| ? row | |
| : lastSelectedRow | |
| const endRow = | |
| lastSelectedRow.id !== startRow.id ? lastSelectedRow : row | |
| const parentRowStart = startRow.getParentRow() ?? startRow | |
| const parentRowEnd = endRow.getParentRow() ?? endRow | |
| const newRowSelection = { ...rowSelection } | |
| // Select-deselect all rows in between | |
| for ( | |
| let i = parentRowStart.index + 1; | |
| i < parentRowEnd.index; | |
| i++ | |
| ) { | |
| const row = parentRowModel.rows[i] | |
| newRowSelection[row.id] = mode | |
| row.subRows.forEach((subRow) => { | |
| newRowSelection[subRow.id] = mode | |
| }) | |
| } | |
| if (parentRowStart.id === parentRowEnd.id) { | |
| // Select within one parent row | |
| const cutoffStart = startRow.getParentRow() | |
| ? startRow.index | |
| : 0 | |
| const cutoffEnd = endRow.getParentRow() | |
| ? endRow.index | |
| : endRow.subRows.length - 1 | |
| for (let i = cutoffStart; i <= cutoffEnd; i++) { | |
| const row = parentRowStart.subRows[i] | |
| newRowSelection[row.id] = mode | |
| } | |
| newRowSelection[parentRowStart.id] = | |
| parentRowStart.subRows.every( | |
| (row) => newRowSelection[row.id], | |
| ) | |
| } else { | |
| // For the start row, select only ones greater than the index | |
| const cutoffStart = startRow.getParentRow() | |
| ? startRow.index | |
| : 0 | |
| for ( | |
| let i = cutoffStart; | |
| i < parentRowStart.subRows.length; | |
| i++ | |
| ) { | |
| const row = parentRowStart.subRows[i] | |
| newRowSelection[row.id] = mode | |
| } | |
| newRowSelection[parentRowStart.id] = | |
| parentRowStart.subRows.every( | |
| (row) => newRowSelection[row.id], | |
| ) | |
| // For the end row, select only ones greater than the index | |
| const cutoffEnd = endRow.getParentRow() | |
| ? endRow.index | |
| : endRow.subRows.length - 1 | |
| for (let i = 0; i <= cutoffEnd; i++) { | |
| const row = parentRowEnd.subRows[i] | |
| newRowSelection[row.id] = mode | |
| } | |
| newRowSelection[parentRowEnd.id] = | |
| parentRowEnd.subRows.every( | |
| (row) => newRowSelection[row.id], | |
| ) | |
| } | |
| setRowSelection(newRowSelection) | |
| // Update anchor to current row for next shift-click | |
| lastSelectedRowIndex.current = row.id | |
| } |
Collaborator
Author
There was a problem hiding this comment.
I expected the null check here to always pass, so I didn't think the ref update throughly. Can you see any way the null check wouldn't pass?
Contributor
There was a problem hiding this comment.
@BoraIneviNMI Yes, there are two scenarios where the null check would fail:
-
First interaction is a shift-click - When a user's very first click on the table is a shift-click (before any regular click establishes an anchor). The ref starts as
nullon Line 82, so if someone shift-clicks first, the check on Line 104 would fail. This is actually acceptable behavior since shift-selection needs an anchor point. -
Consecutive shift-clicks (the main issue) - After completing the first shift-click operation, Line 109 sets
lastSelectedRowIndex.current = null. So if a user tries to shift-click again to extend or modify the selection, the check on Line 104 will fail because the anchor was cleared. This prevents the multi-step shift-selection workflow.
The second scenario is what my comment was primarily flagging - it breaks the common UX pattern where users can perform multiple consecutive shift-clicks to refine their selection range. The current code only allows one shift-selection before requiring a regular click to re-establish the anchor.
To support consecutive shift-selections, you'd want to update the anchor to the current row's ID at the end of the shift-click logic (as shown in my suggestion), rather than nullifying it.
🧠 Learnings used
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
fdm-app/app/components/blocks/rotation/columns.tsx(9 hunks)fdm-app/app/components/blocks/rotation/schema.tsx(1 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(11 hunks)
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
📚 Learning: 2025-11-04T13:46:46.432Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-04T13:46:46.432Z
Learning: In the FDM app, file upload validation uses a two-layer approach: client-side Zod schemas (e.g., in fdm-app/app/components/blocks/soil/form-upload.tsx) provide early user-facing error feedback before upload, while server-side upload handlers enforce the same limits for security.
Applied to files:
fdm-app/app/components/blocks/rotation/schema.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/schema.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-07-31T11:38:50.661Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 194
File: fdm-core/src/harvest.ts:488-644
Timestamp: 2025-07-31T11:38:50.661Z
Learning: The validation logic in updateHarvest is intentionally different from checkHarvestDateCompability because updateHarvest is for updating existing harvests while checkHarvestDateCompability is for inserting new harvests. The insertion function includes checks that don't apply to updates, such as verifying no harvest already exists for "once" harvestable cultivations.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:28:00.183Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-core/src/cultivation.ts:572-597
Timestamp: 2025-02-13T08:28:00.183Z
Learning: In the FDM database schema, `b_id_harvesting` is a primary key in the `cultivationHarvesting` table, ensuring no duplicate harvest entries can exist. Therefore, duplicate checking is not needed when processing harvest records from database queries.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T12:14:28.354Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-core/src/db/schema.ts:370-0
Timestamp: 2025-08-11T12:14:28.354Z
Learning: In the FDM database schema, the field `b_lu_rest_oravib` in the `cultivations_catalogue` table is intentionally nullable (can be undefined). When undefined, it should be treated as `false` in the UI layer. This allows distinguishing between "explicitly set to false" and "not set" at the database level while providing simplified boolean logic in the UI.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (8)
fdm-app/app/components/blocks/rotation/columns.tsx (3)
29-74: LGTM! Discriminated union types properly structure the hierarchical data.The
CropRowandFieldRowtypes form a clean discriminated union with:
- Type discriminator (
type: "crop" | "field") enabling type-safe narrowingCropRow.fields: FieldRow[]for parent rows with nested childrenFieldRow.fields: undefinedcorrectly marking leaf nodesThis aligns with the expandable table model where only crop rows can expand.
114-142: Selection synchronization logic is correct but complex.The checkbox handler (lines 123-139) correctly propagates selections between parent and child rows:
- Child selection computes whether all siblings match the new state (lines 127-132)
- Parent selection updates only if needed (lines 133-137)
selectChildren: falseprevents infinite recursion (line 135)This implementation properly maintains consistency in the hierarchical selection model.
204-308: Crop residue column handles bulk operations with proper confirmation.The implementation correctly:
- Distinguishes between crop-level (bulk) and field-level operations (lines 220-223)
- Shows confirmation dialog only for multi-field crops (lines 299-302)
- Submits field IDs and cultivation IDs for server-side updates (lines 217-233)
- Maps three-state checkbox values (
all/some/none) to UI state (lines 289-296)fdm-app/app/components/blocks/rotation/schema.tsx (1)
1-22: LGTM! Form schema correctly handles string-to-boolean conversion.The preprocessing step (lines 10-17) properly converts form data string representations:
"true"→true"false"→false"null"→nullThis is necessary because HTML form submissions send all values as strings, and the schema coerces them to the expected
boolean | null | undefinedtype for the crop residue field.fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (4)
291-303: Crop residue aggregation logic correctly determines tri-state value.The implementation properly computes the aggregated crop residue state:
"all"when every field has residue for this cultivation"some"when only some fields have residue (mixed state)"none"when no fields have residueThis aligns with the three-state checkbox UI in the columns definition.
326-343: Date filtering correctly scopes to the current cultivation catalogue.Both
b_lu_startandb_lu_endnow filter bycultivation.b_lu_catalogue === b_lu_cataloguebefore extracting dates. This ensures that field rows only show dates for the specific cultivation being displayed, not dates from other cultivations on the same field.
512-579: Action function structure is sound.The implementation correctly:
- Validates form data using the schema (lines 518-521)
- Lazily loads fields only when needed (lines 524-529)
- Filters cultivations by the provided catalogue IDs (lines 554-556)
- Updates all matching cultivations across selected fields (lines 558-571)
- Returns success notification (lines 576-578)
560-569: No action needed. TheupdateCultivationfunction correctly accepts optional parameters (b_lu_catalogue,b_lu_start,b_lu_end,b_lu_variety) and properly handlesundefinedvalues. The implementation checks each parameter before updating (e.g.,if (b_lu_catalogue),if (b_lu_variety !== undefined)) and skips updates when parameters are undefined, preventing unintended field modifications.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
fdm-app/app/components/blocks/rotation/table.tsx (1)
219-220: Potential typo in date format string.Line 220 uses
"d MMMM yyy"with three 'y's. The standard format for a full year is"yyyy"(four 'y's). While"yyy"is technically valid, it's rarely used and typically only for historical dates with year < 1000.Apply this diff to use the standard year format:
const formatDate = (date: Date) => - format(date, "d MMMM yyy", { locale: nl }) + format(date, "d MMMM yyy", { locale: nl })
🧹 Nitpick comments (3)
fdm-app/app/components/blocks/rotation/table.tsx (1)
421-421: Verify URL encoding for field IDs.Line 421 encodes
cultivationIdsbut notfieldIds. For consistency and to prevent potential URL parsing issues with special characters in field IDs, consider encoding both:- to={`/farm/${b_id_farm}/${calendar}/rotation/fertilizer?cultivationIds=${selectedCultivationIds.map(encodeURIComponent).join(",")}&fieldIds=${selectedFieldIds.map(encodeURIComponent).join(",")}`} + to={`/farm/${b_id_farm}/${calendar}/rotation/fertilizer?cultivationIds=${selectedCultivationIds.map(encodeURIComponent).join(",")}&fieldIds=${selectedFieldIds.map(encodeURIComponent).join(",")}`}fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (2)
530-597: Verify action handles empty updates gracefully.The action function correctly validates input and filters cultivations. However, if
cultivationUpdatesis empty after the conditional checks (lines 554-559), the code still loads fields unnecessarily. Consider adding an early return:if ( typeof values.m_cropresidue !== "undefined" && values.m_cropresidue !== null ) { cultivationUpdates.m_cropresidue = values.m_cropresidue } if (Object.keys(cultivationUpdates).length > 0) { const fields = await loadFields() // ... rest of update logic + } else { + return dataWithSuccess(null, { + message: "Geen wijzigingen om toe te passen.", + }) } return dataWithSuccess(null, { message: "Succesvol bewerkt.", })
577-587: Consider documenting optional parameters for updateCultivation.The call to
updateCultivationpasses severalcultivationUpdatesproperties that may beundefined. Based on learnings,updateCultivationlikely accepts optional parameters after the required ones. Consider adding a brief comment to clarify which parameters are optional:cultivations.map((cultivation) => { + // updateCultivation accepts optional parameters; undefined values skip those updates return updateCultivation( fdm, session.principal_id, cultivation.b_lu, cultivationUpdates.b_lu_catalogue, cultivationUpdates.b_lu_start, cultivationUpdates.b_lu_end, cultivationUpdates.m_cropresidue, cultivationUpdates.b_lu_variety, ) }),Based on learnings, optional parameters after
fdmandb_iddon't need to be passed asundefined.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
fdm-app/app/components/blocks/rotation/columns.tsx(9 hunks)fdm-app/app/components/blocks/rotation/fertilizer-display.tsx(3 hunks)fdm-app/app/components/blocks/rotation/table.tsx(10 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(10 hunks)
🧰 Additional context used
🧠 Learnings (26)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-07-31T11:38:50.661Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 194
File: fdm-core/src/harvest.ts:488-644
Timestamp: 2025-07-31T11:38:50.661Z
Learning: The validation logic in updateHarvest is intentionally different from checkHarvestDateCompability because updateHarvest is for updating existing harvests while checkHarvestDateCompability is for inserting new harvests. The insertion function includes checks that don't apply to updates, such as verifying no harvest already exists for "once" harvestable cultivations.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:28:00.183Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-core/src/cultivation.ts:572-597
Timestamp: 2025-02-13T08:28:00.183Z
Learning: In the FDM database schema, `b_id_harvesting` is a primary key in the `cultivationHarvesting` table, ensuring no duplicate harvest entries can exist. Therefore, duplicate checking is not needed when processing harvest records from database queries.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
🧬 Code graph analysis (3)
fdm-app/app/components/blocks/rotation/fertilizer-display.tsx (2)
fdm-core/src/db/schema.ts (1)
fertilizers(136-144)fdm-app/app/components/blocks/rotation/columns.tsx (1)
FieldRow(47-75)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (2)
fdm-core/src/index.ts (5)
Field(112-112)getFields(107-107)Cultivation(56-56)getCultivations(49-49)updateCultivation(53-53)fdm-app/app/components/blocks/rotation/schema.tsx (1)
RotationTableFormSchema(3-18)
fdm-app/app/components/blocks/rotation/table.tsx (2)
fdm-app/app/components/blocks/rotation/columns.tsx (2)
CropRow(31-45)FieldRow(47-75)fdm-core/src/db/schema.ts (1)
fields(92-108)
🪛 Biome (2.1.2)
fdm-app/app/components/blocks/rotation/columns.tsx
[error] 324-324: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
🔇 Additional comments (10)
fdm-app/app/components/blocks/rotation/fertilizer-display.tsx (2)
49-57: LGTM: Field-specific fertilizer handling.The logic correctly prioritizes field-specific fertilizers for field-type cultivations and updates dependencies appropriately.
65-70: LGTM: Fertilizer usage check accounts for field type.The updated logic correctly treats "field" type cultivations as applying to all fields.
fdm-app/app/components/blocks/rotation/table.tsx (2)
283-284: LGTM: Row expansion correctly configured.The
getSubRowsimplementation correctly returns field arrays for crop rows andundefinedfor leaf field rows, enabling hierarchical expansion.
292-296: LGTM: Global filter optimizations.The configuration correctly limits global filtering to the "name" column and enables leaf-row filtering, improving performance while maintaining search functionality.
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (1)
326-343: LGTM: Field dates correctly filtered by cultivation.The
b_lu_startandb_lu_endarrays are now properly filtered to include only dates from cultivations matchingb_lu_catalogue, preventing date mixing from unrelated cultivations.fdm-app/app/components/blocks/rotation/columns.tsx (5)
80-102: LGTM: Expansion control column.The Children column correctly implements row expansion with appropriate visual feedback using the chevron rotation animation.
118-142: LGTM: Selection with parent-child synchronization.The checkbox logic correctly handles indeterminate states and synchronizes child selections with parent rows, maintaining hierarchical consistency.
220-237: LGTM: Crop residue update flow.The submit function correctly constructs the update payload, handling both crop and field types, and properly extracts the cultivation ID from either the parent or current row.
403-443: LGTM: Fields dropdown with responsive scrolling.The implementation correctly guards against undefined fields, provides responsive scrolling for longer lists, and includes proper navigation links.
475-487: LGTM: Area calculation handles both crop and field types.The logic correctly computes the total area for crops by summing nested field areas, and uses the direct
b_areaproperty for fields.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
fdm-app/app/components/blocks/rotation/table.tsx (1)
104-191: Shift-click selection still resets anchor, preventing consecutive shift-selections.The issue from the previous review remains: on Line 109,
lastSelectedRowIndex.currentis set tonullafter reading it during a shift-click. This prevents consecutive shift-clicks from extending the selection range. After the first shift-selection completes, users must click without shift to re-establish an anchor before shift-clicking again.Apply the suggested fix to update the anchor instead of nullifying it:
if (event.shiftKey && lastSelectedRowIndex.current !== null) { document.getSelection()?.removeAllRanges() const parentRowModel = table.getFilteredRowModel() const lastSelectedRowId = lastSelectedRowIndex.current - lastSelectedRowIndex.current = null const lastSelectedRow = table.getRow(lastSelectedRowId) if (lastSelectedRow) { // ... selection logic ... setRowSelection(newRowSelection) + lastSelectedRowIndex.current = row.id } } else {fdm-app/app/components/blocks/rotation/columns.tsx (1)
332-356: Array index as React key may cause issues with reordering.This was flagged in a previous review and by Biome. Using array index
ias the key form_cropresidue_endingitems can cause reconciliation issues if the array is reordered.If the date-value pairs have stable identity, use a composite key:
{row.original.m_cropresidue_ending.map( - ([date, value], i) => ( + ([date, value]) => ( <DropdownMenuItem - key={i} + key={`${date.getTime()}-${value}`} asChild
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/rotation/columns.tsx (1)
160-184: Hooks in cell render function may cause issues with React's rules of hooks.
useParams()is called inside the cell render function (Line 162). While this works in practice because the cell is a React component, it's unconventional and could cause issues if the table library changes how it renders cells. Consider moving route params to a higher level or passing them as context.Consider extracting params at the column definition level or using a wrapper component:
// Alternative: Create a cell component const FieldNameCell = ({ row }: { row: Row<RotationExtended> }) => { const params = useParams() const original = row.original // ... rest of render logic } // Then in column definition: cell: ({ row }) => <FieldNameCell row={row} />
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/components/blocks/rotation/columns.tsx(9 hunks)fdm-app/app/components/blocks/rotation/table.tsx(10 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsx
🧬 Code graph analysis (2)
fdm-app/app/components/blocks/rotation/columns.tsx (4)
fdm-app/app/components/blocks/fields/columns.tsx (1)
columns(43-279)fdm-app/app/components/blocks/rotation/column-header.tsx (1)
DataTableColumnHeader(19-71)fdm-app/app/components/custom/cultivation-colors.ts (1)
getCultivationColor(20-28)fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
DateRangeDisplay(5-22)
fdm-app/app/components/blocks/rotation/table.tsx (1)
fdm-app/app/components/blocks/rotation/columns.tsx (2)
CropRow(31-45)FieldRow(47-75)
🪛 Biome (2.1.2)
fdm-app/app/components/blocks/rotation/columns.tsx
[error] 335-335: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
🔇 Additional comments (12)
fdm-app/app/components/blocks/rotation/table.tsx (5)
212-243: LGTM! Date format and memoization look correct.The date format string now correctly uses
"d MMMM yyyy"with four 'y's (Line 220). The memoization properly builds search targets for both crop-level and field-level data, enabling fuzzy search across the hierarchy.
282-296: LGTM! Table configuration for hierarchical data.The expanded row model and sub-row configuration correctly handle the crop-field hierarchy. The
filterFromLeafRows: truesetting ensures filtering works properly with nested data.
306-328: Biome lint suppression comments are appropriate.The
rowSelectiondependency is correctly needed for the memos to recompute when selection changes, even though it's not directly referenced in the callback body. The table methods likegetFilteredRowModel()andgetFilteredSelectedRowModel()depend on the selection state.
420-422: URL construction properly encodes both cultivationIds and fieldIds.Both arrays are properly mapped through
encodeURIComponentbefore joining, which correctly handles special characters in IDs.
515-525: LGTM! Row state styling for hierarchical selection.The
data-stateattribute correctly handles three states:"selected"for fully selected rows,"indeterminate"for crop rows with partial child selection, and no state otherwise. The CSS classes provide appropriate visual feedback.fdm-app/app/components/blocks/rotation/columns.tsx (7)
47-75:FieldRow.fields: undefinedis intentional for type discrimination.While a past comment flagged this as dead code, having
fields: undefinedexplicitly onFieldRowserves a purpose: it enables exhaustive type checking when accessingrow.original.fields. The discriminated union pattern (type: "crop" | "field") with explicitfieldsproperty allows TypeScript to narrow the type correctly.
80-101: LGTM! Expand/collapse column implementation.The Children column correctly uses
row.getCanExpand()to conditionally render the toggle button, and the chevron rotation animation provides good visual feedback for the expanded state.
117-149: LGTM! Hierarchical checkbox selection logic.The checkbox correctly handles three states (selected, indeterminate, unchecked) and properly propagates selection changes to parent rows. The
selectChildren: falseoption prevents infinite loops when updating parent selection state.
186-205: LGTM! Date columns using DateRangeDisplay.Both Zaaidatum (start date) and Einddatum (end date) columns correctly use the
DateRangeDisplaycomponent to render date arrays, providing consistent formatting across the table.
398-457: LGTM! Percelen column with field dropdown.The fields dropdown correctly guards against non-crop rows with
cultivation.type === "crop", properly sorts fields alphabetically, and uses appropriate scroll behavior for long lists. TheuseMemodependencies are correctly specified.
459-502: LGTM! Area column with hierarchical calculation.The sorting and display logic correctly handles both crop rows (summing child field areas) and field rows (using
b_areadirectly). TheuseMemodependencies properly capture the values needed for recalculation.
226-248:useFetcherhook inside cell render function.Similar to
useParams,useFetcher()is called inside the cell function (Line 226). While functional, this is unusual. Each row's cell will have its own fetcher instance, which is likely intentional for independent submission handling, but verify this is the desired behavior.#!/bin/bash # Check if other column definitions in the codebase use hooks inside cell functions rg -n "cell:.*=>" fdm-app/app/components/blocks --type=tsx -A 10 | rg -B 5 "useFetcher|useParams|useState"
Comment on lines
+282
to
+286
| onClick={() => | ||
| submit(dialogValue === "all").then(() => | ||
| setDialogValue(undefined), | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
Use fetcher.state to detect submission completion instead of chaining .then().
While fetcher.submit() returns Promise<void> in React Router v7, the idiomatic pattern is to detect completion by watching fetcher.state in a useEffect hook rather than chaining promises.
Replace:
- onClick={() =>
- submit(dialogValue === "all").then(() =>
- setDialogValue(undefined),
- )
- }
+ onClick={() => {
+ submit(dialogValue === "all")
+ }}Then handle dialog close via useEffect:
React.useEffect(() => {
if (fetcher.state === "idle" && fetcher.data) {
setDialogValue(undefined)
}
}, [fetcher.state, fetcher.data])🤖 Prompt for AI Agents
In fdm-app/app/components/blocks/rotation/columns.tsx around lines 282–286, the
click handler closes the dialog by chaining .then() on submit; instead call
fetcher.submit(...) without chaining and add a React.useEffect that watches
fetcher.state and fetcher.data, and when fetcher.state === "idle" and
fetcher.data is present call setDialogValue(undefined); remove the .then()
closure and include fetcher.state and fetcher.data in the useEffect dependency
array so dialog closing is driven by fetcher lifecycle.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
fdm-app/app/components/blocks/rotation/columns.tsx (1)
353-377: Array index as React key may cause reconciliation issues.Line 356 uses the array index
ias the key. If them_cropresidue_endingarray is reordered or modified, React may incorrectly reconcile elements.Use the date as part of the key for stable reconciliation:
- key={i} + key={date.getTime()}Or if there's a possibility of duplicate dates:
- key={i} + key={`${date.getTime()}-${value}`}
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
fdm-app/app/components/blocks/rotation/columns.tsx(9 hunks)fdm-app/app/components/blocks/rotation/table.tsx(11 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-07-31T11:38:50.661Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 194
File: fdm-core/src/harvest.ts:488-644
Timestamp: 2025-07-31T11:38:50.661Z
Learning: The validation logic in updateHarvest is intentionally different from checkHarvestDateCompability because updateHarvest is for updating existing harvests while checkHarvestDateCompability is for inserting new harvests. The insertion function includes checks that don't apply to updates, such as verifying no harvest already exists for "once" harvestable cultivations.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:28:00.183Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-core/src/cultivation.ts:572-597
Timestamp: 2025-02-13T08:28:00.183Z
Learning: In the FDM database schema, `b_id_harvesting` is a primary key in the `cultivationHarvesting` table, ensuring no duplicate harvest entries can exist. Therefore, duplicate checking is not needed when processing harvest records from database queries.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
🧬 Code graph analysis (2)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (2)
fdm-core/src/index.ts (5)
Field(112-112)getFields(107-107)Cultivation(56-56)getCultivations(49-49)updateCultivation(53-53)fdm-app/app/components/blocks/rotation/schema.tsx (1)
RotationTableFormSchema(3-18)
fdm-app/app/components/blocks/rotation/columns.tsx (3)
fdm-app/app/components/blocks/rotation/column-header.tsx (1)
DataTableColumnHeader(19-71)fdm-app/app/components/custom/cultivation-colors.ts (1)
getCultivationColor(20-28)fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
DateRangeDisplay(5-22)
🪛 Biome (2.1.2)
fdm-app/app/components/blocks/rotation/columns.tsx
[error] 356-356: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @fdm-app/app/components/blocks/rotation/variety-selector.tsx:
- Around line 32-33: The code uses Object.keys(row.original[name]) named
currentSortedValues which doesn't sort by frequency; instead, convert
row.original[name] (the b_lu_variety Record<string, number>) to entries, sort by
the numeric count descending, map back to keys, and then set value to the first
key if any; update the logic around currentSortedValues and value to use this
sorted-by-count approach so the most common variety becomes the default.
🧹 Nitpick comments (2)
fdm-app/app/components/blocks/rotation/date-selector.tsx (1)
29-33: Add null safety for date array access.Line 32 accesses
value[0]without checking if the array is non-empty. Ifrow.original[name]is an empty array, this will result inundefined?.toISOString()which evaluates toundefined, but it's safer to be explicit.🛡️ Proposed defensive check
const value = row.original[name] const form = useForm({ defaultValues: { - [name]: value ? value[0]?.toISOString() : undefined, + [name]: value?.length ? value[0].toISOString() : undefined, }, })fdm-app/app/components/blocks/rotation/columns.tsx (1)
393-434: Consider memoization stability.The
useMemoat line 434 depends oncultivation.fields, which may cause re-renders if the array reference changes even when contents are identical. However, sincefieldsSortedis created by spreading and sorting, this is likely acceptable for the current use case.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
fdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/fertilizer-display.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-app/app/components/blocks/rotation/fertilizer-display.tsx
🧰 Additional context used
🧠 Learnings (41)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-07-31T11:38:50.661Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 194
File: fdm-core/src/harvest.ts:488-644
Timestamp: 2025-07-31T11:38:50.661Z
Learning: The validation logic in updateHarvest is intentionally different from checkHarvestDateCompability because updateHarvest is for updating existing harvests while checkHarvestDateCompability is for inserting new harvests. The insertion function includes checks that don't apply to updates, such as verifying no harvest already exists for "once" harvestable cultivations.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:28:00.183Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-core/src/cultivation.ts:572-597
Timestamp: 2025-02-13T08:28:00.183Z
Learning: In the FDM database schema, `b_id_harvesting` is a primary key in the `cultivationHarvesting` table, ensuring no duplicate harvest entries can exist. Therefore, duplicate checking is not needed when processing harvest records from database queries.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-11-25T14:50:16.074Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx:63-99
Timestamp: 2024-11-25T14:50:16.074Z
Learning: i18n will be added in future PRs; for now, hardcoded Dutch text is acceptable until a translation system is implemented.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-30T08:51:44.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 64
File: fdm-core/src/farm.ts:25-48
Timestamp: 2025-01-30T08:51:44.741Z
Learning: The `handleError` function always throws an error, but for better code clarity and to satisfy TypeScript's control flow analysis, it's recommended to add a return statement after it, even though it will never be reached.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-02-24T10:49:54.523Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 84
File: fdm-app/app/root.tsx:89-145
Timestamp: 2025-02-24T10:49:54.523Z
Learning: In the ErrorBoundary component of fdm-app/app/root.tsx, all client errors (400, 401, 403, 404) are intentionally displayed with a 404 status code for security purposes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-18T11:00:54.405Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 81
File: fdm-app/app/root.tsx:79-79
Timestamp: 2025-02-18T11:00:54.405Z
Learning: In React Router v7+, to ensure error pages are displayed correctly:
1. Export the ErrorBoundary component from the root route
2. Include the Scripts component in the Layout
3. Export the Layout through the default App component
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-18T11:00:54.405Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 81
File: fdm-app/app/root.tsx:79-79
Timestamp: 2025-02-18T11:00:54.405Z
Learning: In React Router v7+, error boundaries should:
1. Use the useRouteError hook to access the error
2. Be configured using the errorElement prop in route configuration
3. Include proper error type checking using isRouteErrorResponse
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-24T11:56:40.906Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/rollup.config.js:17-30
Timestamp: 2025-01-24T11:56:40.906Z
Learning: For the FDM project, keep error handling simple when basic error notification (console.error) is sufficient, avoiding unnecessary complexity in error handling mechanisms.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/routes/app.addfarm.$b_id_farm.cultivations.$b_lu_catalogue.fertilizers.tsx:1-1
Timestamp: 2024-12-16T10:56:07.561Z
Learning: The project uses `react-router` v7, and the `data` function is exported and used for error handling in loaders and actions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-01-14T16:06:24.294Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:1-1
Timestamp: 2025-01-14T16:06:24.294Z
Learning: In the fdm-app codebase, the `redirect` function should be imported from `react-router`, not `react-router-dom`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-01-17T08:30:02.880Z
Learnt from: BrentWHR
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-01-17T08:30:02.880Z
Learning: In markdown code blocks, title attributes should not have spaces between `title=` and the title string (e.g., ```typescript title="Example"```). Having a space will prevent the title from being properly parsed.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-14T16:06:21.832Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm.settings._index.tsx:1-1
Timestamp: 2025-01-14T16:06:21.832Z
Learning: In the fdm project, `redirect` and other routing utilities should be imported from `react-router` instead of `react-router-dom`.
Applied to files:
fdm-app/app/components/blocks/rotation/table.tsx
🧬 Code graph analysis (5)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (3)
fdm-core/src/authorization.ts (1)
checkPermission(159-215)fdm-app/app/components/blocks/rotation/columns.tsx (1)
CropRow(26-42)fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchema(3-43)
fdm-app/app/components/blocks/rotation/date-selector.tsx (5)
fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchemaType(45-47)fdm-app/app/components/blocks/rotation/columns.tsx (2)
RotationExtended(74-74)CropRow(26-42)fdm-app/app/components/custom/date-picker-v2.tsx (1)
DatePicker(41-166)fdm-app/app/store/active-table-form.tsx (1)
useActiveTableFormStore(3-15)fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
DateRangeDisplay(6-31)
fdm-app/app/components/blocks/rotation/table.tsx (1)
fdm-app/app/store/active-table-form.tsx (2)
clearActiveForm(12-14)useActiveTableFormStore(3-15)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (3)
fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchemaType(45-47)fdm-app/app/components/blocks/rotation/columns.tsx (2)
RotationExtended(74-74)CropRow(26-42)fdm-app/app/store/active-table-form.tsx (1)
useActiveTableFormStore(3-15)
fdm-app/app/components/blocks/rotation/columns.tsx (5)
fdm-app/app/components/blocks/rotation/column-header.tsx (1)
DataTableColumnHeader(19-71)fdm-app/app/components/custom/cultivation-colors.ts (1)
getCultivationColor(20-28)fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
DateRangeDisplay(6-31)fdm-app/app/components/blocks/rotation/date-selector.tsx (1)
TableDateSelector(94-137)fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
TableVarietySelector(115-174)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (1)
597-674: Action implementation looks good.The action correctly validates required parameters, uses schema validation, and employs
Promise.allfor atomic updates (if one fails, all fail). The permission checks are delegated to theupdateCultivationfunction in fdm-core.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/rotation/date-selector.tsx (1)
66-76: Consider adding error handling for the fetcher submission.If the action fails, the user receives no feedback. The form will remain visible (since
onHideonly runs on success via.then()), but there's no error message displayed. Consider checkingfetcher.datafor errors or adding a.catch()handler.♻️ Suggested improvement
fetcher .submit( { [name]: value }, { method: "POST", action: `?cultivationIds=${cultivationIds}&fieldIds=${fieldIds}`, }, ) .then(onHide) + .catch((error) => { + console.error("Failed to update date:", error) + // Consider adding toast notification or error state + })
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-app/app/components/blocks/rotation/variety-selector.tsx
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
📚 Learning: 2025-01-30T08:51:44.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 64
File: fdm-core/src/farm.ts:25-48
Timestamp: 2025-01-30T08:51:44.741Z
Learning: The `handleError` function always throws an error, but for better code clarity and to satisfy TypeScript's control flow analysis, it's recommended to add a return statement after it, even though it will never be reached.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsx
🔇 Additional comments (3)
fdm-app/app/components/blocks/rotation/date-selector.tsx (3)
1-17: LGTM!Imports are well-organized and the type definition appropriately constrains the allowed form fields to date-related properties.
128-132: Zustand state access pattern is correct.Using
useActiveTableFormStore.getState()inside the callback ensures you get the current state at execution time rather than a stale closure value. This is the recommended Zustand pattern for accessing state in callbacks that may be invoked asynchronously.
110-116: Good use ofstopPropagationfor inline editing.Preventing event bubbling ensures clicking the date cell doesn't trigger row expansion or other parent handlers, which is essential for the inline editing UX.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx (1)
904-918: Consider extracting the redirect URL to reduce duplication.The redirect URL construction is duplicated in the DELETE handler (lines 905-907) and the success handler (lines 1012-1014). Extracting this to a helper variable improves maintainability.
♻️ Suggested refactor
export async function action({ request, params }: ActionFunctionArgs) { try { const { b_id_farm, calendar = "all" } = params if (!b_id_farm) { throw new Error("Farm ID is missing") } const session = await getSession(request) const url = new URL(request.url) + const isCreateFlow = url.searchParams.has("create") + const rotationUrl = isCreateFlow + ? `/farm/create/${b_id_farm}/${calendar}/rotation` + : `/farm/${b_id_farm}/${calendar}/rotation` const fieldIds = url.searchParams.get("fieldIds")?.split(",").filter(Boolean) ?? []Then replace the inline ternaries:
return redirectWithSuccess( - url.searchParams.has("create") - ? `/farm/create/${b_id_farm}/${calendar}/rotation` - : `/farm/${b_id_farm}/${calendar}/rotation`, + rotationUrl, { message: `Oogst succesvol verwijderd van ${fieldIds.length} ${fieldIds.length === 1 ? "perceel" : "percelen"}.`, }, )return redirectWithSuccess( - url.searchParams.has("create") - ? `/farm/create/${b_id_farm}/${calendar}/rotation` - : `/farm/${b_id_farm}/${calendar}/rotation`, + rotationUrl, { message: `Oogst succesvol toegevoegd aan ${fieldIds.length} ${fieldIds.length === 1 ? "perceel" : "percelen"}.`, }, )Also applies to: 1011-1018
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.fertilizer._index.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
🧬 Code graph analysis (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx (1)
fdm-app/app/components/blocks/header/base.tsx (1)
Header(6-29)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (3)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx (3)
336-337: LGTM!Clean addition of the
createflag derived from search params. This correctly propagates the create-flow context to the component.
446-453: LGTM!Clean extraction of the backlink logic. The conditional URL construction is consistent and reused properly for both the header action and the NavLink.
570-572: LGTM!Consistent use of the
backlinkvariable for the fallback navigation.
…LoadingSpinner with Spinner
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
fdm-app/app/components/blocks/rotation/date-selector.tsx (1)
66-74: Consider handling submission errors before hiding the form.The form is hidden via
onHideregardless of whether the submission succeeds or fails. If the action returns an error, the user won't see feedback and may not realize their change wasn't saved.Consider checking
fetcher.datafor errors before hiding:♻️ Suggested approach
fetcher .submit( { [name]: value }, { method: "POST", action: `?cultivationIds=${cultivationIds}&fieldIds=${fieldIds}`, }, ) - .then(onHide) + .then(() => { + // Only hide if no error occurred + // Note: Check fetcher.data for action errors if needed + onHide?.() + })Alternatively, you could add error handling in a
useEffectwatchingfetcher.datato display error feedback to the user.fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
71-79: Same error handling consideration as date-selector.The
onHidecallback is called regardless of submission outcome. For consistency, consider applying the same error handling pattern across both components.fdm-app/app/components/blocks/rotation/columns.tsx (3)
287-311: Hooks called inside cell render function for m_cropresidue.Similar to the name column,
useFetcher()is called inside the cell render function. This creates a new fetcher instance for each cell on every render. While functional, this could be optimized.For cells that need fetcher functionality, consider:
- A wrapper component pattern
- Memoizing the cell component
- Lifting the fetcher state management
319-347: Duplicate checkbox rendering logic.The checkbox rendering for editable (
canModify: true) and read-only (canModify: false) cases duplicates thecheckedcalculation. Consider extracting the shared logic:♻️ Suggested refactor
+ const checkedState = ( + { + all: true, + some: "indeterminate", + none: false, + } as const + )[row.original.m_cropresidue] + {row.original.canModify ? ( <Checkbox id={inputId} - checked={ - ( - { - all: true, - some: "indeterminate", - none: false, - } as const - )[row.original.m_cropresidue] - } + checked={checkedState} onCheckedChange={(value) => submit(!!value)} /> ) : ( <Checkbox id={inputId} - checked={ - ( - { - all: true, - some: "indeterminate", - none: false, - } as const - )[row.original.m_cropresidue] - } + checked={checkedState} disabled={true} /> )}
161-191: Extract router dependencies from cell render function.
useParams()anduseLocation()are called inside the cell function (lines 163-164). While functional within React's render cycle, this pattern tightly couples the column to React Router and complicates testing. Sincerotation/table.tsxalready accesses these hooks at the component level, consider refactoring:
- Pass
paramsandlocationto the columns definition as a factory function- Use context to provide router values to the table
- Move NavLink construction logic to a wrapper component
This improves reusability and makes the columns testable independent of router state.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
fdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
📚 Learning: 2025-01-30T08:51:44.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 64
File: fdm-core/src/farm.ts:25-48
Timestamp: 2025-01-30T08:51:44.741Z
Learning: The `handleError` function always throws an error, but for better code clarity and to satisfy TypeScript's control flow analysis, it's recommended to add a return statement after it, even though it will never be reached.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/date-selector.tsxfdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
🧬 Code graph analysis (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (3)
fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchemaType(45-47)fdm-app/app/components/blocks/rotation/columns.tsx (2)
RotationExtended(74-74)CropRow(26-42)fdm-app/app/store/active-table-form.tsx (1)
useActiveTableFormStore(3-15)
🔇 Additional comments (11)
fdm-app/app/components/blocks/rotation/date-selector.tsx (2)
29-34: Date array handling takes only the first value.The component initializes the form with only the first date from the
valuearray (value[0]). This is intentional for bulk editing (setting a single date for all fields), but worth documenting this behavior for future maintainers.
94-137: Clean implementation of the inline date editor toggle.The component correctly:
- Uses Zustand store to track active form state
- Stops event propagation on click to prevent row selection
- Shows spinner during fetch operations
- Properly clears active form state in the
onHidecallback usinggetState()patternfdm-app/app/components/blocks/rotation/variety-selector.tsx (3)
84-86: Good separation of disabled states.The
Selectis disabled when there are no variety options (b_lu_variety_options.length === 0), while theSelectTriggeris disabled when the user lacks modification permissions (!row.original.canModify). These are correctly separate conditions addressing different concerns.
134-142: Clear and user-friendly display logic.The
renderTextfunction handles edge cases well:
- Empty string when no options exist
- "niet aangegeven" when no variety selected
- Comma-separated list for ≤5 items
- Truncated with "en meer" for >5 items
144-146: Intentional read-only rendering for crop rows.Crop rows correctly show text-only display (aggregated summary) while field rows show the editable dropdown. This aligns with the parent/child editing pattern described in the PR objectives.
fdm-app/app/components/blocks/rotation/columns.tsx (6)
26-42: Well-structured discriminated union types.The
CropRowandFieldRowtypes withtypediscriminators enable clean type narrowing throughout the column definitions. Thefields?: undefinedonFieldRow(line 71) ensures TypeScript correctly handles the mutual exclusivity.Also applies to: 44-72
76-100: Clean expand/collapse implementation.The Children column correctly:
- Checks
row.getCanExpand()before rendering the toggle- Uses CSS transition for smooth chevron rotation
- Returns empty string for non-expandable rows
129-144: Robust parent-child selection synchronization.The selection logic correctly synchronizes parent row selection based on child row states. When a child is toggled, it checks if all siblings (including the toggled one with its new value) are selected, then updates the parent accordingly with
selectChildren: falseto prevent infinite loops.
233-251: Good conditional rendering for end date based on harvestability.For cultivations with
b_lu_harvestable !== "multiple", the end date shows a tooltip explaining users should add a harvest instead of editing the end date directly. For "multiple" harvestable cultivations, the inline editor is available. This is a thoughtful UX decision.
466-478: Area calculation with proper type narrowing.The area calculation correctly handles both field rows (direct
b_area) and crop rows (sum of child fields' areas). The useMemo dependencies are correctly specified.
393-434: Correct handling of fields dropdown for crop rows only.The Percelen column correctly:
- Only renders dropdown for crop rows (
cultivation.type === "crop")- Handles empty/undefined fields with nullish coalescing
- Uses proper memoization with correct dependencies
- Uses valid relative path navigation to field routes (calendar parameter comes from route params)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
fdm-app/app/components/blocks/rotation/columns.tsx (4)
30-46: Type inconsistency form_cropresiduebetweenCropRowandFieldRow.
CropRow.m_cropresidueis typed asstring(line 36), whileFieldRow.m_cropresidueis correctly typed as"all" | "some" | "none"(line 57). The cell renderer at lines 319-325 and 346-354 indexes into an object using these exact keys, which would fail silently or produceundefinedifCropRowreceives an unexpected string value.Suggested fix
export type CropRow = { type: "crop" canModify: boolean b_lu_catalogue: string b_lu: string[] b_lu_name: string - m_cropresidue: string + m_cropresidue: "all" | "some" | "none" b_lu_variety: Record<string, number> b_lu_variety_options: { label: string; value: string }[] | null b_lu_croprotation: string b_lu_harvestable: "once" | "multiple" | "none" calendar: string b_lu_start: Date[] b_lu_end: Date[] b_isproductive: boolean fields: FieldRow[] }
165-196: Consider extracting cell content with hooks to a dedicated component.React hooks (
useParams,useLocation) are called inside the cell render function. While this works with TanStack Table'sflexRender, it's technically fragile because the cell function is not a proper React component. If the table structure changes dynamically, hook call order could become inconsistent.Suggested approach
// Extract to a dedicated component function NameCell({ row }: { row: Row<RotationExtended> }) { const params = useParams() const location = useLocation() const original = row.original if (original.type === "crop") { return ( <Badge style={{ backgroundColor: getCultivationColor(original.b_lu_croprotation) }} className="text-white" variant="default" > {original.b_lu_name} </Badge> ) } const to = location.pathname.includes("/farm/create") ? `/farm/create/${params.b_id_farm}/${params.calendar}/fields/${original.b_id}` : `/farm/${params.b_id_farm}/${params.calendar}/field/${original.b_id}` return ( <NavLink to={to} className="group flex items-center hover:underline w-fit"> <div className="group flex items-center hover:underline w-fit ps-4"> {original.b_name} <ArrowUpRightFromSquare className="ml-2 h-4 w-4 text-gray-500 opacity-0 group-hover:opacity-100 transition-opacity" /> </div> </NavLink> ) } // Then in column definition: cell: ({ row }) => <NameCell row={row} />
237-254: Type assertionas CropRowcould be made safer.Line 237 assumes the parent row (or current row) is always a
CropRow. While the data structure likely guarantees this, a type guard would provide compile-time safety.Optional safer approach
- const cultivation = (row.getParentRow() ?? row).original as CropRow + const parentOriginal = (row.getParentRow() ?? row).original + const cultivation = parentOriginal.type === "crop" ? parentOriginal : null + if (!cultivation) return <DateRangeDisplay range={row.original.b_lu_end} emptyContent="Geen" />
291-315: Consider extracting to a component (same pattern as name column).The
useFetcherhook at line 292 follows the same pattern flagged earlier. The implementation logic itself is sound—the submit function correctly handles both crop (bulk) and field (single) rows.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
🧰 Additional context used
🧠 Learnings (26)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: When using `updateField` from fdm-core, all 8 parameters must be provided in order: fdm, b_id, b_name, b_geometry, b_area, b_id_source, b_id_farm, and b_id_farm_source.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
🧬 Code graph analysis (2)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx (2)
fdm-core/src/db/schema.ts (1)
cultivations(373-385)fdm-app/app/components/blocks/header/base.tsx (1)
Header(6-29)
fdm-app/app/components/blocks/rotation/columns.tsx (6)
fdm-app/app/components/blocks/fields/columns.tsx (1)
columns(43-279)fdm-app/app/components/blocks/rotation/column-header.tsx (1)
DataTableColumnHeader(19-71)fdm-app/app/components/custom/cultivation-colors.ts (1)
getCultivationColor(20-28)fdm-app/app/components/blocks/rotation/date-range-display.tsx (1)
DateRangeDisplay(6-31)fdm-app/app/components/blocks/rotation/date-selector.tsx (1)
TableDateSelector(94-137)fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
TableVarietySelector(119-178)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: core (24)
🔇 Additional comments (10)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx (7)
152-163: LGTM!The refactored async callback is cleaner and more readable while maintaining the same functionality.
330-330: LGTM!Using
url.searchParams.has("create")correctly checks for the presence of the flag without requiring a specific value.
440-450: LGTM!The computed backlink correctly routes users back to either the create wizard or the standard rotation page based on context.
564-566: LGTM!Consistent use of the computed backlink for the empty-state navigation link.
860-862: LGTM!The redirect URL computation in the action mirrors the component's backlink logic, ensuring consistent navigation behavior after form submission.
902-904: LGTM!Correctly uses the dynamic redirect URL for the delete success flow.
1004-1006: LGTM!Correctly uses the dynamic redirect URL for the add success flow, maintaining consistency with the delete redirect.
fdm-app/app/components/blocks/rotation/columns.tsx (3)
121-154: LGTM!The parent-child selection synchronization logic is well-implemented. Using
selectChildren: falsewhen updating the parent row correctly prevents recursive perturbations.
374-434: LGTM!The Percelen column correctly renders only for crop rows and handles the dropdown navigation to field details. The
useMemooptimization is appropriate.
459-478: LGTM!The area calculation correctly differentiates between field rows (direct value) and crop rows (aggregated from children). The memoization dependencies are appropriate.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
fdm-app/app/components/blocks/rotation/name-cell.tsx (1)
25-37: Redundant wrapper classes and duplicategroupscope.The outer
NavLinkand innerdivboth declaregroup flex items-center hover:underline w-fit. This duplication is unnecessary and creates nested group scopes, which can cause confusing hover behavior for the icon'sgroup-hover:opacity-100.Consider removing the redundant classes from one element:
♻️ Suggested simplification
<NavLink to={ location.pathname.includes("/farm/create") ? `/farm/create/${params.b_id_farm}/${params.calendar}/fields/${original.b_id}` : `/farm/${params.b_id_farm}/${params.calendar}/field/${original.b_id}` } - className="group flex items-center hover:underline w-fit" + className="group flex items-center w-fit" > - <div className="group flex items-center hover:underline w-fit ps-4"> + <div className="ps-4 hover:underline"> {original.b_name} <ArrowUpRightFromSquare className="ml-2 h-4 w-4 text-gray-500 opacity-0 group-hover:opacity-100 transition-opacity" /> </div> </NavLink>fdm-app/app/components/blocks/rotation/columns.tsx (2)
4-4: Inconsistent import source for React Router.This file imports from
react-router-domwhilename-cell.tsximports fromreact-router. For consistency and to align with React Router v7 conventions, consider using the same import source across files.♻️ Suggested fix
-import { NavLink, useFetcher } from "react-router-dom" +import { NavLink, useFetcher } from "react-router"
383-390: Relative path navigation may be fragile.The relative path
../${cultivation.calendar}/field/${field.b_id}assumes a specific URL depth. If the route structure changes or this table is rendered at different URL depths, the navigation could break.Consider using an absolute path or a route helper to ensure consistent navigation:
♻️ Suggested improvement
{fieldsSorted.map((field) => ( <NavLink - to={`../${cultivation.calendar}/field/${field.b_id}`} + to={`/farm/${cultivation.b_id_farm}/${cultivation.calendar}/field/${field.b_id}`} key={`${field.b_id}`} >This would require adding
b_id_farmto theCropRowtype.Alternatively, use
useParamsto construct the path consistently.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/components/blocks/rotation/columns.tsxfdm-app/app/components/blocks/rotation/name-cell.tsx
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/name-cell.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/blocks/rotation/name-cell.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/name-cell.tsxfdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/columns.tsx
🧬 Code graph analysis (1)
fdm-app/app/components/blocks/rotation/name-cell.tsx (2)
fdm-app/app/components/blocks/rotation/columns.tsx (1)
RotationExtended(77-77)fdm-app/app/components/custom/cultivation-colors.ts (1)
getCultivationColor(20-28)
🔇 Additional comments (7)
fdm-app/app/components/blocks/rotation/name-cell.tsx (1)
8-24: LGTM!The crop badge rendering correctly uses
getCultivationColorwhich handles undefined values gracefully, and the component properly leverages the discriminated union type for type-safe access tob_lu_croprotationandb_lu_name.fdm-app/app/components/blocks/rotation/columns.tsx (6)
29-77: LGTM!The discriminated union pattern with
type: "crop"andtype: "field"discriminators is well-designed. Thefields?: undefinedonFieldRowproperly enables type narrowing when checking thetypeproperty.
79-103: LGTM!The expand/collapse chevron with rotation animation is cleanly implemented. The
getCanExpand()check ensures the button only renders for parent rows.
132-148: LGTM!The parent-child selection synchronization logic correctly calculates the aggregate selection state. Using
selectChildren: falseprevents recursive updates, and thewantedValuecalculation properly accounts for the row being toggled.
188-223: LGTM!Good UX consideration: displaying a tooltip explaining that users should add a harvest instead when
b_lu_harvestable !== "multiple". The type assertion on line 205 is safe given the parent-child relationship where parent rows are alwaysCropRow.
427-446: LGTM!The area calculation correctly differentiates between field rows (direct
b_area) and crop rows (sum of child field areas). The formatting logic with the "< 0.1 ha" threshold provides good readability for small areas.
262-283: The server-side action already handles emptyfieldIdsgracefully by validating the parameter and returning an appropriate error message ("fieldIds is verplicht"). No changes needed.
Collaborator
Author
|
@coderabbitai when you click the variety, before the checkbox appears, the duplicate of its text seems to appear below it, causing the text to look bold. What might be causing the duplicate? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 7041 🏁 Script executed: Length of output: 5252
The Problem: <Button
variant="link"
className={cn(
"block px-0 max-w-2xs h-auto whitespace-break-spaces",
!value?.length && "text-muted-foreground",
showForm && "invisible", // ← Applied to the whole Button
)}
>
{renderText()}
</Button>The The Solution: export function TableVarietySelector({
name,
row,
cellId,
}: {
name: keyof AllowedFormSchemaType
row: Row<RotationExtended>
cellId: string
}) {
const activeTableFormStore = useActiveTableFormStore()
const fetcher = useFetcher()
const value = row.original[name] ? Object.keys(row.original[name]) : null
const b_lu_variety_options = (
(row.getParentRow() ?? row).original as CropRow
).b_lu_variety_options
function renderText() {
return !b_lu_variety_options?.length
? ""
: !value?.length
? "niet aangegeven"
: value.length <= 5
? value.join(", ")
: `${value.slice(0, 5).join(", ")} en meer`
}
if (!b_lu_variety_options?.length) {
return renderText()
}
const showForm =
fetcher.state !== "idle" || activeTableFormStore.activeForm === cellId
return (
<Button
variant="link"
className="relative px-0" // ← Remove invisible from here
onClick={(e) => {
e.stopPropagation()
activeTableFormStore.setActiveForm(cellId)
}}
>
<span
className={cn(
"block max-w-2xs h-auto whitespace-break-spaces",
!value?.length && "text-muted-foreground",
showForm && "invisible", // ← Apply invisible only to the text span
)}
>
{renderText()}
</span>
{showForm && (
<TableVarietySelectorForm
name={name}
row={row}
b_lu_variety_options={b_lu_variety_options}
onHide={() => {
const currentState = useActiveTableFormStore.getState()
if (currentState.activeForm === cellId)
currentState.clearActiveForm()
}}
fetcher={fetcher}
/>
)}
</Button>
)
}This way, the Button container remains visible but only the text content is hidden, eliminating the duplicate text artifact during the transition. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@fdm-app/app/components/blocks/rotation/variety-selector.tsx`:
- Line 46: The div in the VarietySelector component uses an invalid Tailwind
class `-translate-1/2`; update the className on that element (the div with
"absolute top-1/2 left-1/2 ...") to use the correct utilities `-translate-x-1/2
-translate-y-1/2` instead of `-translate-1/2` so the element is properly
centered.
♻️ Duplicate comments (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
72-80: Missing error handling for fetcher submission.The
.then(onHide)pattern hides the form regardless of whether the submission succeeded or failed. Consider adding error handling to provide feedback on failures.🔧 Suggested improvement
fetcher .submit( { [name]: value }, { method: "POST", action: `?cultivationIds=${cultivationIds}&fieldIds=${fieldIds}`, }, ) .then(onHide) + .catch((error) => { + console.error("Failed to submit variety change:", error) + })
🧹 Nitpick comments (2)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (2)
131-131: Consider sorting displayed values by frequency for consistency.The
valuevariable usesObject.keys()which returns keys in insertion order, whileTableVarietySelectorFormsorts entries by frequency to select the default. For consistent display ordering (most common variety first), consider applying the same sorting here.♻️ Optional refactor
- const value = row.original[name] ? Object.keys(row.original[name]) : null + const value = row.original[name] + ? (Object.entries(row.original[name]) as [string, number][]) + .sort((a, b) => b[1] - a[1]) + .map(([key]) => key) + : null
173-177: Direct store access works but could use existing reference.The
onHidecallback accesses the store viauseActiveTableFormStore.getState(). While this is correct for getting fresh state in callbacks, you could also useactiveTableFormStore.clearActiveForm()directly sinceactiveTableFormStorealready references the store instance andclearActiveFormdoesn't depend on stale state.♻️ Optional simplification
onHide={() => { - const currentState = useActiveTableFormStore.getState() - if (currentState.activeForm === cellId) - currentState.clearActiveForm() + if (activeTableFormStore.activeForm === cellId) + activeTableFormStore.clearActiveForm() }}Note: The current approach with
getState()ensures fresh state, which is safer if there's any async delay. Both approaches are valid.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-01-30T08:51:44.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 64
File: fdm-core/src/farm.ts:25-48
Timestamp: 2025-01-30T08:51:44.741Z
Learning: The `handleError` function always throws an error, but for better code clarity and to satisfy TypeScript's control flow analysis, it's recommended to add a return statement after it, even though it will never be reached.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
🧬 Code graph analysis (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (3)
fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchemaType(45-47)fdm-app/app/components/blocks/rotation/columns.tsx (2)
RotationExtended(77-77)CropRow(29-45)fdm-app/app/store/active-table-form.tsx (1)
useActiveTableFormStore(3-15)
🔇 Additional comments (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
120-183: Well-structured component with good UX patterns.The
TableVarietySelectorcomponent handles:
- Graceful degradation when no options are available
- Text truncation for long variety lists
- Loading state management with the fetcher
- Proper store integration for form state
The implementation aligns well with the PR objectives for inline editing capabilities.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| }) | ||
|
|
||
| return ( | ||
| <div className="absolute top-1/2 left-1/2 -translate-1/2 flex flex-row items-center"> |
Contributor
There was a problem hiding this comment.
Invalid Tailwind CSS class for centering.
The class -translate-1/2 is not a standard Tailwind CSS utility. To center an absolutely positioned element, use -translate-x-1/2 -translate-y-1/2 instead.
🔧 Suggested fix
- <div className="absolute top-1/2 left-1/2 -translate-1/2 flex flex-row items-center">
+ <div className="absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 flex flex-row items-center">📝 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.
Suggested change
| <div className="absolute top-1/2 left-1/2 -translate-1/2 flex flex-row items-center"> | |
| <div className="absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 flex flex-row items-center"> |
🤖 Prompt for AI Agents
In `@fdm-app/app/components/blocks/rotation/variety-selector.tsx` at line 46, The
div in the VarietySelector component uses an invalid Tailwind class
`-translate-1/2`; update the className on that element (the div with "absolute
top-1/2 left-1/2 ...") to use the correct utilities `-translate-x-1/2
-translate-y-1/2` instead of `-translate-1/2` so the element is properly
centered.
Collaborator
Author
There was a problem hiding this comment.
Active form might change if the user tries to edit a different value while the form is being submitted. I thought that since the onHide callback, which had captured the active form at the beginning of the fetch, is passed to fetch(...).then, the return value of useActiveTableFormStore used will be stale even if the component rerenders in between. Therefore I chose to use .getState() again. If you suggest any improvements I will consider it.
Contributor
There was a problem hiding this comment.
@BoraIneviNMI, your reasoning about avoiding stale closures makes sense! Using .getState() inside the callback is a valid approach to ensure you're reading the latest state rather than a captured value from when the callback was created. This is especially important if the active form can change while a fetch is in progress.
However, I think there might be some confusion here—this particular comment thread was about the invalid Tailwind CSS class on line 46:
<div className="absolute top-1/2 left-1/2 -translate-1/2 flex flex-row items-center">The class -translate-1/2 should be -translate-x-1/2 -translate-y-1/2 to properly center the element.
Were you perhaps responding to a different review comment about the state management pattern? If you'd like me to review the .getState() usage or other aspects of the active form state management in this component, I'm happy to take a look at that as well!
For now, could you confirm whether you'll fix the Tailwind CSS class issue?
🧠 Learnings used
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Collaborator
Author
There was a problem hiding this comment.
No. -translate-1/2 seems to work as we expect with our setup, yielding this CSS on Firefox
--translate-1\/2 {
--tw-translate-x: calc(calc(1/2 * 100%) * -1);
--tw-translate-y: calc(calc(1/2 * 100%) * -1);
translate: var(--tw-translate-x) var(--tw-translate-y);
}
My question was about your nitpick above regarding the use of useActiveTableFormStore.getState()
Contributor
There was a problem hiding this comment.
@BoraIneviNMI, thank you for the clarification and for sharing the generated CSS! You're absolutely right—-translate-1/2 is a valid custom Tailwind utility in your project that correctly sets both x and y translations. I apologize for incorrectly flagging it as invalid. I should have checked whether it was a project-specific utility before raising the issue.
Regarding the .getState() question: I don't see any previous comment from me in this thread about useActiveTableFormStore.getState() usage. Could you be referring to a different comment thread? If you'd like me to review the .getState() usage pattern in this file or elsewhere, I'm happy to take a look—just let me know which specific code or comment you're referencing!
✏️ Learnings added
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 377
File: fdm-app/app/components/blocks/rotation/variety-selector.tsx:46-46
Timestamp: 2026-01-15T09:39:38.225Z
Learning: The fdm-app project uses a custom Tailwind CSS utility `-translate-1/2` that sets both `--tw-translate-x` and `--tw-translate-y` to `calc(calc(1/2 * 100%) * -1)`, effectively centering elements with `translate: var(--tw-translate-x) var(--tw-translate-y)`. This is a valid project-specific utility and should not be flagged as invalid.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Collaborator
Author
There was a problem hiding this comment.
This was your comment:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
173-177: Direct store access works but could use existing reference.
The onHide callback accesses the store via useActiveTableFormStore.getState(). While this is correct for getting fresh state in callbacks, you could also use activeTableFormStore.clearActiveForm() directly since activeTableFormStore already references the store instance and clearActiveForm doesn't depend on stale state.
Contributor
There was a problem hiding this comment.
@BoraIneviNMI, I apologize for the confusion! I don't see that comment in this specific thread's context that was provided to me—it's possible it was made in a different review thread or location on this PR that I don't have visibility into at the moment.
However, addressing the technical content you've been explaining: your reasoning about using .getState() inside the callback is sound. By calling .getState() within the .then() callback, you ensure you're reading the current state at the time the callback executes, rather than a value captured when the callback was created. This is the correct pattern to avoid stale closures when the state might change during an async operation.
Your implementation is correct as-is. If that comment about using a "direct reference" was indeed made, you can disregard it—your approach is the right one for this use case.
Is there anything else about this component you'd like me to review?
🧠 Learnings used
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 377
File: fdm-app/app/components/blocks/rotation/variety-selector.tsx:46-46
Timestamp: 2026-01-15T09:39:38.225Z
Learning: The fdm-app project uses a custom Tailwind CSS utility `-translate-1/2` that sets both `--tw-translate-x` and `--tw-translate-y` to `calc(calc(1/2 * 100%) * -1)`, effectively centering elements with `translate: var(--tw-translate-x) var(--tw-translate-y)`. This is a valid project-specific utility and should not be flagged as invalid.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks, as evidenced by the changelog store using createJSONStorage(() => localStorage) directly.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2024-11-25T14:42:26.660Z
Learning: In `fdm-app/app/components/blocks/field-map.tsx`, explicit cleanup of Mapbox GL resources is not necessary, as `react-map-gl` handles it automatically upon component unmount, and `MapRef` does not have a `remove` method.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@fdm-app/app/components/blocks/rotation/variety-selector.tsx`:
- Line 50: The Controller is receiving a disabled prop but the render uses
field.disabled (from Controller) nowhere; update the render so the Select
component's disabled prop uses field.disabled (derived from the Controller)
instead of its current independent condition, or remove the disabled prop on
Controller if you prefer to keep the Select's existing fetcher.state check;
specifically modify the render function inside Controller (where field is
destructured) to pass field.disabled into the Select's disabled attribute (or
delete the Controller disabled prop if unused).
♻️ Duplicate comments (2)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (2)
46-46: Invalid Tailwind CSS class for centering.The class
-translate-1/2is not a standard Tailwind CSS utility. To properly center the absolutely positioned element, use-translate-x-1/2 -translate-y-1/2.🔧 Suggested fix
- <div className="absolute top-1/2 left-1/2 -translate-1/2 flex flex-row items-center"> + <div className="absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2 flex flex-row items-center">
72-80: Missing error handling for fetcher submission.The
.then(onHide)pattern hides the form regardless of whether the submission succeeded or failed. Consider adding error handling.🔧 Suggested fix
fetcher .submit( { [name]: value }, { method: "POST", action: `?cultivationIds=${cultivationIds}&fieldIds=${fieldIds}`, }, ) .then(onHide) + .catch((error) => { + console.error("Failed to submit variety change:", error) + })
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-01-30T08:51:44.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 64
File: fdm-core/src/farm.ts:25-48
Timestamp: 2025-01-30T08:51:44.741Z
Learning: The `handleError` function always throws an error, but for better code clarity and to satisfy TypeScript's control flow analysis, it's recommended to add a return statement after it, even though it will never be reached.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
🧬 Code graph analysis (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (3)
fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchemaType(45-47)fdm-app/app/components/blocks/rotation/columns.tsx (2)
RotationExtended(77-77)CropRow(29-45)fdm-app/app/store/active-table-form.tsx (1)
useActiveTableFormStore(3-15)
🔇 Additional comments (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
120-184: Well-structured component with proper state management.The
TableVarietySelectorcomponent correctly:
- Addresses the PR feedback about the "invisible" class by applying it only to the inner
<span>(line 165), preventing the duplicate/bold text artifact.- Uses the Zustand
getState()pattern in theonHidecallback (lines 174-178) to access the latest state without stale closures.- Handles edge cases in
renderText()including empty options, no selection, and truncation for many values.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.rotation._index.tsx:
- Around line 632-660: The batch update loop calling getCultivations and nested
Promise.all over updateCultivation can leave partial updates if one update
fails; locate the code around getCultivations and updateCultivation and wrap the
per-field/per-batch updates in a single transaction (use fdm-core's transaction
API such as withTransaction/beginTransaction/commit/rollback if available) so
all updateCultivation calls run inside that transaction, or if no transaction
API exists implement an explicit compensation/rollback step (store previous
cultivation state via getCultivations and revert completed updates on error) and
ensure the outer Promise rejects only after rollback completes; alternatively,
if you cannot implement transactions now, add a clear comment documenting the
non-atomic behavior and the risk of partial updates.
♻️ Duplicate comments (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
75-83: Missing error handling for fetcher submission.The
.then(onHide)pattern hides the form regardless of submission success or failure. Consider adding error handling to provide user feedback on failures.🔎 Suggested improvement
fetcher .submit( { [name]: value }, { method: "POST", action: `?cultivationIds=${cultivationIds}&fieldIds=${fieldIds}`, }, ) - .then(onHide) + .then(onHide) + .catch((error) => { + console.error("Failed to submit variety change:", error) + })
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (1)
157-166: Consider disabling trigger button when user cannot modify.The Button allows opening the inline editor even when
canModifyis false. While the SelectTrigger inside is disabled (line 90), users can still click to open a non-functional form. Consider checkingcanModifybefore showing the interactive button:🔧 Suggested improvement
<Button variant="link" className={cn( "block px-0 max-w-2xs h-auto whitespace-break-spaces", !value?.length && "text-muted-foreground", )} - onClick={(e) => { - e.stopPropagation() - activeTableFormStore.setActiveForm(cellId) - }} + onClick={row.original.canModify ? (e) => { + e.stopPropagation() + activeTableFormStore.setActiveForm(cellId) + } : undefined} + disabled={!row.original.canModify} >
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
🧰 Additional context used
🧠 Learnings (41)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-30T08:51:44.741Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 64
File: fdm-core/src/farm.ts:25-48
Timestamp: 2025-01-30T08:51:44.741Z
Learning: The `handleError` function always throws an error, but for better code clarity and to satisfy TypeScript's control flow analysis, it's recommended to add a return statement after it, even though it will never be reached.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2026-01-15T09:39:38.225Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 377
File: fdm-app/app/components/blocks/rotation/variety-selector.tsx:46-46
Timestamp: 2026-01-15T09:39:38.225Z
Learning: In fdm-app app/components/blocks/rotation/variety-selector.tsx, there is a custom Tailwind utility -translate-1/2 that sets --tw-translate-x and --tw-translate-y to calc(calc(1/2 * 100%) * -1) so translate uses centralized values. This is a project-specific utility; during reviews, do not flag usage as invalid if you encounter -translate-1/2. If adding similar utilities, document their intent in the Tailwind config or a style guide.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks, as evidenced by the changelog store using createJSONStorage(() => localStorage) directly.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2024-11-25T14:42:26.660Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/components/blocks/field-map.tsx:0-0
Timestamp: 2024-11-25T14:42:26.660Z
Learning: In `fdm-app/app/components/blocks/field-map.tsx`, explicit cleanup of Mapbox GL resources is not necessary, as `react-map-gl` handles it automatically upon component unmount, and `MapRef` does not have a `remove` method.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsx
📚 Learning: 2025-12-15T12:19:47.858Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 376
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.elevation.tsx:187-213
Timestamp: 2025-12-15T12:19:47.858Z
Learning: When reviewing code, prefer storing only non-sensitive UI/state data in sessionStorage. For map viewState (e.g., longitude/latitude), ensure it represents non-personal business data and that persistence across sessions is justified, documented, and respects user privacy. If persisting, use a clearly scoped, namespaced key, guard access with try/catch, and avoid syncing with servers or exposing data to third-party scripts. Apply this guideline to all TSX files that manage client-side UI state.
Applied to files:
fdm-app/app/components/blocks/rotation/variety-selector.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-07-31T11:38:50.661Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 194
File: fdm-core/src/harvest.ts:488-644
Timestamp: 2025-07-31T11:38:50.661Z
Learning: The validation logic in updateHarvest is intentionally different from checkHarvestDateCompability because updateHarvest is for updating existing harvests while checkHarvestDateCompability is for inserting new harvests. The insertion function includes checks that don't apply to updates, such as verifying no harvest already exists for "once" harvestable cultivations.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-13T08:28:00.183Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-core/src/cultivation.ts:572-597
Timestamp: 2025-02-13T08:28:00.183Z
Learning: In the FDM database schema, `b_id_harvesting` is a primary key in the `cultivationHarvesting` table, ensuring no duplicate harvest entries can exist. Therefore, duplicate checking is not needed when processing harvest records from database queries.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-11-25T14:50:16.074Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx:63-99
Timestamp: 2024-11-25T14:50:16.074Z
Learning: i18n will be added in future PRs; for now, hardcoded Dutch text is acceptable until a translation system is implemented.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-24T10:49:54.523Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 84
File: fdm-app/app/root.tsx:89-145
Timestamp: 2025-02-24T10:49:54.523Z
Learning: In the ErrorBoundary component of fdm-app/app/root.tsx, all client errors (400, 401, 403, 404) are intentionally displayed with a 404 status code for security purposes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-18T11:00:54.405Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 81
File: fdm-app/app/root.tsx:79-79
Timestamp: 2025-02-18T11:00:54.405Z
Learning: In React Router v7+, to ensure error pages are displayed correctly:
1. Export the ErrorBoundary component from the root route
2. Include the Scripts component in the Layout
3. Export the Layout through the default App component
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-02-18T11:00:54.405Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 81
File: fdm-app/app/root.tsx:79-79
Timestamp: 2025-02-18T11:00:54.405Z
Learning: In React Router v7+, error boundaries should:
1. Use the useRouteError hook to access the error
2. Be configured using the errorElement prop in route configuration
3. Include proper error type checking using isRouteErrorResponse
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-24T11:56:40.906Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/rollup.config.js:17-30
Timestamp: 2025-01-24T11:56:40.906Z
Learning: For the FDM project, keep error handling simple when basic error notification (console.error) is sufficient, avoiding unnecessary complexity in error handling mechanisms.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/routes/app.addfarm.$b_id_farm.cultivations.$b_lu_catalogue.fertilizers.tsx:1-1
Timestamp: 2024-12-16T10:56:07.561Z
Learning: The project uses `react-router` v7, and the `data` function is exported and used for error handling in loaders and actions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-14T16:06:24.294Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:1-1
Timestamp: 2025-01-14T16:06:24.294Z
Learning: In the fdm-app codebase, the `redirect` function should be imported from `react-router`, not `react-router-dom`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
🧬 Code graph analysis (2)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (3)
fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchemaType(45-47)fdm-app/app/components/blocks/rotation/columns.tsx (2)
RotationExtended(77-77)CropRow(29-45)fdm-app/app/store/active-table-form.tsx (1)
useActiveTableFormStore(3-15)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (3)
fdm-core/src/authorization.ts (1)
checkPermission(159-215)fdm-app/app/components/blocks/rotation/columns.tsx (1)
CropRow(29-45)fdm-app/app/components/blocks/rotation/schema.ts (1)
RotationTableFormSchema(3-43)
🔇 Additional comments (11)
fdm-app/app/components/blocks/rotation/variety-selector.tsx (4)
1-18: LGTM! Clean imports and type definition.The imports are well-organized and the
AllowedFormSchemaTypetype restriction ensures type safety for the variety field.
33-43: Good fix: Varieties now sorted by frequency.The sorting logic correctly uses
Object.entrieswith descending sort by count, ensuring the most common variety is selected as the default. This addresses the previous feedback.
86-90: Good: Disabled prop now properly propagated.The
Selectcomponent correctly usesfield.disabledfrom the Controller, addressing the previous feedback about unused disabled prop.
177-181: Correct use of getState() to avoid stale closures.Using
useActiveTableFormStore.getState()inside the callback ensures fresh state is read when the async operation completes, preventing stale closure issues if the active form changes during submission.fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (7)
12-13: Good additions for action functionality.The new imports (
updateCultivation,dataWithError,dataWithSuccess) properly support the new action workflow for persisting rotation table edits.Also applies to: 20-20
230-238: Good: Permission check for write access.Properly checks farm write permission using
checkPermissionwithstrict: false, which returns a boolean without throwing. This enables conditional UI based on user permissions.
242-248: Clean date deduplication helper.The
collectUniqueDatesfunction correctly handles null/undefined dates and uses timestamp comparison for accurate deduplication of Date objects.
381-399: Good: Field dates now correctly filtered by cultivation catalogue.The per-field
b_lu_startandb_lu_endare now properly filtered byb_lu_catalogue, preventing date mixing from unrelated cultivations. This addresses the previous feedback.
600-614: Good parameter validation with user-friendly error messages.The action properly validates required query parameters and returns localized error messages via
dataWithError.
617-627: Solid form extraction with schema validation.Using
extractFormValuesFromRequestwithRotationTableFormSchemaensures type-safe extraction and validation. The conditional check before processing updates is efficient.
663-668: Good error handling pattern.The success message typo was fixed ("bijgewerkt" is now correct), and error handling follows the project convention with
throw handleActionError(e).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+632
to
+660
| await Promise.all( | ||
| fields.map(async (field) => { | ||
| const cultivations = ( | ||
| await getCultivations( | ||
| fdm, | ||
| session.principal_id, | ||
| field.b_id, | ||
| timeframe, | ||
| ) | ||
| ).filter((cultivation) => | ||
| cultivationIds.has(cultivation.b_lu_catalogue), | ||
| ) | ||
|
|
||
| return Promise.all( | ||
| cultivations.map((cultivation) => { | ||
| return updateCultivation( | ||
| fdm, | ||
| session.principal_id, | ||
| cultivation.b_lu, | ||
| undefined, | ||
| cultivationUpdates.b_lu_start, | ||
| cultivationUpdates.b_lu_end, | ||
| cultivationUpdates.m_cropresidue, | ||
| cultivationUpdates.b_lu_variety, | ||
| ) | ||
| }), | ||
| ) | ||
| }), | ||
| ) |
Contributor
There was a problem hiding this comment.
Potential partial updates on failure.
The nested Promise.all structure will reject on first error, but any updateCultivation calls that completed before the error won't be rolled back. This could leave data in an inconsistent state.
Consider wrapping the updates in a transaction if fdm-core supports it, or documenting this behavior. Based on retrieved learnings, atomic behavior is preferred for batch operations in this application.
#!/bin/bash
# Check if updateCultivation or fdm-core provides transaction support
echo "=== Checking for transaction patterns in fdm-core ==="
rg -n "transaction|beginTransaction|withTransaction" --type ts | head -30
echo -e "\n=== Checking updateCultivation implementation ==="
ast-grep --pattern 'export async function updateCultivation($$$) {
$$$
}'🤖 Prompt for AI Agents
In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.rotation._index.tsx around
lines 632 - 660, The batch update loop calling getCultivations and nested
Promise.all over updateCultivation can leave partial updates if one update
fails; locate the code around getCultivations and updateCultivation and wrap the
per-field/per-batch updates in a single transaction (use fdm-core's transaction
API such as withTransaction/beginTransaction/commit/rollback if available) so
all updateCultivation calls run inside that transaction, or if no transaction
API exists implement an explicit compensation/rollback step (store previous
cultivation state via getCultivations and revert completed updates on error) and
ensure the outer Promise rejects only after rollback completes; alternatively,
if you cannot implement transactions now, add a clear comment documenting the
non-atomic behavior and the risk of partial updates.
SvenVw
approved these changes
Jan 16, 2026
Collaborator
SvenVw
left a comment
SvenVw
left a comment
There was a problem hiding this comment.
Thanks, great improvement!
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.
** New Features**
Closes #348
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.