Skip to content

Implement calendar-aware date handling for forms and date pickers#495

Merged
SvenVw merged 19 commits into
mainfrom
hotfix/FDM494-2
Mar 9, 2026
Merged

Implement calendar-aware date handling for forms and date pickers#495
SvenVw merged 19 commits into
mainfrom
hotfix/FDM494-2

Conversation

@SvenVw
Copy link
Copy Markdown
Collaborator

@SvenVw SvenVw commented Mar 9, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Calendar-year-aware default dates for fertilizer applications, soil analysis, and harvest records.
    • Support for Dutch numeric date formats (DD-MM) in date pickers.
    • Default harvest dates populated from crop catalog data.
  • Improvements

    • Form data now persisted separately per calendar year.
    • Navigation progress indicator delay increased; can be disabled per route.
    • Enhanced date parsing robustness across multiple formats.
  • Bug Fixes

    • Fixed calendar initialization guard.
    • Refined farm nitrogen balance calculation.

Closes #494

@SvenVw SvenVw self-assigned this Mar 9, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 9, 2026

👋 Hotfix Branch PR Detected!

Before merging this Pull Request into main, please ensure you have finalized the hotfix by manually running the 'Release' workflow on this hotfix/FDM494-2 branch.

This will:

  1. Bump package versions.
  2. Generate changelogs.
  3. Create Git tags.

You can trigger the workflow from the 'Actions' tab, selecting the 'Release' workflow, and choosing this hotfix/FDM494-2 branch.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 9, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The changes implement calendar-year-aware date parsing and contextual default date initialization across forms and date picker components. Date inputs are now parsed using the active cultivation year from the calendar store for disambiguation, and form defaults leverage domain-specific dates aligned with the calendar year rather than the current system date. Navigation progress metrics are enhanced with Sentry instrumentation and route-level opt-out support, and nitrogen balance calculation is refined to use ammonia-specific emissions.

Changes

Cohort / File(s) Summary
Date Picker Enhancements
fdm-app/app/components/custom/date-picker.tsx, fdm-app/app/components/custom/date-picker-v2.tsx
Added calendar-year-aware date parsing with support for Dutch numeric formats (DD-MM, DD-MM-YY, DD-MM-YYYY) and chrono-node parsing respecting the active calendar year. Initial month and reference date now align with calendar context instead of today's date.
Calendar Utility
fdm-app/app/lib/calendar.ts
Added new exported function getContextualDate() that computes context-aware dates using a provided calendar year; returns today if calendar year matches current year, otherwise returns a fixed date on the specified month/day within that year.
Form Default Date Handling
fdm-app/app/components/blocks/fertilizer-applications/form.tsx, fdm-app/app/components/blocks/harvest/form.tsx, fdm-app/app/components/blocks/soil/form.tsx
Updated forms to initialize date fields using getContextualDate() based on active calendar; fertilizer applications default to March 1st, harvests to catalogue-provided b_date_harvest_default, and soil analysis to February 1st within the calendar year. All now thread calendar through form store interactions.
Harvest Form Props
fdm-app/app/components/blocks/harvest/form.tsx
Added b_date_harvest_default prop to component and hook; refactored HarvestFields prop type from ReturnType<typeof useHarvestRemixForm> to generic UseRemixFormReturn<z.infer<typeof FormSchema>>; introduced getDefaultHarvestDate() helper to compute calendar-year-aware harvest defaults.
Route Loaders & Data Threading
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.new.tsx, fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation_.harvest._index.tsx
Extended loaders to compute and return b_date_harvest_default from cultivation catalogue; propagated value through route components into harvest form as new prop.
Fertilizer Navigation Enhancement
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.fertilizer._index.tsx
Added computed returnUrl validation and dynamic header label in loader and component; header now uses loader-derived URL and conditionally labels navigation as "rotation" or "fields" based on return path.
Form Store Calendar Threading
fdm-app/app/store/field-fertilizer-form.tsx
Added optional calendar parameter to all store methods (save, load, delete) and makeId helper; calendar is incorporated into generated storage keys for multi-calendar data isolation.
Navigation Progress & Metrics
fdm-app/app/components/custom/navigation-progress.tsx
Increased show delay from 300ms to 500ms; added route-level opt-out via hideNavigationProgress handle detection; integrated Sentry metrics including page-tagged counters and duration distributions with pathname tracking.
Bulk Operation Route Handlers
fdm-app/app/routes/farm.$b_id_farm.soil-analysis.bulk.tsx, fdm-app/app/routes/farm.create.$b_id_farm.$calendar.soil-analysis.bulk.tsx, fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
Added handle export with { hideNavigationProgress: true } to suppress navigation progress indicator during bulk operations and file uploads.
Calendar Store Guard
fdm-app/app/routes/farm.tsx
Added undefined guard in calendar initialization effect to prevent passing undefined to store setter.
Nitrogen Balance Calculation
fdm-calculator/src/balance/nitrogen/index.ts
Replaced avgFarmEmission with avgFarmEmissionAmmonia in farm-level nitrogen balance computation; updated accompanying comment to reflect ammonia-focused emission aggregation.
Maintenance
fdm-app/CHANGELOG.md, fdm-app/package.json, fdm-calculator/CHANGELOG.md, fdm-calculator/package.json
Version bumps: fdm-app 0.28.0 → 0.28.1, fdm-calculator 0.12.0 → 0.12.1; added changelog entries documenting date picker and nitrogen balance improvements.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant DatePicker as DatePicker Component
    participant Calendar as useCalendarStore
    participant Parser as parseDateString / parseDutchNumericDate
    participant Form as Form Component
    
    User->>DatePicker: Input date text (e.g., "15 april")
    DatePicker->>Calendar: Read active calendar year
    Calendar-->>DatePicker: Return calendar year (e.g., 2023)
    DatePicker->>Parser: parseDateString(input, calendarYear)
    
    alt Dutch Numeric Format
        Parser->>Parser: parseDutchNumericDate("15-04", 2023)
        Parser-->>DatePicker: Date(2023-04-15)
    else Chrono Parsing
        Parser->>Parser: Chrono parse with referenceDate in calendar year
        Parser-->>DatePicker: Date(2023-04-15)
    else ISO Format
        Parser->>Parser: Parse ISO string YYYY-MM-DD
        Parser-->>DatePicker: Date parsed
    end
    
    DatePicker->>Form: Set field value
    Form-->>User: Display parsed date with calendar context
Loading
sequenceDiagram
    participant Form as Form Component
    participant Calendar as useCalendarStore
    participant Lib as getContextualDate()
    participant Store as FieldFertilizerFormStore
    
    Form->>Calendar: Read active calendar year
    Calendar-->>Form: Return calendar (e.g., "2023")
    
    alt Creating New Record
        Form->>Lib: getContextualDate(calendar, month, day)
        Lib->>Lib: Check if calendar year == current year
        alt Calendar Year is Current Year
            Lib-->>Form: Today's date
        else Calendar Year is Different
            Lib-->>Form: month/day in calendar year
        end
        Form->>Form: Use contextual date as defaultValue
    else Loading Existing Record
        Form->>Store: load(b_id_farm, b_id, calendar)
        Store-->>Form: Saved form data
        Form->>Form: Use loaded values
    end
    
    Form-->>User: Display form with contextual default date
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #456 — Modifies harvest form component and rotation/harvest route loaders; shares changes to harvest date defaulting and form prop threading.
  • PR #488 — Introduces navigation progress indicator and Sentry integration; this PR extends that component with timing adjustments, route-level opt-out, and page-tagged metrics.
  • PR #354 — Updates date-picker parsing logic; this PR builds on that foundation by adding calendar-year context and Dutch numeric format support.

Suggested labels

enhancement, fdm-app, fdm-calculator, branch:main

Suggested reviewers

  • BoraIneviNMI
  • gerardhros

Poem

🐰 Hops through time with calendar cheer,
Default dates now context-aware, clear,
Parse "15 april" to the right year,
Dutch formats hop, no more pain here,
Forms aligned with cultivation's sphere! 🌾

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes out-of-scope changes: modifications to field-fertilizer-form store signatures, harvest form signatures, navigation-progress metrics, and nitrogen balance calculations are not part of the core date-handling objectives. Review and potentially isolate changes to field-fertilizer-form store, HarvestForm/HarvestFormDialog signatures, navigation-progress metrics, and nitrogen balance calculations into separate PRs aligned with their specific objectives.
Docstring Coverage ⚠️ Warning Docstring coverage is 27.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Implement calendar-aware date handling for forms and date pickers' accurately and concisely summarizes the main objective of implementing context-aware date handling across forms and date picker components.
Linked Issues check ✅ Passed The PR implements all major requirements from issue #494: adds getContextualDate utility, makes date pickers calendar-aware via useCalendarStore, and applies contextual defaults to fertilizer, harvest, and soil analysis forms.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix/FDM494-2

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@SvenVw
Copy link
Copy Markdown
Collaborator Author

SvenVw commented Mar 9, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot changed the title @coderabbitai Implement calendar-aware date handling for forms and date pickers Mar 9, 2026
@nmi-agro nmi-agro deleted a comment from coderabbitai Bot Mar 9, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 9, 2026

✅ Created PR with unit tests: #496

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (2)
fdm-app/app/components/blocks/fertilizer-applications/form.tsx (1)

83-100: ⚠️ Potential issue | 🟠 Major

Make the saved fertilizer draft calendar-aware.

The new default now depends on calendar, but the persisted draft still keys only on (b_id_farm, b_id_or_b_lu_catalogue). A draft saved in one cultivation year will therefore be reloaded unchanged in another for the same farm/catalogue, overriding getContextualDate(calendar, 3, 1) with the wrong p_app_date. Include calendar in the save/load/delete key, or drop a restored date whose year no longer matches the active calendar.

🧭 Call-site changes needed here
-            const savedFormValues = fieldFertilizerFormStore.load(
-                b_id_farm,
-                b_id_or_b_lu_catalogue,
-            )
+            const savedFormValues = fieldFertilizerFormStore.load(
+                b_id_farm,
+                b_id_or_b_lu_catalogue,
+                calendar,
+            )
...
-            fieldFertilizerFormStore.delete(b_id_farm, b_id_or_b_lu_catalogue)
+            fieldFertilizerFormStore.delete(
+                b_id_farm,
+                b_id_or_b_lu_catalogue,
+                calendar,
+            )
...
-            fieldFertilizerFormStore.save(
-                b_id_farm,
-                b_id_or_b_lu_catalogue,
-                form.getValues(),
-            )
+            fieldFertilizerFormStore.save(
+                b_id_farm,
+                b_id_or_b_lu_catalogue,
+                calendar,
+                form.getValues(),
+            )

This needs the corresponding store API update as well.

Also applies to: 121-143, 160-177

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/app/components/blocks/fertilizer-applications/form.tsx` around lines
83 - 100, The draft save/load/delete logic for fertilizer applications is not
calendar-aware: when building defaultValues (notably p_app_date computed via
getContextualDate(calendar, 3, 1)) you must either include the active calendar
value from useCalendarStore() in the draft storage key OR, on restore, discard
any restored p_app_date whose year doesn't match the active calendar; update the
code paths that persist and read drafts (the store API used by this component)
to accept the calendar identifier and use it in the key (or implement the
year-check/drop behavior) so restored drafts don't override the contextual date;
ensure changes touch the save, load and delete call sites mentioned in this file
(and the store API) so keys remain consistent across cultivation years.
fdm-app/app/components/custom/navigation-progress.tsx (1)

29-55: ⚠️ Potential issue | 🟡 Minor

Add hideProgress to the effect's dependency array.

The effect reads hideProgress to conditionally start the timer (line 30), but it's not included in the dependency array (line 55). If hideProgress changes during navigation (e.g., user navigates between routes with different handle configurations), the effect won't re-run, potentially showing the indicator when it should be hidden.

🛠️ Proposed fix
-    }, [state, show])
+    }, [state, show, hideProgress])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/app/components/custom/navigation-progress.tsx` around lines 29 - 55,
The useEffect reading hideProgress (used in the conditional that starts the
timer) must include hideProgress in its dependency array so the effect re-runs
when it changes; update the dependency list for the useEffect that references
state, show, startTimeRef, setShow and clientConfig/Sentry.metrics to also
include hideProgress so the timer is started or cancelled correctly when
hideProgress toggles during navigation.
🧹 Nitpick comments (3)
fdm-app/app/lib/calendar.ts (1)

52-64: Harden getContextualDate against invalid year strings.

Number(calendar) can become NaN, and new Date(NaN, ...) then feeds an Invalid Date into the form defaults. A small guard here would make the helper safe if it gets reused outside the store-backed path.

🛡️ Suggested guard
 export function getContextualDate(
     calendar: string | undefined,
     defaultMonth: number,
     defaultDay: number,
 ): Date {
     const currentYear = new Date().getFullYear()
-    const calendarYear = calendar ? Number(calendar) : currentYear
+    const parsedYear = calendar ? Number(calendar) : currentYear
+    const calendarYear = Number.isInteger(parsedYear)
+        ? parsedYear
+        : currentYear
 
     if (calendarYear === currentYear) {
         return new Date()
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/app/lib/calendar.ts` around lines 52 - 64, The getContextualDate
function can produce NaN when parsing calendar with Number(calendar); change the
guard in getContextualDate to validate parsed year (e.g., const parsed =
Number(calendar)) and if parsed is NaN or not an integer within a sane range,
fall back to currentYear; keep the existing behavior of returning new Date()
when year equals currentYear and otherwise returning new Date(parsed,
defaultMonth - 1, defaultDay) so callers never receive Invalid Date.
fdm-app/app/components/custom/date-picker-v2.tsx (1)

188-260: Consider extracting shared date parsing logic to a utility module.

Both date-picker.tsx and date-picker-v2.tsx now have nearly identical implementations of parseDutchNumericDate and similar calendar-aware parsing logic. Extracting these to a shared utility (e.g., ~/lib/date-utils.ts) would reduce duplication and ensure consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/app/components/custom/date-picker-v2.tsx` around lines 188 - 260,
Duplicate date parsing logic exists in parseDateText and parseDutchNumericDate
across date-picker.tsx and date-picker-v2.tsx; extract the shared functions into
a new utility module (e.g., ~/lib/date-utils.ts) that exports
parseDutchNumericDate and a calendar-aware parseDateText (or smaller helpers
used by parseDateText), update both components to import those functions, remove
the duplicated implementations from both components, and ensure exports/imports
preserve signatures (parseDateText(date: string|Date|undefined, calendarYear?:
number) and parseDutchNumericDate(text: string, targetYear: number)) so behavior
remains identical.
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation_.harvest._index.tsx (1)

329-333: Consider simplifying redundant catalogue lookup.

targetCultivation is already found from cultivationCatalogueData at line 176-178. Searching the same array again for the same b_lu_catalogue will return the same object.

♻️ Suggested simplification
-        const b_date_harvest_default =
-            cultivationCatalogueData.find(
-                (item) =>
-                    item.b_lu_catalogue === targetCultivation.b_lu_catalogue,
-            )?.b_date_harvest_default ?? null
+        const b_date_harvest_default =
+            targetCultivation.b_date_harvest_default ?? null
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.rotation_.harvest._index.tsx
around lines 329 - 333, The code redundantly re-searches
cultivationCatalogueData to derive b_date_harvest_default; instead use the
already-found targetCultivation. Replace the find-based lookup with reading
targetCultivation?.b_date_harvest_default ?? null so b_date_harvest_default
comes directly from targetCultivation (references: b_date_harvest_default,
cultivationCatalogueData, targetCultivation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.changeset/fifty-frogs-float.md:
- Line 5: Update the release note in .changeset/fifty-frogs-float.md to correct
the harvest example: remove the fixed “August 1st” wording and state that the
harvest flow uses the cultivation's configured b_date_harvest_default for
non-current years while harvest dates remain empty for the current calendar
year; also keep the DatePicker behavior note about resolving partial text to the
active calendar year unchanged.

In @.changeset/seven-days-nail.md:
- Line 5: Update the changelog entry string that contains the typo "calender" to
the correct spelling "calendar" in the .changeset/seven-days-nail.md file;
specifically replace the phrase "Fix that going to Fertilizers page does not
reset the selected calender year to current year" with the corrected phrase "Fix
that going to Fertilizers page does not reset the selected calendar year to
current year" so the change description reads correctly.

In `@fdm-app/app/components/blocks/harvest/form.tsx`:
- Around line 84-86: The code converts calendar from useCalendarStore() with
Number(calendar) which can produce NaN for non-numeric values like "all"; change
the logic that sets calendarYear so it parses calendar into a number (e.g.,
const parsed = Number(calendar)) and uses Number.isNaN(parsed) to fall back to
currentYear when parsed is NaN, ensuring calendarYear is always a valid number
before later comparisons and before constructing dates (the new Date(...) call)
so you never pass NaN into Date.

---

Outside diff comments:
In `@fdm-app/app/components/blocks/fertilizer-applications/form.tsx`:
- Around line 83-100: The draft save/load/delete logic for fertilizer
applications is not calendar-aware: when building defaultValues (notably
p_app_date computed via getContextualDate(calendar, 3, 1)) you must either
include the active calendar value from useCalendarStore() in the draft storage
key OR, on restore, discard any restored p_app_date whose year doesn't match the
active calendar; update the code paths that persist and read drafts (the store
API used by this component) to accept the calendar identifier and use it in the
key (or implement the year-check/drop behavior) so restored drafts don't
override the contextual date; ensure changes touch the save, load and delete
call sites mentioned in this file (and the store API) so keys remain consistent
across cultivation years.

In `@fdm-app/app/components/custom/navigation-progress.tsx`:
- Around line 29-55: The useEffect reading hideProgress (used in the conditional
that starts the timer) must include hideProgress in its dependency array so the
effect re-runs when it changes; update the dependency list for the useEffect
that references state, show, startTimeRef, setShow and
clientConfig/Sentry.metrics to also include hideProgress so the timer is started
or cancelled correctly when hideProgress toggles during navigation.

---

Nitpick comments:
In `@fdm-app/app/components/custom/date-picker-v2.tsx`:
- Around line 188-260: Duplicate date parsing logic exists in parseDateText and
parseDutchNumericDate across date-picker.tsx and date-picker-v2.tsx; extract the
shared functions into a new utility module (e.g., ~/lib/date-utils.ts) that
exports parseDutchNumericDate and a calendar-aware parseDateText (or smaller
helpers used by parseDateText), update both components to import those
functions, remove the duplicated implementations from both components, and
ensure exports/imports preserve signatures (parseDateText(date:
string|Date|undefined, calendarYear?: number) and parseDutchNumericDate(text:
string, targetYear: number)) so behavior remains identical.

In `@fdm-app/app/lib/calendar.ts`:
- Around line 52-64: The getContextualDate function can produce NaN when parsing
calendar with Number(calendar); change the guard in getContextualDate to
validate parsed year (e.g., const parsed = Number(calendar)) and if parsed is
NaN or not an integer within a sane range, fall back to currentYear; keep the
existing behavior of returning new Date() when year equals currentYear and
otherwise returning new Date(parsed, defaultMonth - 1, defaultDay) so callers
never receive Invalid Date.

In `@fdm-app/app/routes/farm`.$b_id_farm.$calendar.rotation_.harvest._index.tsx:
- Around line 329-333: The code redundantly re-searches cultivationCatalogueData
to derive b_date_harvest_default; instead use the already-found
targetCultivation. Replace the find-based lookup with reading
targetCultivation?.b_date_harvest_default ?? null so b_date_harvest_default
comes directly from targetCultivation (references: b_date_harvest_default,
cultivationCatalogueData, targetCultivation).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ce7d834f-9350-47c9-95c4-bf45f6a01ed5

📥 Commits

Reviewing files that changed from the base of the PR and between 4c28e7d and 06fafd1.

📒 Files selected for processing (19)
  • .changeset/brave-eagles-hide.md
  • .changeset/dirty-rooms-doubt.md
  • .changeset/fifty-frogs-float.md
  • .changeset/nine-cups-start.md
  • .changeset/seven-days-nail.md
  • fdm-app/app/components/blocks/fertilizer-applications/form.tsx
  • fdm-app/app/components/blocks/harvest/form.tsx
  • fdm-app/app/components/blocks/soil/form.tsx
  • fdm-app/app/components/custom/date-picker-v2.tsx
  • fdm-app/app/components/custom/date-picker.tsx
  • fdm-app/app/components/custom/navigation-progress.tsx
  • fdm-app/app/lib/calendar.ts
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.new.tsx
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.field.fertilizer._index.tsx
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation_.harvest._index.tsx
  • fdm-app/app/routes/farm.$b_id_farm.soil-analysis.bulk.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.soil-analysis.bulk.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
  • fdm-app/app/routes/farm.tsx

Comment thread .changeset/fifty-frogs-float.md Outdated
Comment thread .changeset/seven-days-nail.md Outdated
Comment thread fdm-app/app/components/blocks/harvest/form.tsx Outdated
@SvenVw
Copy link
Copy Markdown
Collaborator Author

SvenVw commented Mar 9, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 9, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

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/components/blocks/fertilizer-applications/form.tsx (1)

83-100: ⚠️ Potential issue | 🟠 Major

The fertilizer default date still races the route calendar.

useCalendarStore() starts with the real current year, and fdm-app/app/routes/farm.tsx:103-112 only syncs the URL calendar in an effect. That means Line 100 can still seed p_app_date with today's date on the first render of a non-current-year route, and the load effect at Lines 121-145 does nothing when there is no saved draft for that key. Because useRemixForm does not re-apply defaultValues, this form can still open with the wrong year. Please reset p_app_date when calendar changes, or derive the route calendar before calling useRemixForm.

Also applies to: 121-145

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/app/components/blocks/fertilizer-applications/form.tsx` around lines
83 - 100, The form can seed p_app_date with the wrong year because
useCalendarStore() starts with the real current year and useRemixForm's
defaultValues aren't re-applied; fix by deriving the route/calendar value before
calling useRemixForm or by adding an effect that updates the form value when
calendar changes: compute the correct calendar (the same logic used in the
route) prior to invoking useRemixForm so getContextualDate(calendar, 3, 1) uses
the route calendar, or after form creation add a useEffect watching calendar
that calls the form API (e.g., setValue or reset) to set
FieldFertilizerFormValues.p_app_date via getContextualDate when calendar changes
(also apply same fix to the load/draft effect area around the existing load
logic).
🧹 Nitpick comments (1)
fdm-app/app/components/custom/navigation-progress.tsx (1)

56-66: Inconsistent indentation in Sentry scope block.

The Sentry.withScope block on lines 56-65 has extra indentation compared to the similar block on lines 40-46. This appears to be a formatting inconsistency.

🧹 Suggested fix for consistent indentation
         if (clientConfig.analytics.sentry) {
-                    Sentry.withScope((scope) => {
-                        scope.setTag(
-                            "page",
-                            startPathnameRef.current ?? pathname,
-                        )
-                        Sentry.metrics.distribution(
-                            "navigation_progress.duration_ms",
-                            duration,
-                        )
-                    })
-                }
+            Sentry.withScope((scope) => {
+                scope.setTag(
+                    "page",
+                    startPathnameRef.current ?? pathname,
+                )
+                Sentry.metrics.distribution(
+                    "navigation_progress.duration_ms",
+                    duration,
+                )
+            })
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/app/components/custom/navigation-progress.tsx` around lines 56 - 66,
The Sentry.withScope block is indented inconsistently compared to the earlier
similar block; update the indentation of the Sentry.withScope callback so its
lines (Sentry.withScope, scope.setTag(...), and
Sentry.metrics.distribution(...)) align with the other Sentry.withScope usage in
the file, keeping the same nesting level and spacing and preserving references
to startPathnameRef.current, pathname, and duration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@fdm-app/app/components/blocks/harvest/form.tsx`:
- Around line 84-89: The form's defaultValues are built before
useCalendarStore() is populated, so getDefaultHarvestDate() can return undefined
and useRemixForm will keep that initial value; update the logic to reset or
rebuild the form when calendar changes (or accept the route calendar before
computing defaults). Specifically, after calling useRemixForm (or wherever
defaultValues are passed), call the form's reset method (or re-invoke
useRemixForm with new defaultValues) inside an effect that depends on calendar
from useCalendarStore(), and ensure getDefaultHarvestDate(...) is re-evaluated
using the updated calendar value (reference useCalendarStore,
getDefaultHarvestDate, useRemixForm, defaultValues, and reset) so the catalogue
default date for the active cultivation year is applied when the route-provided
calendar arrives.

---

Outside diff comments:
In `@fdm-app/app/components/blocks/fertilizer-applications/form.tsx`:
- Around line 83-100: The form can seed p_app_date with the wrong year because
useCalendarStore() starts with the real current year and useRemixForm's
defaultValues aren't re-applied; fix by deriving the route/calendar value before
calling useRemixForm or by adding an effect that updates the form value when
calendar changes: compute the correct calendar (the same logic used in the
route) prior to invoking useRemixForm so getContextualDate(calendar, 3, 1) uses
the route calendar, or after form creation add a useEffect watching calendar
that calls the form API (e.g., setValue or reset) to set
FieldFertilizerFormValues.p_app_date via getContextualDate when calendar changes
(also apply same fix to the load/draft effect area around the existing load
logic).

---

Nitpick comments:
In `@fdm-app/app/components/custom/navigation-progress.tsx`:
- Around line 56-66: The Sentry.withScope block is indented inconsistently
compared to the earlier similar block; update the indentation of the
Sentry.withScope callback so its lines (Sentry.withScope, scope.setTag(...), and
Sentry.metrics.distribution(...)) align with the other Sentry.withScope usage in
the file, keeping the same nesting level and spacing and preserving references
to startPathnameRef.current, pathname, and duration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 13a2d89d-6ef4-46a7-8b75-01e72f85b08a

📥 Commits

Reviewing files that changed from the base of the PR and between 06fafd1 and c6dfd07.

📒 Files selected for processing (8)
  • .changeset/fifty-frogs-float.md
  • .changeset/seven-days-nail.md
  • .changeset/social-cycles-stare.md
  • fdm-app/app/components/blocks/fertilizer-applications/form.tsx
  • fdm-app/app/components/blocks/harvest/form.tsx
  • fdm-app/app/components/custom/navigation-progress.tsx
  • fdm-app/app/lib/calendar.ts
  • fdm-app/app/store/field-fertilizer-form.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
  • .changeset/seven-days-nail.md
  • fdm-app/app/lib/calendar.ts
  • .changeset/fifty-frogs-float.md

Comment thread fdm-app/app/components/blocks/harvest/form.tsx
@SvenVw SvenVw requested a review from gerardhros March 9, 2026 15:09
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@fdm-app/app/components/blocks/fertilizer-applications/form.tsx`:
- Around line 119-131: The effect that backfills p_app_date (inside the
useEffect block) currently calls form.setValue unconditionally when calendar
loads and can overwrite a user-picked date; change the guard to only call
setValue when the form field is still empty and not dirty: inside the same
useEffect that references getContextualDate(calendar, 3, 1), read the current
field state/value via form.getFieldState("p_app_date")?.isDirty or
form.getValues("p_app_date") and only invoke form.setValue("p_app_date", ...) if
there is no exampleFertilizerApplication and the field value is falsy and
isDirty is false (or undefined).
- Around line 137-140: The calendar-scoped draft keys are half-migrated:
form.tsx now calls fieldFertilizerFormStore.load(b_id_farm,
b_id_or_b_lu_catalogue, calendar) but fertilizer-applications/card.tsx still
uses the legacy key shape (calls to fieldFertilizerFormStore.load and
fieldFertilizerFormStore.delete without calendar), causing drafts to be
orphaned; fix by either updating the calls in card.tsx to pass the calendar
argument wherever load/delete are invoked (the load calls around the previous
Lines 77-84 and delete calls around 98-101 and 139-142 and the other occurrences
at 178-198), or modify the store implementation (fieldFertilizerFormStore.load
and .delete) to try the new calendar-aware key first and fall back to the legacy
key (and migrate/clear the legacy entry when found) so both callers remain
compatible during rollout.

In `@fdm-app/app/components/blocks/harvest/form.tsx`:
- Around line 187-200: The effect is resetting the entire form via form.reset
when the calendar loads, which can overwrite a user-entered b_lu_harvest_date
and clear touched/dirty state; change the logic in the useEffect so that when
calendar updates you only set the harvest date if it’s currently empty (check
b_lu_harvest_date and example_b_lu_harvest_date) by calling
form.setValue('b_lu_harvest_date', getDefaultHarvestDate()) or, if you must call
form.reset, pass the option to preserve user edits (e.g., keepDirtyValues: true)
and avoid touching other fields; update the useEffect body around
form.reset/getDefaultHarvestDate accordingly to preserve dirty/touched state and
not overwrite an existing user value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4bb00a25-2f8f-4382-804f-53f26958ee9b

📥 Commits

Reviewing files that changed from the base of the PR and between c6dfd07 and b2d17cd.

📒 Files selected for processing (2)
  • fdm-app/app/components/blocks/fertilizer-applications/form.tsx
  • fdm-app/app/components/blocks/harvest/form.tsx

Comment thread fdm-app/app/components/blocks/fertilizer-applications/form.tsx
Comment thread fdm-app/app/components/blocks/fertilizer-applications/form.tsx
Comment thread fdm-app/app/components/blocks/harvest/form.tsx
gerardhros
gerardhros previously approved these changes Mar 9, 2026
Copy link
Copy Markdown
Collaborator

@gerardhros gerardhros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@fdm-calculator/CHANGELOG.md`:
- Line 7: The CHANGELOG entry for PR `#495` is incorrect: it mentions "Fixes for
farm nitrogen balance to exclude nitrate leaching" while the PR implements
calendar-aware date handling for DatePicker, DatePicker-v2 and form components
using the calendar store; update the CHANGELOG entry to accurately summarize the
date/calendar changes (or, if this PR actually includes the nitrogen-balance
changes too, document both features), referencing the affected components
DatePicker, DatePicker-v2 and the calendar store so reviewers can verify the
described changes match the code in PR `#495`.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4a73142f-517e-4363-86c8-f4facbb7ceda

📥 Commits

Reviewing files that changed from the base of the PR and between 65ebf42 and 4a3bd50.

📒 Files selected for processing (4)
  • fdm-app/CHANGELOG.md
  • fdm-app/package.json
  • fdm-calculator/CHANGELOG.md
  • fdm-calculator/package.json
✅ Files skipped from review due to trivial changes (3)
  • fdm-app/CHANGELOG.md
  • fdm-app/package.json
  • fdm-calculator/package.json

Comment thread fdm-calculator/CHANGELOG.md Outdated
@SvenVw SvenVw merged commit 86338f0 into main Mar 9, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants