Add start and end date validation for the cultivation harvest bulk action#414
Conversation
🦋 Changeset detectedLatest commit: 200d7fe 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 |
WalkthroughAdds loader-computed boundary dates (latest sowing and earliest cultivation end) to the rotation harvest route, surfaces them to HarvestForm, and updates client-side validation messages to include Dutch-formatted boundary dates for harvest date checks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User
participant Browser as Browser (Rotation page)
participant Loader as Server Loader
participant Action as Server Action
participant DB as Database
User->>Browser: Open Rotation → Add Harvest
Browser->>Loader: Request loader data
Loader-->>Browser: Return payload (cultivations, b_lu_start, b_lu_end, ...)
Browser-->>User: Render HarvestForm with b_lu_start / b_lu_end
User->>Browser: Fill form (harvest_date, values) and submit
Browser->>Action: Submit form (includes harvest_date, b_lu_start, b_lu_end)
Action->>Action: Validate harvest_date against b_lu_start / b_lu_end
alt valid
Action->>DB: Persist harvest
DB-->>Action: OK
Action-->>Browser: Success / redirect
else invalid
Action-->>Browser: Return validation errors (messages include formatted dates)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
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 #414 +/- ##
============================================
Coverage 88.07% 88.07%
============================================
Files 91 91
Lines 4620 4620
Branches 1492 1492
============================================
Hits 4069 4069
Misses 551 551
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
BoraIneviNMI
changed the title
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
297-319: Logic issue:mixedDatesflag doesn't reflect actual date variance.The
mixedDatesflag is set totruewhen there's more than one field selected (b_lu_starts.length > 1), but this doesn't indicate whether the dates actually differ. Two fields could have the same sowing date, yetmixedDateswould betrue.Additionally, the arrays may contain
undefinedentries when a cultivation doesn't have a start/end date set, but these are still counted toward the length.Consider checking if dates actually differ:
🔧 Suggested improvement
- const mixedDates = b_lu_starts.length > 1 || b_lu_ends.length > 1 + // Check if there are actually different dates among selected fields + const uniqueStarts = new Set(b_lu_starts.filter(Boolean).map(d => d?.getTime())) + const uniqueEnds = new Set(b_lu_ends.filter(Boolean).map(d => d?.getTime())) + const mixedDates = uniqueStarts.size > 1 || uniqueEnds.size > 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/long-pianos-admire.mdfdm-app/app/components/blocks/harvest/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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.
📚 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:
.changeset/long-pianos-admire.md
📚 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:
.changeset/long-pianos-admire.mdfdm-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/components/blocks/harvest/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._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/components/blocks/harvest/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._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/components/blocks/harvest/form.tsxfdm-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/components/blocks/harvest/form.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/harvest/form.tsxfdm-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/components/blocks/harvest/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._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.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.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.tsx
📚 Learning: 2025-08-13T10:33:05.313Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.
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
🔇 Additional comments (9)
fdm-app/app/components/blocks/harvest/form.tsx (5)
37-38: LGTM! Good choice using date-fns with Dutch locale.The imports for date formatting with the Dutch locale are appropriate for this application's target audience.
55-55: LGTM!The optional
mixedDatesprop is correctly typed and positioned within the interface.
157-187: LGTM! Well-structured conditional rendering for date messages.The logic correctly handles all date combinations with appropriate Dutch messaging. The short-circuit evaluation safely handles null/undefined dates.
663-663: LGTM!The alignment change from
items-centertoitems-startcorrectly accommodates the new date message block below the date picker.
190-218: Validation is already implemented in the schema.The harvest date validation against sowing and end dates is already in place in
schema.tsx(lines 211–235). The schema validates that:
- Harvest date must occur after the sowing date (b_lu_start)
- For "multiple" harvestable cultivations, harvest date must not exceed the end date (b_lu_end)
The UI correctly displays these date boundaries to users for guidance, and the form enforces these constraints before submission.
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx (3)
309-318: LGTM! Correct aggregation logic for date boundaries.The reducer correctly finds the latest sowing date (max) and earliest ending date (min), which establishes the valid date range for the harvest. The fallback handling for undefined dates is appropriate.
360-362: LGTM!The new date boundary fields and
mixedDatesflag are correctly exposed from the loader and passed to theHarvestFormcomponent.Also applies to: 767-769
1021-1027: Server-side date validation is properly implemented and user-friendly errors are handled.The
addHarvestfunction infdm-corecallscheckHarvestDateCompability, which validates that the harvest date:
- Is not missing
- Is on or after the sowing date
- For "once" harvestable cultivations, equals the terminating date
- For "multiple" harvestable cultivations, is before the terminating date
Each validation failure throws a descriptive error message (e.g., "Harvest date must be after or on sowing date"), which is caught and properly handled by the action's error handling flow. This addresses issue #400's concern about unhandled errors.
.changeset/long-pianos-admire.md (1)
1-5: LGTM!The changeset correctly describes the validation feature and appropriately marks this as a patch release.
|
Did you consider using Zod for validation? |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation.harvest._index.tsx (1)
713-765: Duplicatekeyprop on HarvestForm.The
keyprop is assigned twice (Lines 714 and 764), which causes React to use the second value and may trigger unexpected behavior. Remove one of the duplicate assignments.🐛 Proposed fix
<HarvestForm key={selectedFieldIds.join(",")} harvestParameters={ loaderData.harvestParameters } b_lu_harvest_date={ loaderData.harvestApplication .b_lu_harvest_date } b_lu_yield={ loaderData.harvestableAnalysis .b_lu_yield } b_lu_yield_fresh={ loaderData.harvestableAnalysis .b_lu_yield_fresh } b_lu_yield_bruto={ loaderData.harvestableAnalysis .b_lu_yield_bruto } b_lu_tarra={ loaderData.harvestableAnalysis .b_lu_tarra } b_lu_uww={ loaderData.harvestableAnalysis .b_lu_uww } b_lu_moist={ loaderData.harvestableAnalysis .b_lu_moist } b_lu_dm={ loaderData.harvestableAnalysis .b_lu_dm } b_lu_cp={ loaderData.harvestableAnalysis .b_lu_cp } b_lu_n_harvestable={ loaderData.harvestableAnalysis .b_lu_n_harvestable } b_lu_harvestable={ loaderData.cultivation .b_lu_harvestable } b_lu_start={loaderData.b_lu_start} b_lu_end={loaderData.b_lu_end} - key={selectedFieldIds.join(",")} action={modifySearchParams( `${location.pathname}${location.search}`, (searchParams) => searchParams.set( "fieldIds", selectedFieldIds.join( ",", ), ), )} handleConfirmation={ handleConfirmation } />
2 participants
Enhancements
Closes #400
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.