Release/2025-10#320
Conversation
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Sven Verweij <37927107+SvenVw@users.noreply.github.com>
Add four Dutch b_acquiring_method options; export acquiringMethodEnum
Chore/20251014
…ing to perform grazing
…n`, `getGrazingIntention` and `getGrazingIntentions` to manage if a farm is planning to do grazing
…ications and grazing intentions as well
…den" or "volledig maaien"
Add default cultivation dates: schema fields, core util, app routes
Add per-field nitrogen balance, error capture, aggregation, UI updates
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
fdm-calculator/src/nutrient-advice/types.d.ts (1)
1-1: Import correctly added.The missing
CurrentSoilDataimport flagged in the previous review has been addressed.
🧹 Nitpick comments (3)
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx (1)
120-140: Consider adding loading and optimistic UI states.The Switch toggles work correctly, but users receive no visual feedback during the submission. Consider enhancing the UX by:
- Showing a loading state while
fetcher.state === "submitting"(e.g., disable the Switch or show a spinner)- Implementing optimistic UI updates so the Switch appears to respond immediately
- Displaying error feedback if the action fails
Example for loading state:
<Switch checked={hasGrazingIntention} + disabled={fetcher.state === "submitting"} onCheckedChange={() => {fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (1)
206-217: Verify undefined handling forhas_grazing_intention.The grasland logic treats
undefinedas falsy, defaulting to "volledig maaien" when grazing intention is not specified. If this is intentional (i.e., absence of grazing intention means no grazing), consider making it explicit for clarity:- return has_grazing_intention ? "beweiden" : "volledig maaien" + return has_grazing_intention === true ? "beweiden" : "volledig maaien"Otherwise, if
undefinedrepresents an unknown state that should be handled differently, additional logic may be needed.fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx (1)
421-438: Consider centralizing calendar-year normalization.The
parsedYear/currentYearnormalization just introduced here is duplicated infarm.create.$b_id_farm.$calendar.atlas.tsx(Lines 433-452). Pulling this into a shared helper (e.g., in~/lib/calendar) would keep the farms flows aligned and makes it easier to adjust validation in one place later. Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
fdm-app/app/components/blocks/fertilizer-applications/card.tsx(4 hunks)fdm-app/app/components/blocks/fertilizer/formschema.tsx(1 hunks)fdm-app/app/routes/about.whats-new._index.tsx(1 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx(1 hunks)fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx(2 hunks)fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts(7 hunks)fdm-calculator/src/nutrient-advice/index.ts(1 hunks)fdm-calculator/src/nutrient-advice/types.d.ts(1 hunks)fdm-core/src/cultivation.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- fdm-app/app/components/blocks/fertilizer/formschema.tsx
- fdm-app/app/components/blocks/fertilizer-applications/card.tsx
- fdm-calculator/src/nutrient-advice/index.ts
🧰 Additional context used
🧠 Learnings (31)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 156
File: fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts:295-303
Timestamp: 2025-07-21T12:06:07.070Z
Learning: Functions in the fdm-calculator with "NL2025" in their names are specifically designed for Netherlands 2025 agricultural norms calculation and hardcoded 2025 dates are appropriate in this context, as different years would have separate calculation modules.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
📚 Learning: 2024-11-25T14:50:16.074Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/app/routes/app.addfarm.$b_id_farm.fields.tsx:63-99
Timestamp: 2024-11-25T14:50:16.074Z
Learning: i18n will be added in future PRs; for now, hardcoded Dutch text is acceptable until a translation system is implemented.
Applied to files:
fdm-app/app/routes/about.whats-new._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsxfdm-core/src/cultivation.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-core/src/cultivation.tsfdm-calculator/src/nutrient-advice/types.d.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-core/src/cultivation.ts
📚 Learning: 2025-07-21T12:06:07.070Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 156
File: fdm-calculator/src/norms/nl/2025/stikstofgebruiksnorm.ts:295-303
Timestamp: 2025-07-21T12:06:07.070Z
Learning: Functions in the fdm-calculator with "NL2025" in their names are specifically designed for Netherlands 2025 agricultural norms calculation and hardcoded 2025 dates are appropriate in this context, as different years would have separate calculation modules.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-core/src/cultivation.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-core/src/cultivation.ts
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-core/src/cultivation.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: When using `updateField` from fdm-core, all 8 parameters must be provided in order: fdm, b_id, b_name, b_geometry, b_area, b_id_source, b_id_farm, and b_id_farm_source.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-core/src/cultivation.ts
📚 Learning: 2025-01-31T15:34:20.850Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx
📚 Learning: 2025-08-13T10:33:05.313Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-core/src/cultivation.tsfdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxfdm-core/src/cultivation.ts
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx
📚 Learning: 2025-01-24T11:39:57.805Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/cultivation.ts:625-671
Timestamp: 2025-01-24T11:39:57.805Z
Learning: Every cultivation has an associated `cultivationTerminating` record created by `addCultivation`, with `b_terminate_date` initially set to null. Therefore, `updateCultivation` can safely assume the record exists and only needs to handle updates.
Applied to files:
fdm-core/src/cultivation.ts
📚 Learning: 2025-07-31T11:38:50.661Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 194
File: fdm-core/src/harvest.ts:488-644
Timestamp: 2025-07-31T11:38:50.661Z
Learning: The validation logic in updateHarvest is intentionally different from checkHarvestDateCompability because updateHarvest is for updating existing harvests while checkHarvestDateCompability is for inserting new harvests. The insertion function includes checks that don't apply to updates, such as verifying no harvest already exists for "once" harvestable cultivations.
Applied to files:
fdm-core/src/cultivation.ts
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-core/src/cultivation.ts
📚 Learning: 2024-11-27T11:27:27.797Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T11:27:27.797Z
Learning: When cleaning up test data in `afterAll` hooks in `fdm-data/src/cultivations/index.test.ts`, use `fdm.delete(schema.tableName).execute();` to delete rows from a table without dropping the table itself.
Applied to files:
fdm-core/src/cultivation.ts
📚 Learning: 2025-01-24T11:38:05.693Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-24T11:38:05.693Z
Learning: The field creation and cultivation addition should be performed within a single database transaction to ensure atomicity and prevent transaction isolation issues that could lead to "Field does not exist" errors.
Applied to files:
fdm-core/src/cultivation.ts
🧬 Code graph analysis (6)
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx (2)
fdm-core/src/cultivation.ts (1)
getDefaultDatesOfCultivation(191-296)fdm-core/src/index.ts (1)
getDefaultDatesOfCultivation(46-46)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx (2)
fdm-core/src/cultivation.ts (1)
getDefaultDatesOfCultivation(191-296)fdm-core/src/index.ts (1)
getDefaultDatesOfCultivation(46-46)
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx (3)
fdm-app/app/lib/auth.server.ts (1)
getSession(55-92)fdm-app/app/lib/error.ts (2)
handleLoaderError(39-149)handleActionError(151-248)fdm-app/app/lib/calendar.ts (1)
getCalendarSelection(39-50)
fdm-core/src/cultivation.ts (4)
fdm-core/src/db/schema.ts (1)
cultivationsCatalogue(440-475)fdm-core/src/cultivation.d.ts (1)
CultivationDefaultDates(62-65)fdm-core/src/authorization.ts (1)
checkPermission(156-240)fdm-core/src/field.ts (1)
determineIfFieldIsProductive(660-675)
fdm-calculator/src/nutrient-advice/types.d.ts (1)
fdm-calculator/src/index.ts (3)
NutrientAdvice(56-56)NutrientAdviceResponse(58-58)NutrientAdviceInputs(57-57)
fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (2)
fdm-calculator/src/norms/nl/2025/value/types.d.ts (2)
NL2025NormsInput(14-26)GebruiksnormResult(160-170)fdm-core/src/calculator.ts (1)
withCalculationCache(163-284)
🔇 Additional comments (15)
fdm-app/app/routes/about.whats-new._index.tsx (1)
26-44: LGTM! Previous typo has been corrected.The new v0.24.0 changelog entry is well-structured and consistent with existing entries. The typo "bemetingsadvies" flagged in the previous review has been properly corrected to "bemestingsadvies" on line 31. The content is comprehensive, clearly formatted, and properly integrated at the top of the changelog array.
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx (4)
1-28: LGTM!All imports are properly structured and used throughout the file.
30-46: LGTM!The loader follows the standard pattern: validates parameters, authenticates via session, fetches data, and handles errors appropriately.
48-78: LGTM!The action correctly implements toggle logic by inverting the submitted state (line 68) and providing appropriate user feedback. Parameter validation and error handling follow project conventions.
84-87: Past issue resolved: dynamic year calculation implemented.The hard-coded 2025 ceiling flagged in the previous review has been fixed. The code now correctly computes
currentYeardynamically and filters up tocurrentYear + 1, ensuring the settings will continue working in future years.fdm-calculator/src/nutrient-advice/types.d.ts (3)
7-42: Well-structured nutrient advice type.The
NutrientAdvicetype is comprehensive and clearly documents all 17 nutrient fields with appropriate units. The consistent naming convention and detailed JSDoc comments make this easy to use and maintain.
47-73: Well-designed API response type.The
NutrientAdviceResponsetype follows standard API response patterns and makes good use of TypeScript literal types for theyieldclassandcutfields. The documentation clearly explains the conditional nature of thecutdata.
78-87: Undefined API key is properly handled.The implementation correctly validates the
nmiApiKeyat line 27-29 offdm-calculator/src/nutrient-advice/index.ts, throwing a descriptive error ("NMI API key not provided") if undefined or falsy. The type design is correct.fdm-calculator/src/norms/nl/2025/value/stikstofgebruiksnorm.ts (7)
1-5: LGTM! Clean integration of caching and precise arithmetic.The imports properly bring in
withCalculationCachefor memoization,Decimalfor precise nitrogen norm calculations, andpkgfor version-based cache keys. Path updates reflect the repository structure correctly.
148-152: LGTM! Improved function signature.Consolidating the sub-type determination into a single
subTypeOmschrijvingparameter simplifies the function interface and improves separation of concerns.
409-412: LGTM! Correct null handling.The strict equality check for
nullproperly handles ongoing cultivations (no end date) as completed vanggewassen.
534-538: LGTM! Clear naming and proper input extraction.The function naming convention (internal
calculate*vs public memoizedget*) is clear, andhas_grazing_intentionis correctly extracted from the input structure.
603-615: LGTM! Proper parameter threading.The
has_grazing_intentionand computedsubTypeOmschrijvingare correctly passed through the calculation pipeline.
632-650: LGTM! Excellent use of Decimal and defensive coding.Using
Decimal.jsensures precise arithmetic for nitrogen norm calculations. The non-negative guard (lines 642-644) is particularly good defensive coding that prevents invalid scenarios where korting (reduction) might exceed the base norm, which could otherwise produce nonsensical negative nitrogen limits.
653-669: LGTM! Proper memoization implementation.The wrapper correctly applies
withCalculationCacheto optimize repeated calculations. The cache key includes the function name and calculator version, ensuring proper cache invalidation when the calculation logic or version changes. This follows the established pattern from fdm-core.
|
@coderabbitai ignore |
✅ Actions performedReviews paused. |
There was a problem hiding this comment.
Actionable comments posted: 8
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/card.tsx (1)
22-186: Wire the newclassNameprop through to<Card>.You added
className?: stringto the public API but never apply it, so consumers can’t style the card despite the new prop.Apply this diff so the prop works:
-export function FertilizerApplicationCard({ - fertilizerApplications, - applicationMethodOptions, - fertilizers, - fertilizerOptions, -}: { +export function FertilizerApplicationCard({ + fertilizerApplications, + applicationMethodOptions, + fertilizers, + fertilizerOptions, + className, +}: { fertilizerApplications: FertilizerApplication[] applicationMethodOptions: { value: ApplicationMethods label: string }[] fertilizers: Fertilizer[] fertilizerOptions: FertilizerOption[] dose: Dose className?: string }) { @@ - return ( - <Card> + return ( + <Card className={className}>
♻️ Duplicate comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.$b_id.tsx (1)
417-425: Still hydrate the timestamp on the serverWe flagged this earlier: rendering
timestamp: new Date()keeps generating different values between SSR and hydration, so React warns every time this error card shows up. Capture the timestamp on the server (e.g. in the async loader block when you buildfieldNormData) and pass it through instead of instantiating it during render.- let errorMessage: string | null = null + let errorMessage: string | null = null + let errorTimestamp: string | null = null @@ - } catch (error) { - errorMessage = String(error).replace("Error: ", "") - fieldNormData = { ...fieldNormData, errorMessage } - } - - return { fieldNormData, errorMessage } + } catch (error) { + errorMessage = String(error).replace("Error: ", "") + errorTimestamp = new Date().toISOString() + fieldNormData = { + ...fieldNormData, + errorMessage, + errorTimestamp, + } + } + + return { fieldNormData, errorMessage, errorTimestamp } @@ - const { fieldNormData, errorMessage } = use(loaderData.asyncData) + const { fieldNormData, errorMessage, errorTimestamp } = use( + loaderData.asyncData, + ) @@ - timestamp: new Date(), + timestamp: errorTimestamp,
🧹 Nitpick comments (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx (1)
88-120: Consider improving error message format for better tracking.The loader correctly reports field-level errors to Sentry while allowing the page to render with partial data. However, joining
fieldErrorMessageswith",\n"creates a single string that may be harder to parse in error tracking tools.Consider passing the errors as structured data in the context:
if (nitrogenBalanceResult.hasErrors) { reportError( - nitrogenBalanceResult.fieldErrorMessages.join(",\n"), + new Error(`Nitrogen balance calculation failed for ${nitrogenBalanceResult.fieldErrorMessages.length} field(s)`), { page: "farm/{b_id_farm}/{calendar}/balance/nitrogen/_index", scope: "loader", }, { b_id_farm, timeframe, userId: session.principal_id, + fieldErrors: nitrogenBalanceResult.fieldErrorMessages, }, ) }This preserves the individual field error messages in the context while providing a clearer summary message.
| <p className="text-sm font-semibold"> | ||
| {normValue.toFixed(0)} kg | ||
| </p> | ||
| </div> | ||
| </div> | ||
| {filling !== undefined && ( | ||
| <div className="space-y-1"> | ||
| <p className="text-right text-xs text-muted-foreground"> | ||
| {fillingValue.toFixed(0)} kg gebruikt | ||
| </p> | ||
| <ProgressBar value={percentage} /> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Use the supplied unit when rendering norm values
NormItem always prints plain “kg”, so we lose the distinction between kg N/ha and kg P₂O₅/ha. That’s misleading in the per-field cards. Please render the passed unit for both the norm and the filling.
- <p className="text-sm font-semibold">
- {normValue.toFixed(0)} kg
- </p>
+ <p className="text-sm font-semibold">
+ {normValue.toFixed(0)} {unit}
+ </p>
@@
- <p className="text-right text-xs text-muted-foreground">
- {fillingValue.toFixed(0)} kg gebruikt
- </p>
+ <p className="text-right text-xs text-muted-foreground">
+ {fillingValue.toFixed(0)} {unit} gebruikt
+ </p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <p className="text-sm font-semibold"> | |
| {normValue.toFixed(0)} kg | |
| </p> | |
| </div> | |
| </div> | |
| {filling !== undefined && ( | |
| <div className="space-y-1"> | |
| <p className="text-right text-xs text-muted-foreground"> | |
| {fillingValue.toFixed(0)} kg gebruikt | |
| </p> | |
| <ProgressBar value={percentage} /> | |
| </div> | |
| )} | |
| <p className="text-sm font-semibold"> | |
| {normValue.toFixed(0)} {unit} | |
| </p> | |
| </div> | |
| </div> | |
| {filling !== undefined && ( | |
| <div className="space-y-1"> | |
| <p className="text-right text-xs text-muted-foreground"> | |
| {fillingValue.toFixed(0)} {unit} gebruikt | |
| </p> | |
| <ProgressBar value={percentage} /> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In fdm-app/app/components/blocks/norms/field-norms.tsx around lines 91 to 103,
the code currently renders a hardcoded "kg" for the norm and filling text;
change both occurrences to use the supplied unit prop (e.g.
`${normValue.toFixed(0)} ${unit}` and `${fillingValue.toFixed(0)} ${unit}`) so
the component shows kg N/ha vs kg P₂O₅/ha correctly; ensure you reference the
unit variable from props (or use a safe fallback like unit ?? 'kg' if necessary)
and update both the norm and filling display lines and any related aria/alt text
if present.
| <pre className="bg-gray-200 dark:bg-gray-800 p-4 rounded-md overflow-x-auto text-sm text-gray-800 dark:text-gray-200"> | ||
| {JSON.stringify( | ||
| { | ||
| message: errorMessage, | ||
| // message: | ||
| // fieldResult.errorMessage, | ||
| errorId: fieldResult.errorId, | ||
| page: page, | ||
| timestamp: new Date(), | ||
| }, |
There was a problem hiding this comment.
Avoid client-side new Date() in the error payload
When this branch renders, new Date() differs between SSR and hydration, triggering React warnings. Capture the timestamp where you build the error result in the loader (right after reportError) and include it on fieldResult, then render that stored value here.
- if (fieldResult.errorMessage) {
+ if (fieldResult.errorMessage) {
+ const errorTimestamp = new Date().toISOString()
const errorId = reportError(
fieldResult.errorMessage,
@@
fieldResult = {
b_id: b_id,
b_area: field?.b_area ?? 0,
errorMessage: fieldResult.errorMessage,
errorId: errorId,
+ errorTimestamp,
}
}
@@
- {JSON.stringify(
- {
- errorId: fieldResult.errorId,
- page: page,
- timestamp: new Date(),
- },
- null,
- 2,
- )}
+ {JSON.stringify(
+ {
+ errorId: fieldResult.errorId,
+ page,
+ timestamp:
+ fieldResult.errorTimestamp,
+ },
+ null,
+ 2,
+ )}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen.$b_id.tsx
around lines 237 to 245, the component currently calls new Date() during render
which creates a different timestamp between SSR and hydration and causes React
warnings; instead capture the timestamp in the loader immediately after calling
reportError and attach it to fieldResult (as a serializable value like ISO
string or epoch number), then render that stored timestamp here (use the
fieldResult.timestamp value) so SSR and client hydration use the same
precomputed value.
| { | ||
| renderChunk: (code, map) => { | ||
| const replacement = `"fdm-calculator:${packageJson.version}"` | ||
|
|
||
| const placeholder = `"fdm-calculator:{FDM_CALCULATOR_VERSION}"` | ||
| const occurrences = | ||
| code.match( | ||
| new RegExp( | ||
| placeholder.replace(/[|\\{}()[\]^$+*?.]/g, "\\$&"), | ||
| "g", | ||
| ), | ||
| ) || [] | ||
|
|
||
| if (occurrences.length === 0) { | ||
| console.warn( | ||
| `⚠️ Version placeholder "${placeholder}" not found in bundle`, | ||
| ) | ||
| } else if (occurrences.length > 1) { | ||
| console.warn( | ||
| `⚠️ Version placeholder "${placeholder}" appears ${occurrences.length} times`, | ||
| ) | ||
| } | ||
|
|
||
| if (replacement.length > placeholder.length) { | ||
| console.warn( | ||
| "⚠️ Replacement fdm-calculator version string ended up longer than the placeholder in package.ts. Source map will be broken.", | ||
| ) | ||
| } | ||
|
|
||
| return { | ||
| code: code.replace( | ||
| placeholder, | ||
| replacement.padEnd(placeholder.length, " "), // Pad to not break the source map | ||
| ), | ||
| map, | ||
| } | ||
| }, | ||
| }, // Modifies bundled package.ts to contain the actual package version |
There was a problem hiding this comment.
Fix the renderChunk hook signature and return value.
The parameter naming and return value are incorrect:
- Rollup's
renderChunkhook signature is(code, chunk, outputOptions)— the second parameter is chunk metadata, not a source map. - Returning
{ code, map }wheremapis actuallychunkis incorrect and may cause source map issues.
Since you're preserving code length via padding, return just the modified code to let Rollup maintain the existing source map.
Apply this diff:
- {
- renderChunk: (code, map) => {
+ {
+ renderChunk: (code) => {
const replacement = `"fdm-calculator:${packageJson.version}"`
const placeholder = `"fdm-calculator:{FDM_CALCULATOR_VERSION}"`
const occurrences =
code.match(
new RegExp(
placeholder.replace(/[|\\{}()[\]^$+*?.]/g, "\\$&"),
"g",
),
) || []
if (occurrences.length === 0) {
console.warn(
`⚠️ Version placeholder "${placeholder}" not found in bundle`,
)
} else if (occurrences.length > 1) {
console.warn(
`⚠️ Version placeholder "${placeholder}" appears ${occurrences.length} times`,
)
}
if (replacement.length > placeholder.length) {
console.warn(
"⚠️ Replacement fdm-calculator version string ended up longer than the placeholder in package.ts. Source map will be broken.",
)
}
return {
code: code.replace(
placeholder,
replacement.padEnd(placeholder.length, " "), // Pad to not break the source map
),
- map,
}
},
}, // Modifies bundled package.ts to contain the actual package version📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| renderChunk: (code, map) => { | |
| const replacement = `"fdm-calculator:${packageJson.version}"` | |
| const placeholder = `"fdm-calculator:{FDM_CALCULATOR_VERSION}"` | |
| const occurrences = | |
| code.match( | |
| new RegExp( | |
| placeholder.replace(/[|\\{}()[\]^$+*?.]/g, "\\$&"), | |
| "g", | |
| ), | |
| ) || [] | |
| if (occurrences.length === 0) { | |
| console.warn( | |
| `⚠️ Version placeholder "${placeholder}" not found in bundle`, | |
| ) | |
| } else if (occurrences.length > 1) { | |
| console.warn( | |
| `⚠️ Version placeholder "${placeholder}" appears ${occurrences.length} times`, | |
| ) | |
| } | |
| if (replacement.length > placeholder.length) { | |
| console.warn( | |
| "⚠️ Replacement fdm-calculator version string ended up longer than the placeholder in package.ts. Source map will be broken.", | |
| ) | |
| } | |
| return { | |
| code: code.replace( | |
| placeholder, | |
| replacement.padEnd(placeholder.length, " "), // Pad to not break the source map | |
| ), | |
| map, | |
| } | |
| }, | |
| }, // Modifies bundled package.ts to contain the actual package version | |
| { | |
| renderChunk: (code) => { | |
| const replacement = `"fdm-calculator:${packageJson.version}"` | |
| const placeholder = `"fdm-calculator:{FDM_CALCULATOR_VERSION}"` | |
| const occurrences = | |
| code.match( | |
| new RegExp( | |
| placeholder.replace(/[|\\{}()[\]^$+*?.]/g, "\\$&"), | |
| "g", | |
| ), | |
| ) || [] | |
| if (occurrences.length === 0) { | |
| console.warn( | |
| `⚠️ Version placeholder "${placeholder}" not found in bundle`, | |
| ) | |
| } else if (occurrences.length > 1) { | |
| console.warn( | |
| `⚠️ Version placeholder "${placeholder}" appears ${occurrences.length} times`, | |
| ) | |
| } | |
| if (replacement.length > placeholder.length) { | |
| console.warn( | |
| "⚠️ Replacement fdm-calculator version string ended up longer than the placeholder in package.ts. Source map will be broken.", | |
| ) | |
| } | |
| return { | |
| code: code.replace( | |
| placeholder, | |
| replacement.padEnd(placeholder.length, " "), // Pad to not break the source map | |
| ), | |
| } | |
| }, | |
| }, // Modifies bundled package.ts to contain the actual package version |
🤖 Prompt for AI Agents
In fdm-calculator/rollup.config.js around lines 33 to 70, the renderChunk hook
uses the wrong parameter names and returns an incorrect map object; change the
signature to (code, chunk, outputOptions) (or at least accept a second param as
chunk metadata), stop treating the second param as a source map, and return only
the modified code (a string) instead of { code, map } so Rollup preserves the
original source map; keep the placeholder/replacement logic and padding but
rename the second parameter and adjust the return to just the padded code
string.
| export const getNL2025DierlijkeMestGebruiksNorm = withCalculationCache( | ||
| calculateNL2025DierlijkeMestGebruiksNorm, | ||
| "calculateNL2025DierlijkeMestGebruiksNorm", | ||
| pkg.calculatorVersion, | ||
| ) |
There was a problem hiding this comment.
Verify the memoized wrapper signature matches withCalculationCache requirements.
The withCalculationCache wrapper is being used with calculateNL2025DierlijkeMestGebruiksNorm. According to the relevant code snippet from fdm-core/src/calculator.ts, withCalculationCache returns a function with signature async (fdm: FdmType, input: T_Input) => Promise<T_Output>.
However, the original calculateNL2025DierlijkeMestGebruiksNorm function (line 150) only takes input: NL2025NormsInput and doesn't have an fdm parameter. This mismatch could cause issues.
Verify this implementation by running the following script:
#!/bin/bash
# Check the signature of calculateNL2025DierlijkeMestGebruiksNorm and how withCalculationCache is used
rg -n -A 5 "export async function calculateNL2025DierlijkeMestGebruiksNorm" fdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.ts
rg -n -A 10 "export function withCalculationCache" fdm-core/src/calculator.ts🤖 Prompt for AI Agents
In fdm-calculator/src/norms/nl/2025/value/dierlijke-mest-gebruiksnorm.ts around
lines 206-210, the memoized wrapper withCalculationCache expects a function of
signature async(fdm: FdmType, input: T_Input) => Promise<T_Output>, but
calculateNL2025DierlijkeMestGebruiksNorm currently only accepts input:
NL2025NormsInput; change the calculateNL2025DierlijkeMestGebruiksNorm function
signature to async function calculateNL2025DierlijkeMestGebruiksNorm(fdm:
FdmType, input: NL2025NormsInput) and update any internal references to use the
provided fdm parameter (and update all local callers/tests to pass the fdm
argument) so the function matches withCalculationCache requirements.
| import { withCalculationCache } from "@svenvw/fdm-core" | ||
| import pkg from "../package" | ||
| import type { | ||
| NutrientAdvice, | ||
| NutrientAdviceInputs, | ||
| NutrientAdviceResponse, | ||
| } from "./types" | ||
|
|
||
| /** | ||
| * Requests nutrient advice from the NMI API based on provided field and soil data. | ||
| * | ||
| * @param {NutrientAdviceInputs} inputs - An object containing all necessary inputs for the nutrient advice calculation. | ||
| * @param {string} inputs.b_lu_catalogue - The BRP cultivation catalogue identifier (e.g., "nl_2014"). | ||
| * @param {[number, number]} inputs.b_centroid - The centroid coordinates of the field [longitude, latitude]. | ||
| * @param {CurrentSoilData} inputs.currentSoilData - Current soil data for the field, used to extract Nmin values and other soil parameters. | ||
| * @param {string | undefined} inputs.nmiApiKey - The NMI API key for authentication. | ||
| * @returns {Promise<NutrientAdvice>} A promise that resolves to an object containing the nutrient advice for the year. | ||
| * @throws {Error} If the NMI API key is not provided or if the request to the NMI API fails. | ||
| */ | ||
| export async function requestNutrientAdvice({ | ||
| b_lu_catalogue, | ||
| b_centroid, | ||
| currentSoilData, | ||
| nmiApiKey, | ||
| }: NutrientAdviceInputs): Promise<NutrientAdvice> { | ||
| try { | ||
| if (!nmiApiKey) { | ||
| throw new Error("NMI API key not provided") | ||
| } | ||
|
|
||
| let a_nmin_cc_d30: number | undefined | ||
| let a_nmin_cc_d60: number | undefined | ||
| const soilData: Record<string, number | string> = {} | ||
| // Extract Nmin values and other soil parameters from currentSoilData | ||
| for (const item of currentSoilData) { | ||
| // Exclude 'a_nmin_cc' from soilData as it's handled separately | ||
| if (item.parameter === "a_nmin_cc") { | ||
| if (item.a_depth_lower <= 30) { | ||
| a_nmin_cc_d30 = item.value | ||
| } else if (item.a_depth_lower <= 60) { | ||
| a_nmin_cc_d60 = item.value | ||
| } | ||
| continue // Skip adding a_nmin_cc to soilData | ||
| } | ||
| soilData[item.parameter] = item.value | ||
| } | ||
|
|
||
| // Create request body for the NMI API | ||
| const brpSegments = b_lu_catalogue.split("_") | ||
| const brpCode = brpSegments[brpSegments.length - 1] | ||
| if (!brpCode) { | ||
| throw new Error("Invalid b_lu_catalogue provided") | ||
| } | ||
| const body = { | ||
| a_lon: b_centroid[0], | ||
| a_lat: b_centroid[1], | ||
| b_lu_brp: [brpCode], | ||
| a_nmin_cc_d30: a_nmin_cc_d30, | ||
| a_nmin_cc_d60: a_nmin_cc_d60, | ||
| ...soilData, // Include all other soil data parameters | ||
| } | ||
|
|
||
| // Send request to NMI API | ||
| const responseApi = await fetch( | ||
| "https://api.nmi-agro.nl/bemestingsplan/nutrients", | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| Authorization: `Bearer ${nmiApiKey}`, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify(body), | ||
| }, | ||
| ) | ||
|
|
||
| if (!responseApi.ok) { | ||
| throw new Error( | ||
| `Request to NMI API failed with status ${responseApi.status}: ${responseApi.statusText}`, | ||
| ) | ||
| } | ||
|
|
||
| const result: NutrientAdviceResponse = await responseApi.json() | ||
| const response: NutrientAdvice = result.data.year | ||
|
|
||
| return response | ||
| } catch (error) { | ||
| console.error("Error fetching nutrient advice:", error) | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Cached version of `requestNutrientAdvice`. | ||
| * This function uses `withCalculationCache` to store and retrieve results, | ||
| * improving performance for repeated calls with the same inputs. | ||
| * The cache key is based on the inputs and the calculator version. | ||
| */ | ||
| export const getNutrientAdvice = withCalculationCache( | ||
| requestNutrientAdvice, | ||
| "requestNutrientAdvice", | ||
| pkg.calculatorVersion, | ||
| ) |
There was a problem hiding this comment.
Remove caching that persists the NMI API key.
withCalculationCache hashes and stores the entire inputs object in calculation_cache, so every call writes the plain-text nmiApiKey to the database. That’s a credential leak and a critical security violation. Until the caching layer can sanitize secrets, this wrapper must not include the API key in the cached payload—either strip the key before caching or drop the decorator for this call. One safe quick fix is to revert to a thin forwarder without caching:
-import { withCalculationCache } from "@svenvw/fdm-core"
-import pkg from "../package"
+import type { FdmType } from "@svenvw/fdm-core"
@@
-export const getNutrientAdvice = withCalculationCache(
- requestNutrientAdvice,
- "requestNutrientAdvice",
- pkg.calculatorVersion,
-)
+export async function getNutrientAdvice(
+ _fdm: FdmType,
+ inputs: NutrientAdviceInputs,
+): Promise<NutrientAdvice> {
+ return requestNutrientAdvice(inputs)
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In fdm-calculator/src/nutrient-advice/index.ts lines 1-102, the cached wrapper
persists the full inputs (including nmiApiKey) to the calculation cache causing
a credential leak; remove the withCalculationCache wrapper and instead export a
thin forwarder that directly calls requestNutrientAdvice (so nothing is
hashed/stored by the cache), i.e. replace the withCalculationCache export with a
direct assignment/export to requestNutrientAdvice; keep the rest of the function
intact and run tests to ensure callers still import getNutrientAdvice correctly.
| uniqueIndex("derogation_one_per_farm_per").on( | ||
| table.b_id_derogation, | ||
| ), |
There was a problem hiding this comment.
Remove the incorrect unique constraint on derogation applications
derogation_applying must allow many farms to reference the same b_id_derogation (one record per year). The new uniqueIndex("derogation_one_per_farm_per").on(table.b_id_derogation) forbids that—after the first farm links to a given derogation, every subsequent insert for the same year will violate the constraint. The existing primary key on (b_id_farm, b_id_derogation) already enforces “one row per farm per derogation.” Please drop this unique index (or redesign the constraint around the intended columns).
- uniqueIndex("derogation_one_per_farm_per").on(
- table.b_id_derogation,
- ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uniqueIndex("derogation_one_per_farm_per").on( | |
| table.b_id_derogation, | |
| ), |
🤖 Prompt for AI Agents
In fdm-core/src/db/schema.ts around lines 802–804, the added unique index
uniqueIndex("derogation_one_per_farm_per").on(table.b_id_derogation) is
incorrect because it prevents multiple farms from referencing the same
b_id_derogation; remove that uniqueIndex declaration (or replace it with a
constraint that includes both b_id_farm and b_id_derogation if a uniqueness rule
across those columns is intended) so the existing primary key (b_id_farm,
b_id_derogation) remains the enforcement for one row per farm per derogation.
| expect(certifications.length).toBe(2) | ||
| expect(certifications[0].b_organic_traces).toBe(traces1) | ||
| expect(certifications[1].b_organic_traces).toBe(traces2) | ||
| }) |
There was a problem hiding this comment.
Don’t rely on implicit SQL ordering.
listOrganicCertifications doesn’t guarantee row order, so asserting on certifications[0]/[1] will intermittently fail depending on the DB planner. Please sort the results (or assert via unordered matching) before making positional expectations.
Apply this diff to keep the assertion stable:
- expect(certifications[0].b_organic_traces).toBe(traces1)
- expect(certifications[1].b_organic_traces).toBe(traces2)
+ const traces = certifications
+ .map((cert) => cert.b_organic_traces)
+ .sort()
+ expect(traces).toEqual([traces1, traces2].sort())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(certifications.length).toBe(2) | |
| expect(certifications[0].b_organic_traces).toBe(traces1) | |
| expect(certifications[1].b_organic_traces).toBe(traces2) | |
| }) | |
| expect(certifications.length).toBe(2) | |
| const traces = certifications | |
| .map((cert) => cert.b_organic_traces) | |
| .sort() | |
| expect(traces).toEqual([traces1, traces2].sort()) | |
| }) |
🤖 Prompt for AI Agents
In fdm-core/src/organic.test.ts around lines 182 to 185, the test is making
positional assertions (certifications[0]/[1]) but listOrganicCertifications does
not guarantee order; update the test to make it deterministic by either sorting
the certifications array by a stable key (e.g., id or b_organic_traces) before
asserting the elements, or change the assertions to unordered matching (e.g.,
expect(certifications).toEqual(expect.arrayContaining([expect.objectContaining({
b_organic_traces: traces1 }), expect.objectContaining({ b_organic_traces:
traces2 })])) so the test no longer depends on SQL row ordering.
| return await fdm.transaction(async (tx: FdmType) => { | ||
| await checkPermission( | ||
| tx, | ||
| "farm", | ||
| "read", | ||
| b_id_farm, | ||
| principal_id, | ||
| "isOrganicCertificationValid", | ||
| ) | ||
|
|
||
| const result = await tx | ||
| .select({ id: schema.organicCertifications.b_id_organic }) | ||
| .from(schema.organicCertifications) | ||
| .leftJoin( | ||
| schema.organicCertificationsHolding, | ||
| eq( | ||
| schema.organicCertifications.b_id_organic, | ||
| schema.organicCertificationsHolding.b_id_organic, | ||
| ), | ||
| ) | ||
| .where( | ||
| and( | ||
| eq( | ||
| schema.organicCertificationsHolding.b_id_farm, | ||
| b_id_farm, | ||
| ), | ||
| lte( | ||
| schema.organicCertifications.b_organic_issued, | ||
| date, | ||
| ), | ||
| gte( | ||
| schema.organicCertifications.b_organic_expires, | ||
| date, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Handle open-ended certification dates in validity checks
b_organic_issued and b_organic_expires are nullable in the schema, and addOrganicCertification doesn’t prohibit null. The predicates here demand non-null bounds via <=/>=, so any certificate with an open start or end date is silently treated as invalid, even though the data model allows it. Wrap both comparisons in or(isNull(...), comparison) so open intervals remain valid.
-import { and, eq, gte, inArray, lte } from "drizzle-orm"
+import { and, eq, gte, inArray, isNull, lte, or } from "drizzle-orm"
…
- lte(
- schema.organicCertifications.b_organic_issued,
- date,
- ),
- gte(
- schema.organicCertifications.b_organic_expires,
- date,
- ),
+ or(
+ isNull(schema.organicCertifications.b_organic_issued),
+ lte(
+ schema.organicCertifications.b_organic_issued,
+ date,
+ ),
+ ),
+ or(
+ isNull(schema.organicCertifications.b_organic_expires),
+ gte(
+ schema.organicCertifications.b_organic_expires,
+ date,
+ ),
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return await fdm.transaction(async (tx: FdmType) => { | |
| await checkPermission( | |
| tx, | |
| "farm", | |
| "read", | |
| b_id_farm, | |
| principal_id, | |
| "isOrganicCertificationValid", | |
| ) | |
| const result = await tx | |
| .select({ id: schema.organicCertifications.b_id_organic }) | |
| .from(schema.organicCertifications) | |
| .leftJoin( | |
| schema.organicCertificationsHolding, | |
| eq( | |
| schema.organicCertifications.b_id_organic, | |
| schema.organicCertificationsHolding.b_id_organic, | |
| ), | |
| ) | |
| .where( | |
| and( | |
| eq( | |
| schema.organicCertificationsHolding.b_id_farm, | |
| b_id_farm, | |
| ), | |
| lte( | |
| schema.organicCertifications.b_organic_issued, | |
| date, | |
| ), | |
| gte( | |
| schema.organicCertifications.b_organic_expires, | |
| date, | |
| ), | |
| ), | |
| return await fdm.transaction(async (tx: FdmType) => { | |
| await checkPermission( | |
| tx, | |
| "farm", | |
| "read", | |
| b_id_farm, | |
| principal_id, | |
| "isOrganicCertificationValid", | |
| ) | |
| const result = await tx | |
| .select({ id: schema.organicCertifications.b_id_organic }) | |
| .from(schema.organicCertifications) | |
| .leftJoin( | |
| schema.organicCertificationsHolding, | |
| eq( | |
| schema.organicCertifications.b_id_organic, | |
| schema.organicCertificationsHolding.b_id_organic, | |
| ), | |
| ) | |
| .where( | |
| and( | |
| eq( | |
| schema.organicCertificationsHolding.b_id_farm, | |
| b_id_farm, | |
| ), | |
| or( | |
| isNull(schema.organicCertifications.b_organic_issued), | |
| lte( | |
| schema.organicCertifications.b_organic_issued, | |
| date, | |
| ), | |
| ), | |
| or( | |
| isNull(schema.organicCertifications.b_organic_expires), | |
| gte( | |
| schema.organicCertifications.b_organic_expires, | |
| date, | |
| ), | |
| ), | |
| ), |
🤖 Prompt for AI Agents
In fdm-core/src/organic.ts around lines 364 to 398, the validity check uses
lte(b_organic_issued, date) and gte(b_organic_expires, date) which treat null
start/end as invalid; update the where predicates to allow open-ended intervals
by wrapping each comparison in an or(isNull(column), comparison) — i.e., replace
lte(b_organic_issued, date) with or(isNull(b_organic_issued),
lte(b_organic_issued, date)) and replace gte(b_organic_expires, date) with
or(isNull(b_organic_expires), gte(b_organic_expires, date)) so null
issued/expires are considered valid bounds.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Chores