Skip to content

Add nutrient advice app#164

Merged
SvenVw merged 42 commits into
developmentfrom
add-nutrient-advice-app
Jun 18, 2025
Merged

Add nutrient advice app#164
SvenVw merged 42 commits into
developmentfrom
add-nutrient-advice-app

Conversation

@SvenVw
Copy link
Copy Markdown
Collaborator

@SvenVw SvenVw commented Jun 11, 2025

Summary by CodeRabbit

  • New Features

    • Introduced a comprehensive "Nutrient Advice" application accessible from the sidebar, providing tailored nutrient recommendations for farm fields.
    • Added detailed nutrient advice pages with categorized nutrient cards, KPI summaries for applications, deficits, and excesses, and interactive navigation for farms and fields.
    • Implemented new components for nutrient advice display, application breakdowns, progress indicators, and breadcrumb navigation.
    • Integrated external nutrient advice data based on soil and land use information.
  • Enhancements

    • Expanded nutrient dose calculations to cover a broader range of nutrients with per-application breakdowns.
    • Improved naming consistency and clarity for nutrient dose properties.
    • Refined loader data returns for fertilizer doses to simplify data consumption.
  • Bug Fixes

    • Corrected typos in fertilizer property names to ensure accurate data handling.
  • Tests

    • Updated and expanded test coverage for nutrient dose calculations and fertilizer logic reflecting new features and naming conventions.
  • Chores

    • Added a new UI dependency for progress indicators.

@SvenVw SvenVw self-assigned this Jun 11, 2025
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 11, 2025

🦋 Changeset detected

Latest commit: 8b7ea60

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@svenvw/fdm-app Minor
@svenvw/fdm-core Patch
@svenvw/fdm-calculator Minor

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 11, 2025

Walkthrough

This update introduces a comprehensive "nutrient advice" feature, including new React components, route modules, and utility functions for nutrient management in a farm application. It expands nutrient dose calculations, updates domain models, adds integration with an external nutrient advice API, and revises related tests. Sidebar navigation and UI elements are also enhanced to support the new functionality.

Changes

Files / Areas Change Summary
.changeset/*.md Added changeset files documenting new features, bug fixes, and minor/patch updates for nutrient advice, dose calculation, and typo corrections.
fdm-app/app/components/blocks/nutrient-advice/cards.tsx, kpi.tsx, nutrients.ts, types.d.ts Introduced new React components, types, and utilities for displaying nutrient advice, KPIs, and nutrient metadata.
fdm-app/app/components/blocks/header/nutrient-advice.tsx Added a header component for nutrient advice navigation with breadcrumb and field selection functionality.
fdm-app/app/components/blocks/sidebar/apps.tsx Added a sidebar menu item for nutrient advice with conditional navigation logic and icon.
fdm-app/app/integrations/nmi.ts Added function to fetch nutrient advice from an external NMI API, including soil data processing and error handling.
fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice*.tsx Added new route modules, loaders, and React components for nutrient advice pages at farm and field levels, including data fetching, error handling, and UI rendering.
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.tsx, farm.create... Updated loader functions to return only the numeric dose value instead of the full dose object.
fdm-app/app/components/blocks/fertilizer-applications/cards.tsx Updated fertilizer card value sources to use new dose property names (p_dose_p, p_dose_k).
fdm-app/package.json Added @radix-ui/react-progress dependency for progress bar UI elements.
fdm-calculator/src/doses/calculate-dose.ts, d.ts Refactored dose calculation logic to support more nutrients, return detailed application doses, update property names, and add input validation.
fdm-calculator/src/doses/calculate-dose.test.ts, get-dose-field.test.ts Rewrote and expanded tests to cover new dose calculation logic, additional nutrients, error cases, and output structure.
fdm-core/src/fertilizer.ts, fertilizer.test.ts Fixed typo in fertilizer parameter key (p_cr_rt), updated affected tests.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Sidebar
    participant Router
    participant Loader
    participant NMI_API
    participant UI

    User->>Sidebar: Clicks "Bemestingsadvies"
    Sidebar->>Router: Navigates to /farm/{farmId}/{calendar}/nutrient_advice
    Router->>Loader: Calls loader with farm and field params
    Loader->>NMI_API: Fetches nutrient advice (soil data, land use, centroid)
    NMI_API-->>Loader: Returns nutrient advice data
    Loader-->>UI: Supplies nutrient advice, doses, applications, fields, etc.
    UI->>User: Renders nutrient advice cards, KPIs, navigation
Loading

Possibly related PRs

  • SvenVw/fdm#75: Introduced the initial calculateDose and getDoseForField functions, which are now extended and integrated for the new nutrient advice features.
  • SvenVw/fdm#111: Contains the typo fix for the p_cr_rt parameter in addFertilizerToCatalogue, directly related to the typo correction in this PR.
  • SvenVw/fdm#16: Adds fertilizer application form and CRUD functionality; related to fertilizer application handling but distinct from nutrient advice presentation.

Suggested reviewers

  • gerardhros

Poem

In the garden of code where nutrients bloom,
A rabbit hops by, dispelling the gloom.
Doses expanded, advice now precise,
With KPIs, cards, and fields that entice.
Sidebar and header, all shiny and new—
This patch brings fresh carrots for you! 🥕


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 999eb31 and 8b7ea60.

📒 Files selected for processing (1)
  • fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • fdm-app/app/components/blocks/nutrient-advice/nutrients.ts
⏰ Context from checks skipped due to timeout of 300000ms (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: core (22)
  • GitHub Check: calculator (22)
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate Unit Tests
  • Create PR with Unit Tests
  • Commit Unit Tests in branch add-nutrient-advice-app
  • Post Copyable Unit Tests in Comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.11%. Comparing base (96aca83) to head (8b7ea60).
Report is 43 commits behind head on development.

Additional details and impacted files
@@               Coverage Diff               @@
##           development     #164      +/-   ##
===============================================
+ Coverage        93.07%   93.11%   +0.03%     
===============================================
  Files               68       68              
  Lines             8061     8101      +40     
  Branches           957      967      +10     
===============================================
+ Hits              7503     7543      +40     
  Misses             556      556              
  Partials             2        2              
Flag Coverage Δ
fdm-calculator 94.84% <100.00%> (+0.15%) ⬆️
fdm-core 92.75% <ø> (ø)
fdm-data 92.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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: 7

🔭 Outside diff range comments (2)
fdm-app/app/components/blocks/fertilizer-applications/cards.tsx (1)

116-130: 💡 Verification agent

🧩 Analysis chain

Dose property switch may desynchronise unit / label — verify and rename accordingly

dose.p_dose_p and dose.p_dose_k now supply the numeric values, yet the card still shows the short-name P2O5 / K2O and the unit kg/ha.
If the underlying calculator now returns elemental P and K (rather than P₂O₅ / K₂O), we will be displaying the wrong chemical form to the user.

Action:

- shortname: "P2O5",
- unit: "kg/ha",
+ shortname: "P",
+ unit: "kg P/ha",

and similar for K → K / kg K/ha.

(If the calculator still returns P₂O₅ & K₂O but merely renamed the fields, ignore the diff but please leave a clarifying comment so future readers don’t have to reverse-engineer this.)


🏁 Script executed:

#!/bin/bash
# Search for where p_dose_p and p_dose_k are defined or referenced
rg "p_dose_p" -n -A3 -B3
rg "p_dose_k" -n -A3 -B3

Length of output: 16419


Clarify that p_dose_p / p_dose_k denote oxide masses (P₂O₅ / K₂O)

The calculator’s types and tests confirm that p_dose_p and p_dose_k still return phosphate (P₂O₅) and potassium‐oxide (K₂O) masses in kg/ha. The current shortnames are therefore correct, and the generic "kg/ha" unit paired with “P2O5” / “K2O” remains unambiguous. To avoid future confusion, please add inline comments in the cards explaining exactly what each value represents.

• File: fdm-app/app/components/blocks/fertilizer-applications/cards.tsx
• Around lines 116–121:
```diff
{
title: "Fosfaat, totaal",
shortname: "P2O5",
value: dose.p_dose_p,
unit: "kg/ha",

  •    // dose.p_dose_p is the mass of P₂O₅ (phosphate) in kg/ha
       limit: undefined,
       advice: undefined,
    
    },
    • Around lines 123–129:
    ```diff
     {
         title: "Kalium, totaal",
         shortname: "K2O",
         value: dose.p_dose_k,
         unit: "kg/ha",
    
  •    // dose.p_dose_k is the mass of K₂O (potassium oxide) in kg/ha
       limit: undefined,
       advice: undefined,
    
    },
    
    
fdm-calculator/src/doses/get-dose-field.test.ts (1)

20-28: 🛠️ Refactor suggestion

Close the FDM server or clean DB between tests

beforeEach spins-up a fresh server but there is no matching afterEach/afterAll to dispose connections or wipe the test schema. Orphaned connections easily exhaust the pool when the suite scales.
Add teardown logic (e.g. await fdm.close() or DB truncate) to keep tests hermetic.

🧹 Nitpick comments (14)
fdm-app/app/integrations/nmi.ts (2)

3-3: Remove unused import to avoid dead code warnings

number is imported from zod but never used.
Lean imports keep the bundle smaller and ensure linters stay quiet.

-import { number, z } from "zod"
+import { z } from "zod"

206-208: Returning only result.data.year may drop valuable information

getNutrientAdvice collapses the full API response to a single year value.
If callers later need other properties (e.g. nutrient targets, metadata), you’ll have to re-touch this integration.

Consider returning result.data (or a typed subset) and letting callers pick what they need.

fdm-app/package.json (1)

29-29: Check tree-shaking impact of @radix-ui/react-progress

Radix components ship both JS and optional CSS.
Confirm that unused styles are purged in production to avoid bundle bloat.
No action required if Tailwind’s purge or an equivalent setup is already in place.

fdm-app/app/components/blocks/fertilizer-applications/cards.tsx (1)

28-30: Gracefully handle unknown dose values

Math.round(value) will throw NaN when value is undefined (e.g. a yet-unsupported nutrient or a failed calculation), which renders the whole component as "NaN kg/ha".

A small guard prevents silent UI breakage:

- <div className="text-2xl font-bold">{`${Math.round(value)} ${unit}`}</div>
+ <div className="text-2xl font-bold">
+   {value != null ? `${Math.round(value)} ${unit}` : "-"}
+ </div>
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.tsx (1)

112-123: Returning dose.dose drops useful details – check downstream consumers

calculateDose() returns an object that (per the new API) contains:

{ dose: Dose; perApplication: Dose[]; /* … */ }

By stripping it down to dose.dose you lose per-application breakdown and any extra metadata.
Before committing to this change, verify that no current or near-future screen (e.g. the new nutrient-advice pages) needs those details.

If you still want only the aggregate values, at least rename the local variable to avoid the stutter:

- const dose = calculateDose({ … })
-
-     dose: dose.dose,
+ const { dose: totalDose } = calculateDose({ … })
+
+     dose: totalDose,
fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1)

1-8: Minor style nit – semicolons keep TypeScript declarations tidy

Type literals favour semicolons for consistency with interfaces:

-export type NutrientDescription = {
-    name:  string,
-    symbol: string
-    type: "primary" | "secondary" | "trace",
-    unit: string
-    adviceParameter: string
-    doseParameter: string
-}
+export interface NutrientDescription {
+    name: string;
+    symbol: string;
+    type: "primary" | "secondary" | "trace";
+    unit: string;
+    adviceParameter: string;
+    doseParameter: string;
+}

Purely cosmetic – ignore if you prefer the current style.

fdm-app/app/components/blocks/sidebar/apps.tsx (1)

48-56: Repeated link-construction logic – extract helper to DRY up sidebar

The atlasLink, nutrientBalanceLink, and the newly added nutrientAdviceLink all follow identical conditional logic. A small utility will cut the boilerplate and reduce future merge conflicts:

function buildFarmLink(path: string, isCreate: boolean, farmId?: string, cal?: string) {
    return !isCreate && farmId ? `/farm/${farmId}/${cal}/${path}` : undefined
}

const atlasLink = buildFarmLink("atlas", isCreateFarmWizard, farmId, selectedCalendar)
const nutrientBalanceLink = buildFarmLink("balance", isCreateFarmWizard, farmId, selectedCalendar)
const nutrientAdviceLink = buildFarmLink("nutrient_advice", isCreateFarmWizard, farmId, selectedCalendar)
fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (2)

1-4: Export a constant instead of recreating the same array on every call

getNutrientsDescription() returns a static list; turning it into a const avoids needless allocations and enables tree-shaking:

-export function getNutrientsDescription(): NutrientDescription[] {
-    const nutrientDescription: NutrientDescription[] = [
+export const nutrientDescriptions: readonly NutrientDescription[] = [-    ]
-    return nutrientDescription
-}
+] as const

If you prefer keeping the function, at least wrap the array in useMemo at the call site.


102-108: Inconsistent language – keep nutrient names either NL or EN

Most entries use Dutch labels (Mangaan, Fosfaat) but Molybdenum is English.
For a polished UI pick one language and stick to it (Mol​ybdeen for Dutch).

fdm-calculator/src/doses/get-dose-field.test.ts (1)

128-169: Prefer toMatchObject over deep equality to decouple from future dose fields

toEqual demands exact structural equality. As the Dose interface keeps expanding (this PR already added many nutrients), new harmless fields will break this test.
Use partial matching:

-expect(await getDoseForField({ fdm, principal_id, b_id })).toEqual(
-    expectedResult,
-)
+await expect(
+  getDoseForField({ fdm, principal_id, b_id }),
+).resolves.toMatchObject(expectedResult)

This keeps the assertion robust while still verifying the values you care about.

fdm-app/app/components/blocks/header/nutrient-advice.tsx (1)

53-66: Dropdown link placement may swallow pointer events

DropdownMenuCheckboxItem renders a focusable role. Nesting a <NavLink> inside violates a11y expectations and can interfere with Radix menu event handling. Prefer the Radix-recommended asChild pattern:

-<DropdownMenuCheckboxItem …>
-  <NavLink to={…}>{option.b_name}</NavLink>
-</DropdownMenuCheckboxItem>
+<DropdownMenuCheckboxItem … asChild>
+  <NavLink to={…}>{option.b_name}</NavLink>
+</DropdownMenuCheckboxItem>
fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (2)

127-135: Component name misaligned with route purpose

export default function FarmBalanceNitrogenBlock() suggests a nitrogen balance view, yet the route serves nutrient advice. Rename for clarity and future discoverability, e.g. FarmNutrientAdviceLayout.


83-89: Float rounding silently loses precision

Rounding b_area to one decimal inside loader leaks presentation concerns into data-layer logic. Return the raw value and let the UI format it; this prevents accidental data loss when other consumers need higher precision.

fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx (1)

162-162: Remove commented console.log

-    // console.log(primaryNutrients)
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 96aca83 and 9c0ccc1.

⛔ Files ignored due to path filters (2)
  • fdm-app/app/components/ui/progress.tsx is excluded by !fdm-app/app/components/ui/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (23)
  • .changeset/calm-candies-mix.md (1 hunks)
  • .changeset/eager-mangos-shake.md (1 hunks)
  • .changeset/solid-planes-sell.md (1 hunks)
  • fdm-app/app/components/blocks/fertilizer-applications/cards.tsx (1 hunks)
  • fdm-app/app/components/blocks/header/nutrient-advice.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/cards.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1 hunks)
  • fdm-app/app/components/blocks/sidebar/apps.tsx (3 hunks)
  • fdm-app/app/integrations/nmi.ts (2 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.tsx (1 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx (1 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice._index.tsx (1 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (1 hunks)
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.cultivations.$b_lu_catalogue.fertilizers.tsx (1 hunks)
  • fdm-app/package.json (1 hunks)
  • fdm-calculator/src/doses/calculate-dose.test.ts (1 hunks)
  • fdm-calculator/src/doses/calculate-dose.ts (1 hunks)
  • fdm-calculator/src/doses/d.ts (1 hunks)
  • fdm-calculator/src/doses/get-dose-field.test.ts (2 hunks)
  • fdm-core/src/fertilizer.test.ts (7 hunks)
  • fdm-core/src/fertilizer.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
.changeset/solid-planes-sell.md (1)
Learnt from: SvenVw
PR: SvenVw/fdm#75
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.fertilizer.tsx:68-71
Timestamp: 2025-02-14T09:56:37.606Z
Learning: The `calculateDose` function in `@svenvw/fdm-calculator` is a synchronous function that includes built-in validation for negative application amounts and nutrient rates.
fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (1)
Learnt from: SvenVw
PR: SvenVw/fdm#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.
🧬 Code Graph Analysis (7)
fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1)
fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1)
  • NutrientDescription (1-8)
fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (3)
fdm-calculator/src/doses/d.ts (1)
  • Dose (21-39)
fdm-app/app/components/custom/loadingspinner.tsx (1)
  • LoadingSpinner (13-35)
fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1)
  • NutrientDescription (1-8)
fdm-calculator/src/doses/calculate-dose.ts (2)
fdm-calculator/src/doses/d.ts (1)
  • Dose (21-39)
fdm-core/src/db/schema.ts (1)
  • fertilizers (112-120)
fdm-app/app/components/blocks/nutrient-advice/cards.tsx (3)
fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1)
  • NutrientDescription (1-8)
fdm-calculator/src/doses/d.ts (1)
  • Dose (21-39)
fdm-core/src/db/schema.ts (2)
  • fertilizerApplication (154-171)
  • fertilizers (112-120)
fdm-core/src/fertilizer.ts (1)
fdm-core/src/db/schema.ts (1)
  • fertilizersCatalogueTypeInsert (239-240)
fdm-calculator/src/doses/calculate-dose.test.ts (1)
fdm-calculator/src/doses/calculate-dose.ts (1)
  • calculateDose (37-135)
fdm-app/app/components/blocks/header/nutrient-advice.tsx (1)
fdm-app/app/store/calendar.tsx (1)
  • useCalendarStore (11-15)
🪛 Biome (1.9.4)
fdm-app/app/integrations/nmi.ts

[error] 148-148: Unexpected any. Specify a different type.

any disables many type checking rules. Its use should be avoided.

(lint/suspicious/noExplicitAny)

⏰ Context from checks skipped due to timeout of 300000ms (1)
  • GitHub Check: calculator (22)
🔇 Additional comments (21)
.changeset/eager-mangos-shake.md (1)

5-5: Patch description update approved.

The changeset accurately describes the typo fix for p_cr_rt, aligning with the implementation changes. Ready to merge.

fdm-core/src/fertilizer.ts (1)

127-127: Typo correction in fertilizer properties parameter.

Changed property key from pr_cr_rt to p_cr_rt matches the schema and ensures consistency across the API.

fdm-core/src/fertilizer.test.ts (7)

123-123: Corrected test property key to p_cr_rt.

Typo fix aligns test data with the updated implementation.


198-198: Corrected test property key to p_cr_rt.

Typo fix aligns test data with the updated implementation.


273-273: Corrected test property key to p_cr_rt.

Typo fix aligns test data with the updated implementation.


361-361: Corrected test property key to p_cr_rt.

Typo fix aligns test data with the updated implementation.


457-457: Corrected test property key to p_cr_rt.

Typo fix aligns test data with the updated implementation.


671-671: Corrected test property key to p_cr_rt.

Typo fix aligns test data with the updated implementation.


785-785: Corrected test property key to p_cr_rt.

Typo fix aligns test data with the updated implementation.

fdm-app/app/routes/farm.create.$b_id_farm.$calendar.cultivations.$b_lu_catalogue.fertilizers.tsx (1)

135-146: Verify downstream expectations after narrowing loader output

The loader now exposes dose: dose.dose.
Ensure that every consumer (here FertilizerApplicationsCards) indeed expects the flattened Dose object and no longer relies on the per-application details that were previously available on dose.applications.

If any component still expects the richer structure this change will silently break UI rendering.

.changeset/calm-candies-mix.md (1)

1-6: Changeset looks good

Semantic version bump and description are clear.

.changeset/solid-planes-sell.md (1)

1-6: Changeset looks good

Minor bump matches the API surface change in calculateDose.

fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1)

54-60: Unit mismatch: Calcium expressed as kg Ca/ha while most agronomic tables use CaO

Double-check whether the advice engine returns Ca or CaO; if it is CaO the unit should read kg CaO/ha to avoid agronomist confusion.

fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice._index.tsx (1)

37-64: Guard against missing principal_id

getSession(request) can legally return a session object without a principal_id (e.g. anonymous visitor, corrupt cookie).
Both getFields and downstream services will crash with an opaque 500. Bail out early:

 const session = await getSession(request)
-if (!session.principal_id) throw new Error("Unauthenticated")
+if (!session?.principal_id) {
+  throw new Response("Unauthenticated", { status: 401 })
+}
fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (1)

1-8: ⚠️ Potential issue

data is not exported by react-router; use json

react-router v6.4+ exposes json for constructing typed Response objects. Importing data will throw at compile time.

-import {
-    type LoaderFunctionArgs,
-    type MetaFunction,
-    Outlet,
-    data,
-    useLoaderData,
-} from "react-router"
+import {
+    type LoaderFunctionArgs,
+    type MetaFunction,
+    Outlet,
+    json,
+    useLoaderData,
+} from "react-router"

Replace subsequent data( calls with json(.

⛔ Skipped due to learnings
Learnt from: SvenVw
PR: SvenVw/fdm#132
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.access.tsx:54-68
Timestamp: 2025-04-29T11:28:44.181Z
Learning: In React Router v7, the `json()` function has been replaced with `data()` for creating responses in loaders and actions.
Learnt from: SvenVw
PR: SvenVw/fdm#132
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.access.tsx:54-68
Timestamp: 2025-04-29T11:28:44.181Z
Learning: In React Router v7, the `json()` function has been deprecated and removed. Instead, either return plain JavaScript objects directly from loaders/actions, or use the `data()` function for responses with custom status codes and headers.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-calculator/src/doses/d.ts (2)

1-20: Excellent JSDoc documentation!

The documentation clearly describes each nutrient dose property with proper units and explanations.


7-8: Note: Property naming convention changed from oxide to elemental notation

The properties have been renamed from p_dose_p2o5 and p_dose_k2o to p_dose_p and p_dose_k. While the JSDoc still mentions P2O5 and K2O in the descriptions, the property names use elemental notation. Ensure this naming convention change is intentional and consistently applied throughout the codebase.

fdm-calculator/src/doses/calculate-dose.test.ts (2)

24-90: Comprehensive test coverage for all nutrients

The test properly validates calculations for all nutrient types including macro and micronutrients, with appropriate use of toBeCloseTo for floating-point precision.


5-21: Good practice: Centralized initial dose baseline

Using a shared initialDose constant ensures consistency across tests and makes it easy to maintain the expected zero-state structure.

fdm-calculator/src/doses/calculate-dose.ts (2)

4-36: Excellent documentation with clear examples

The JSDoc provides comprehensive parameter descriptions, return type details, error conditions, and a practical usage example.


97-121: Correct unit conversions for agricultural standards

The dose calculations properly convert nutrient rates:

  • Macro/secondary nutrients: g/kg → kg/kg (÷1000)
  • Trace elements: mg/kg → kg/kg (÷1000000)

This aligns with standard agricultural measurement units.

Comment thread fdm-app/app/integrations/nmi.ts Outdated
Comment thread fdm-app/app/integrations/nmi.ts Outdated
Comment thread fdm-app/app/components/blocks/nutrient-advice/cards.tsx Outdated
Comment thread fdm-app/app/components/blocks/nutrient-advice/cards.tsx
Comment thread fdm-app/app/components/blocks/nutrient-advice/kpi.tsx
Comment thread fdm-app/app/components/blocks/nutrient-advice/kpi.tsx
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: 8

🔭 Outside diff range comments (3)
fdm-core/src/fertilizer.ts (1)

120-130: 💡 Verification agent

🧩 Analysis chain

Typo fixed – double-check other call-sites

p_cr_rt is now correct, nice. Please grep the repo for the old pr_cr_rt spelling to avoid silent runtime undefined values.


🏁 Script executed:

#!/bin/bash
# Find any remaining occurrences of the old typo
rg --line-number 'pr_cr_rt'

Length of output: 195


Fix remaining pr_cr_rt typos

Two more instances of the old typo were found. Please update them to p_cr_rt to prevent runtime undefined values:

• fdm-core/src/cultivation.test.ts:712

-                    pr_cr_rt: 350,
+                    p_cr_rt: 350,

• fdm-app/app/routes/farm.$b_id_farm.fertilizers.new.tsx:295

-                pr_cr_rt: undefined,
+                p_cr_rt: undefined,
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.tsx (1)

112-123: ⚠️ Potential issue

Type mismatch risk between loader and UI

loader() now returns dose: dose.dose, whereas <FertilizerApplicationsCards> expects a full Dose object (it accesses p_dose_n, p_dose_k, …).

If calculateDose() returns
{ dose: number } → runtime error in the UI.
{ dose: Dose } → OK.

Please confirm the actual return type; otherwise pass the full object and let the component pick the fields it needs.

fdm-core/src/fertilizer.test.ts (1)

120-132: 🛠️ Refactor suggestion

Heavy duplication – extract a reusable helper

Each test re-creates the same gigantic fertilizer object. This hurts readability and maintenance.

Create a createTestFertilizer() factory in a test util and spread-override only the fields that vary:

// test/utils/fertilizer.ts
export function createTestFertilizer(overrides: Partial<FertilizerInput> = {}) {
  return {
    p_name_nl: "Test Fertilizer",
    /* … full default object … */
    ...overrides,
  };
}

Then in tests:

await addFertilizerToCatalogue(fdm, principal_id, b_id_farm, createTestFertilizer({
  p_cr_rt: 350,
}));

Also applies to: 195-205, 270-280, 360-370, 455-465, 669-679

🧹 Nitpick comments (24)
fdm-calculator/src/doses/d.ts (2)

1-20: JSDoc no longer matches the renamed properties

The text still refers to “phosphate (P2O5)” and “potassium (K2O)” while the keys were renamed to p_dose_p and p_dose_k.
Either update the wording (e.g. “elemental P”, “elemental K”) or revert to the original *_p2o5 / *_k2o keys to avoid confusion in downstream consumers and generated docs.


22-23: Index signature is overly permissive

[key: string]: string | number | undefined allows any string key with a string value, yet every declared nutrient dose is numeric.
Narrowing to number | undefined prevents accidental string assignment and preserves type-safety.

-    [key: string]: string | number | undefined
+    [key: string]: number | undefined
.changeset/calm-candies-mix.md (1)

5-5: Minor phrasing nit

“Add nutrient advice as new application” → “Add Nutrient Advice application” for consistent casing with other changeset titles.

.changeset/eager-mangos-shake.md (1)

5-5: Clarify parameter name in description

Consider quoting the incorrect name as well for clarity:
“Fix typo: pr_cr_rtp_cr_rt in addFertilizerToCatalogue input parameter.”

.changeset/rich-eagles-rest.md (1)

5-5: Spelling

“Expand number of nutrients in the output of calculateDose

.changeset/solid-planes-sell.md (1)

1-6: Consider adding a short rationale to the changeset body

The summary only states what changed. A one-liner explaining why the extra per-application dose information is useful (e.g. UI needs, API contract) makes the changelog more helpful downstream.

fdm-app/app/components/blocks/fertilizer-applications/cards.tsx (1)

124-130: Verify that the new p_dose_k/p_dose_p fields are always defined

If calculateDose omits a property, Math.round(undefined) in the card will display NaN kg/ha.
A tiny guard avoids leaking this to production:

- value: dose.p_dose_k,
+ value: dose.p_dose_k ?? 0,  // default to 0 when undefined
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.cultivations.$b_lu_catalogue.fertilizers.tsx (1)

135-138: Minor perf / readability: consider a Map instead of O(n²) duplicate detection

Array#find inside the reduce creates an O(n²) worst-case loop when many applications exist.
Using a Map keyed by the composite signature (e.g. ${p_id_catalogue}-${p_app_amount}-${p_app_method}-${p_app_date.getTime()}) removes the nested scan and simplifies the code.

fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1)

1-8: Nit: make the descriptor immutable for stronger type safety

-export type NutrientDescription = {
+export type NutrientDescription = {
     name:  string,
     symbol: string
     type: "primary" | "secondary" | "trace",
     unit: string
     adviceParameter: string
     doseParameter: string
-}
+} & Readonly<Record<string, unknown>>

Alternatively, declare the returned array in nutrients.ts as as const so callers get literal string types instead of broad string.

fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1)

3-5: Return the array as const to preserve literal types

-    const nutrientDescription: NutrientDescription[] = [
+    const nutrientDescription = [-    ]
-    return nutrientDescription
+    ] as const satisfies readonly NutrientDescription[]
+    return nutrientDescription

Benefits:
• Callers get exact literal types for symbol, unit, … (better autocomplete / exhaustiveness checks).
• No extra variable annotation needed.

fdm-app/app/components/blocks/sidebar/apps.tsx (2)

48-55: Reduce duplication when building navigation links

You now have three nearly identical blocks (atlasLink, nutrientBalanceLink, nutrientAdviceLink).
Consider a helper:

function buildLink(base: string): string | undefined {
  if (isCreateFarmWizard) return undefined
  return farmId ? `/farm/${farmId}/${selectedCalendar}/${base}` : undefined
}

…and call buildLink("nutrient_advice"), etc.


104-123: Accessibility: indicate disabled state with aria-disabled

The fallback <span> variant visually mutes the item but does not expose disabled state to screen readers.
Add aria-disabled="true" (or drop‐in tabIndex={-1}) to avoid confusing keyboard users.

fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice._index.tsx (2)

37-64: Consistent error-handling style between loaders

Other loaders (fertilizers.tsx, etc.) return handleLoaderError(error) while this one throw handleLoaderError(error).
Both work, but mixing patterns can confuse future maintainers and complicate test setups. Pick one style project-wide.


40-44: Edge-case: guard against empty but defined b_id_farm

if (!b_id_farm) rejects "" and undefined, but not the string " ".
if (!b_id_farm?.trim()) ensures the param contains a non-blank value.

fdm-calculator/src/doses/get-dose-field.test.ts (2)

128-166: Assertion is brittle – use partial matching for dynamic objects

toEqual requires an exact structural match, which makes the test fragile when extra properties are added by the implementation (e.g. future nutrients). Prefer toMatchObject (or expect.objectContaining) to assert only the properties that matter for this scenario.

- expect(await getDoseForField({ fdm, principal_id, b_id })).toEqual(
-     expectedResult,
- )
+ expect(await getDoseForField({ fdm, principal_id, b_id })).toMatchObject(
+     expectedResult,
+ )

203-225: Same brittleness in the “no applications” case

Apply the same partial-matching strategy here for consistency and future-proofing.

fdm-app/app/components/blocks/header/nutrient-advice.tsx (1)

45-51: UX: show a fallback label when b_name is missing

Currently undefined will render literally. Small guard avoids an empty menu item.

- {option.b_name}
+ {option.b_name ?? "Onbekend perceel"}
fdm-app/app/integrations/nmi.ts (2)

3-3: Remove unused import

number is imported from zod but never used.


148-149: currentSoilData typed as any

Declare a proper interface for the soil-measurement objects to regain type-safety and satisfy noExplicitAny.

interface SoilMeasurement {
    parameter: string
    value: number
    a_depth_lower: number
}

async function getNutrientAdvice(
    b_lu_catalogue: string,
    b_centroid: [number, number],
-   currentSoilData: any,
+   currentSoilData: SoilMeasurement[],
)
🧰 Tools
🪛 Biome (1.9.4)

[error] 148-148: Unexpected any. Specify a different type.

any disables many type checking rules. Its use should be avoided.

(lint/suspicious/noExplicitAny)

fdm-app/app/components/blocks/nutrient-advice/cards.tsx (1)

95-109: percentage can exceed 100% for trace nutrients – adjust colour logic

Trace elements (e.g. Cu, Zn) may legitimately exceed 100 %. Instead of hard-coding EOC/others, consider parameterizing the threshold per nutrient or simply clamping the value.

Not urgent, but may avoid misleading orange/red bars for micronutrients.

fdm-calculator/src/doses/calculate-dose.test.ts (2)

69-86: Magic expected values – add inline explanation

The constants (e.g. 0.000006 kg Na / ha) are not self-evident. Readers (and future maintainers) cannot verify correctness without manual recalculation.

Add a short comment explaining the formula or intermediary steps for at least one micro-nutrient; this speeds up future test audits.


92-110: initialDose duplication – expose from calculator package

initialDose is re-declared in multiple test files. Export a helper (e.g. createZeroDose()) from fdm-calculator to keep tests DRY and synced with model changes.

fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (1)

127-131: Component name is misleading.

FarmBalanceNitrogenBlock no longer describes the nutrient-advice layout. Rename for clarity and easier grep-ability.

fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (1)

95-107: Zero advice values distort deficit detection.

percentage = advice ? … : 0 marks nutrients with advice === 0 as 0 % applied → always deficit. Skip nutrients with a zero advice to avoid false warnings.

-if (percentage < deficitThreshold) {
+if (advice > 0 && percentage < deficitThreshold) {

Apply similar guard in the excess card (percentage >= …).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 96aca83 and 858ae43.

⛔ Files ignored due to path filters (2)
  • fdm-app/app/components/ui/progress.tsx is excluded by !fdm-app/app/components/ui/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (24)
  • .changeset/calm-candies-mix.md (1 hunks)
  • .changeset/eager-mangos-shake.md (1 hunks)
  • .changeset/rich-eagles-rest.md (1 hunks)
  • .changeset/solid-planes-sell.md (1 hunks)
  • fdm-app/app/components/blocks/fertilizer-applications/cards.tsx (1 hunks)
  • fdm-app/app/components/blocks/header/nutrient-advice.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/cards.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1 hunks)
  • fdm-app/app/components/blocks/sidebar/apps.tsx (3 hunks)
  • fdm-app/app/integrations/nmi.ts (2 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.tsx (1 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx (1 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice._index.tsx (1 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (1 hunks)
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.cultivations.$b_lu_catalogue.fertilizers.tsx (1 hunks)
  • fdm-app/package.json (1 hunks)
  • fdm-calculator/src/doses/calculate-dose.test.ts (1 hunks)
  • fdm-calculator/src/doses/calculate-dose.ts (1 hunks)
  • fdm-calculator/src/doses/d.ts (1 hunks)
  • fdm-calculator/src/doses/get-dose-field.test.ts (2 hunks)
  • fdm-core/src/fertilizer.test.ts (7 hunks)
  • fdm-core/src/fertilizer.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
.changeset/solid-planes-sell.md (1)
Learnt from: SvenVw
PR: SvenVw/fdm#75
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.fertilizer.tsx:68-71
Timestamp: 2025-02-14T09:56:37.606Z
Learning: The `calculateDose` function in `@svenvw/fdm-calculator` is a synchronous function that includes built-in validation for negative application amounts and nutrient rates.
.changeset/rich-eagles-rest.md (1)
Learnt from: SvenVw
PR: SvenVw/fdm#75
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.fertilizer.tsx:68-71
Timestamp: 2025-02-14T09:56:37.606Z
Learning: The `calculateDose` function in `@svenvw/fdm-calculator` is a synchronous function that includes built-in validation for negative application amounts and nutrient rates.
fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (1)
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-calculator/src/doses/calculate-dose.ts (1)
Learnt from: SvenVw
PR: SvenVw/fdm#75
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.fertilizer.tsx:68-71
Timestamp: 2025-02-14T09:56:37.606Z
Learning: The `calculateDose` function in `@svenvw/fdm-calculator` is a synchronous function that includes built-in validation for negative application amounts and nutrient rates.
🧬 Code Graph Analysis (5)
fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1)
fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1)
  • NutrientDescription (1-8)
fdm-core/src/fertilizer.ts (1)
fdm-core/src/db/schema.ts (1)
  • fertilizersCatalogueTypeInsert (239-240)
fdm-app/app/components/blocks/header/nutrient-advice.tsx (1)
fdm-app/app/store/calendar.tsx (1)
  • useCalendarStore (11-15)
fdm-calculator/src/doses/calculate-dose.ts (2)
fdm-calculator/src/doses/d.ts (1)
  • Dose (21-39)
fdm-core/src/db/schema.ts (1)
  • fertilizers (112-120)
fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (3)
fdm-calculator/src/doses/d.ts (1)
  • Dose (21-39)
fdm-app/app/components/custom/loadingspinner.tsx (1)
  • LoadingSpinner (13-35)
fdm-app/app/components/blocks/nutrient-advice/types.d.ts (1)
  • NutrientDescription (1-8)
🪛 Biome (1.9.4)
fdm-app/app/integrations/nmi.ts

[error] 148-148: Unexpected any. Specify a different type.

any disables many type checking rules. Its use should be avoided.

(lint/suspicious/noExplicitAny)

🔇 Additional comments (8)
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.cultivations.$b_lu_catalogue.fertilizers.tsx (1)

145-146: Verify if only the scalar dose value is sufficient downstream

The loader now exposes dose: dose.dose, discarding the rest of the object returned by calculateDose.
Please confirm that:

  1. <FertilizerApplicationsCards /> consumes a scalar number and not the richer object that previously contained e.g. per-nutrient splits or meta-data.
  2. All other loaders that supply dose to shared components have been refactored identically, otherwise type mismatches will surface.

If the full object is still required somewhere, re-expose it or rename the scalar (e.g. totalDose) to avoid confusion.

fdm-app/app/components/blocks/nutrient-advice/nutrients.ts (1)

70-76: Consistency: unit for trace elements should include the oxide form or not ‑ choose one

Most secondary nutrients use oxide units (kg MgO/ha, kg K2O/ha), but trace elements list the element itself (kg Cu/ha).
Confirm this is intentional; agronomy tables often use oxides for P, K, Mg but not necessarily for Cu, Zn etc. If consistency is desirable adjust here.

fdm-app/app/components/blocks/sidebar/apps.tsx (1)

5-6: Verify BookOpenText exists in current lucide-react version

Some Lucide releases only expose BookOpen. Using a non-existing icon will crash at runtime.

fdm-app/app/components/blocks/header/nutrient-advice.tsx (1)

1-3: ⚠️ Potential issue

Import should come from react-router-dom (or Remix) – avoid the slim react-router entry

Unless you explicitly add react-router to your bundle, import hooks & components from react-router-dom (@remix-run/react in Remix). The thin react-router package excludes DOM bindings and may break at runtime.

-import { NavLink, useLocation } from "react-router"
+import { NavLink, useLocation } from "react-router-dom"
⛔ Skipped due to learnings
Learnt from: SvenVw
PR: SvenVw/fdm#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.
Learnt from: SvenVw
PR: SvenVw/fdm#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`.
Learnt from: SvenVw
PR: SvenVw/fdm#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`.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-app/app/components/blocks/nutrient-advice/cards.tsx (1)

160-172: Date formatting assumes Date object

p_app_date may arrive as ISO string from the API. Wrap in new Date(...) or ensure type is Date upstream to avoid “Invalid Date” in Safari.

- format(fertilizerApplication?.p_app_date, "PP", { locale: nl })
+ format(new Date(fertilizerApplication.p_app_date), "PP", { locale: nl })
fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.tsx (2)

91-99: Guard against getFields returning undefined.

getFields might yield undefined or an empty array; calling .map without a check will crash. Consider:

-const fields = await getFields(...)
-const fieldOptions = fields.map(...)
+const fields = (await getFields(...)) ?? []
+if (fields.length === 0) {
+    throw json("not found: fields", { status: 404 })
+}
+const fieldOptions = fields.map(...)

1-8: ⚠️ Potential issue

Replace non-existent data() with json() (or plain Response).

react-router exports json, defer, redirect, but not data. Importing and calling data() will throw instantly in the loader, so the route can never resolve.

-import {
-    type LoaderFunctionArgs,
-    type MetaFunction,
-    Outlet,
-    data,
-    useLoaderData,
-} from "react-router"
+import {
+    type LoaderFunctionArgs,
+    type MetaFunction,
+    Outlet,
+    json,
+    useLoaderData,
+} from "react-router"
...
-            throw data("invalid: b_id_farm", { status: 400 })
+            throw json("invalid: b_id_farm", { status: 400 })

Repeat for the two other data() calls.

Also applies to: 50-54, 68-72

⛔ Skipped due to learnings
Learnt from: SvenVw
PR: SvenVw/fdm#132
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.access.tsx:54-68
Timestamp: 2025-04-29T11:28:44.181Z
Learning: In React Router v7, the `json()` function has been replaced with `data()` for creating responses in loaders and actions.
Learnt from: SvenVw
PR: SvenVw/fdm#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.
fdm-calculator/src/doses/calculate-dose.ts (1)

48-67: Property list likely misses p_eoc_rt and silently ignores unknown keys.

The validation array contains "p_eoc" while later you read fertilizer.p_eoc.
If the database field is actually p_eoc_rt (keeping the _rt suffix like other nutrients) the current guard won’t catch negative values and downstream math will always multiply by undefined → 0. Verify the correct key and keep naming consistent.

Comment thread fdm-app/package.json
Comment thread fdm-app/app/components/blocks/fertilizer-applications/cards.tsx
Comment thread fdm-app/app/components/blocks/header/nutrient-advice.tsx
Comment thread fdm-app/app/integrations/nmi.ts Outdated
Comment thread fdm-app/app/integrations/nmi.ts Outdated
Comment thread fdm-app/app/components/blocks/nutrient-advice/cards.tsx Outdated
Comment thread fdm-calculator/src/doses/calculate-dose.ts Outdated
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: 2

♻️ Duplicate comments (1)
fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (1)

268-272: (Duplicate) No loading state style for excess-card icon container

The icon container of the excess KPI does not handle navigation.state === "loading", unlike the deficit card.
This was mentioned previously and marked “as intended”; re-flagging only for completeness.

🧹 Nitpick comments (3)
fdm-calculator/src/doses/calculate-dose.ts (3)

65-70: False negatives in the non-negative-rate check

fert[rate] ? fert[rate] < 0 : false skips validation when the value is 0 or explicitly set to null.
A simpler, safer guard:

-            nutrientRates.some((rate) => (fert[rate] ? fert[rate] < 0 : false)),
+            nutrientRates.some((rate) => fert[rate] !== undefined && fert[rate]! < 0),

This still allows 0, but no longer swallows null.


100-102: if (fertilizer) is redundant

The preceding throw guarantees fertilizer is defined – the extra guard and indentation can be dropped to reduce nesting.


129-134: Minor: p_app_id check is unnecessary

totalDose never contains p_app_id, so the if (key !== "p_app_id") branch costs CPU without benefit.
Either remove the key from the iteration list up-front or drop the conditional.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 858ae43 and f233043.

📒 Files selected for processing (6)
  • fdm-app/app/components/blocks/header/nutrient-advice.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/cards.tsx (1 hunks)
  • fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (1 hunks)
  • fdm-app/app/integrations/nmi.ts (2 hunks)
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx (1 hunks)
  • fdm-calculator/src/doses/calculate-dose.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • fdm-app/app/integrations/nmi.ts
  • fdm-app/app/components/blocks/nutrient-advice/cards.tsx
  • fdm-app/app/routes/farm.$b_id_farm.$calendar.nutrient_advice.$b_id.tsx
  • fdm-app/app/components/blocks/header/nutrient-advice.tsx
🧰 Additional context used
🧠 Learnings (1)
fdm-calculator/src/doses/calculate-dose.ts (1)
Learnt from: SvenVw
PR: SvenVw/fdm#75
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.fertilizer.tsx:68-71
Timestamp: 2025-02-14T09:56:37.606Z
Learning: The `calculateDose` function in `@svenvw/fdm-calculator` is a synchronous function that includes built-in validation for negative application amounts and nutrient rates.
🧬 Code Graph Analysis (1)
fdm-calculator/src/doses/calculate-dose.ts (2)
fdm-calculator/src/doses/d.ts (1)
  • Dose (21-39)
fdm-core/src/db/schema.ts (1)
  • fertilizers (112-120)
🪛 GitHub Check: calculator (22)
fdm-calculator/src/doses/calculate-dose.ts

[failure] 96-96: src/doses/calculate-dose.test.ts > calculateDose > should handle empty fertilizers array
Error: Fertilizer fert1 not found for application app1
❯ calculateDose src/doses/calculate-dose.ts:96:19
❯ src/doses/calculate-dose.test.ts:158:50


[failure] 96-96: src/doses/calculate-dose.test.ts > calculateDose > should handle missing fertilizers by returning zero doses
Error: Fertilizer fert_missing not found for application app1
❯ calculateDose src/doses/calculate-dose.ts:96:19
❯ src/doses/calculate-dose.test.ts:137:50

🔇 Additional comments (1)
fdm-app/app/components/blocks/nutrient-advice/kpi.tsx (1)

7-7: ⚠️ Potential issue

Import useNavigation from react-router-dom instead of react-router

The hook is exported by react-router-dom; importing from react-router works only in monorepo builds where both packages are re-exported, which is brittle.

-import { useNavigation } from "react-router"
+import { useNavigation } from "react-router-dom"
⛔ Skipped due to learnings
Learnt from: SvenVw
PR: SvenVw/fdm#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`.
Learnt from: SvenVw
PR: SvenVw/fdm#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`.

Comment thread fdm-calculator/src/doses/calculate-dose.ts
Comment thread fdm-calculator/src/doses/calculate-dose.ts
@SvenVw SvenVw requested a review from gerardhros June 13, 2025 11:07
}: NutrientKPICardForNutrientExcessProps) {
const navigation = useNavigation()
const excessThreshold = 105

Copy link
Copy Markdown
Collaborator

@gerardhros gerardhros Jun 13, 2025

Choose a reason for hiding this comment

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

is thre threshold fixed? is a 5% deviation not "acceptable deviation or uncertainty"?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It is now hardcoded, but we can adjust the value if needed

},
{
name: "Organische koolstof",
symbol: "EOC",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

oke. we might check use of EOM and EOC for use in the srm list.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The NMI API returns an advice for EOC, if am a correct, so I would suggest we use the parameter returnded by NMI API

},
{
name: "Boor",
symbol: "B",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

niet Borium?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Can be used both, but Wikipedia uses Boor as first name so I thought to use that one: https://nl.wikipedia.org/wiki/Boor_(element)

Comment thread fdm-app/app/components/blocks/nutrient-advice/nutrients.ts
a_nmin_cc_d30 = item.value
} else if (item.a_depth_lower <= 60) {
a_nmin_cc_d60 = item.value
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

note that a_nmin_cc_d30 and a_nmin_cc_d60 are also paramters in pandex. the new SoilReader package is using the new variable names a_nh4_cc, a_no3_cc and a_nmin_cc with separate depth.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but as the NMI API uses A_NMIN_CC_D30 and A_NMIN_CC_D60 a conversion has to be made

* @param {LoaderFunctionArgs} args - The arguments passed to the loader function, including the request and parameters.
* @returns {Promise<{ field: Field }>} - A promise that resolves to an object containing the fetched field data.
* @throws {Error} - Throws an error if the `b_id_farm` or `b_id` parameter is missing.
* @throws {Error} - Throws an error if any error occurs during the process, handled by `handleLoaderError`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

throws has two outputs/error messages

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, both errors are possible.

{
p_id: "fert1",
p_n_rt: 100,
p_p_rt: 50,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

its correct that n_effective not in?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Can you please explain? This is just regulat total nitrogen as that is the definition of p_n_rt

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.

nice update. please note that soilReader output has changed as requested.

@SvenVw SvenVw merged commit 0182b19 into development Jun 18, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants