Add BLN3 measures to fdm database#606
Merged
Merged
Conversation
…tMeasuresCatalogue(catalogueName, nmiApiKey)` as a dispatcher (mirroring `getFertilizersCatalogue`), with BLN3 implemented in `measures/catalogues/bln.ts`. Adding future catalogues (e.g. ANLb) only requires a new file and extending the `CatalogueMeasureName` union. Also exports `hashMeasure` and the `CatalogueMeasure`, `CatalogueMeasureItem`, `CatalogueMeasureName` types using pandex naming conventions (`m_id`, `m_source`, `m_name`, etc.).
…sures_catalogue`, `measures`, and `measure_adopting` follow the action-asset model. Exports `addMeasure`, `getMeasure`, `getMeasures`, `getMeasuresForFarm`, `getMeasuresFromCatalogue`, `updateMeasure`, `removeMeasure`, `syncMeasuresCatalogueArray`, `enableMeasureCatalogue`, `disableMeasureCatalogue`, `isMeasureCatalogueEnabled`, `getEnabledMeasureCatalogues`, and the `Measure` / `MeasureCatalogue` types. `syncCatalogues` now accepts an optional `nmiApiKey` to populate the measures catalogue. All existing farms have the `bln` catalogue enabled by default via migration.
…s are included as well
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR implements BLN3 measures: adds DB schema and migration, Drizzle types, full measure CRUD and catalogue sync APIs, fdm-data BLN fetch + hashing, tests, app wiring to enable BLN on farm create, and NMI_API_KEY configuration. ChangesMeasures Feature Implementation
Sequence DiagramsequenceDiagram
participant User
participant App as fdm-app
participant Core as fdm-core
participant Data as fdm-data
participant API as NMI API
participant DB as Database
User->>App: Create farm
App->>Core: enableMeasureCatalogue(fdm, principal, farm,"bln")
Core->>DB: insert measure_catalogue_enabling
Migrate->>Core: syncCatalogues(fdm, {nmiApiKey})
Core->>Data: getMeasuresCatalogue("bln", nmiApiKey)
Data->>API: HTTP fetch BLN3 (Bearer token)
API-->>Data: JSON measures
Data->>Data: hashMeasure items
Data-->>Core: CatalogueMeasure[]
Core->>DB: upsert measures_catalogue
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
fdm-data/src/measures/hash.test.ts (1)
35-41: ⚡ Quick winAdd an immutability test for
hashMeasureinput.After fixing the mutation bug, add a regression test asserting the input object is unchanged after hashing (especially
hash).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@fdm-data/src/measures/hash.test.ts` around lines 35 - 41, Add a regression test in hash.test.ts that verifies hashMeasure does not mutate its input: call hashMeasure with an object whose hash property is null or set to a sentinel, capture a deep-cloned copy (or the original), then after awaiting hashMeasure assert the original object's properties (especially the hash field) are unchanged and equal to the pre-call copy; reference the existing test case that uses baseMeasure and the hashMeasure function so you extend that block to include these immutability assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@fdm-core/src/db/migrations/0028_spotty_greymalkin.sql`:
- Around line 39-43: The two new foreign keys that reference farms and fields
are created with ON DELETE NO ACTION, which will block deletions once
measure_catalogue_enabling rows are inserted; update the ALTER TABLE statements
that add constraints measure_adopting_b_id_fields_b_id_fk (on table
measure_adopting column b_id -> fdm.fields.b_id) and
measure_catalogue_enabling_b_id_farm_farms_b_id_farm_fk (on table
measure_catalogue_enabling column b_id_farm -> fdm.farms.b_id_farm) to use ON
DELETE CASCADE (keep ON UPDATE NO ACTION) so deleting a farm or field will
cascade and remove dependent rows instead of failing.
In `@fdm-core/src/db/schema.ts`:
- Around line 973-978: The foreign-key references for b_id and b_id_measure
currently default to NO ACTION; update the Drizzle schema so these references
include an ON DELETE CASCADE behavior (e.g., add onDelete: 'CASCADE' to the
references call for the b_id reference to fields.b_id and for the b_id_measure
reference to measures.b_id_measure), and make the same change for the other
occurrence around lines 994-996 to mirror the SQL migration and allow cascading
deletes of fields/measures/farms.
In `@fdm-core/src/field.ts`:
- Around line 659-676: The code deletes rows from schema.measures for every
measure linked to the removed field (measuresToDelete) but does not ensure those
measures are not still referenced by other rows in schema.measureAdopting;
update the logic in the same transaction (where the current deletes occur) to
only delete measures that have no remaining adopting rows: after deleting from
schema.measureAdopting for the target b_id, compute the set of candidate
measureIds (from measuresToDelete) and issue a delete on schema.measures
restricted to those ids and where NOT EXISTS (or equivalent) a row in
schema.measureAdopting with the same b_id_measure — i.e., only remove measures
with zero remaining references. Use the existing symbols (measuresToDelete,
measureIds, tx, schema.measureAdopting, schema.measures) so the change is
localized.
In `@fdm-core/src/measure.ts`:
- Around line 33-40: Validate and reject inverted date ranges on both write
paths: in addMeasure ensure that when m_end is provided it is not earlier than
m_start (compare m_end.getTime() >= m_start.getTime()) and throw or return a
rejected error if it is; apply the same check to the update measure routine in
this file (the update function covering lines ~342-378) so any update that sets
m_end earlier than m_start is rejected before persisting.
- Around line 33-61: The addMeasure function currently only checks field write
permission and FKs, so callers can still add measures from a catalogue that was
disabled for the farm; before inserting into schema.measures and
schema.measureAdopting you must verify the catalogue is enabled for this b_id
and m_id (the same logic used by disableMeasureCatalogue). Add a check in
addMeasure (e.g., call or inline the enablement query used by
disableMeasureCatalogue) that queries the measures_catalogue/enablement table
for the (b_id, m_id) pair and throw/return an error if the catalogue is
disabled, then proceed with creating b_id_measure and the transaction only when
enabled.
In `@fdm-data/src/measures/catalogues/bln.ts`:
- Around line 25-27: The fetch call in bln.ts that requests
"https://api.nmi-agro.nl/maatwerk/bln3/measures" has no timeout and can block
startup; update the call in the function that performs the catalogue sync (the
code path invoked by syncCatalogues) to use AbortSignal.timeout(...) to create a
signal (e.g. const signal = AbortSignal.timeout(TIMEOUT_MS)) and pass that
signal into fetch options, and ensure you handle the abort by catching the
thrown DOMException/AbortError and logging or throwing a clear timeout error
instead of letting the process hang; keep the existing Authorization header when
adding the signal.
- Around line 33-34: Guard access to json.data before calling .map: after const
json = await res.json(), check that json && Array.isArray(json.data) and if not
either throw a descriptive Error (including response status/URL/context) or
return an empty array; update the code around the json.data.map(...) expression
(the mapping that converts BLN3ApiMeasure items) so it only runs when json.data
is an array and otherwise provides a clear error message referencing the
response and json shape.
In `@fdm-data/src/measures/hash.ts`:
- Around line 4-27: hashMeasure currently mutates the incoming
CatalogueMeasureItem by setting measure.hash = null; instead make hashing pure
by creating a shallow copy (e.g. copy = { ...measure }) and set copy.hash = null
before filtering/sorting, then use that copy in the Object.entries/filter and
when building sortedMeasure; update references in the function (measure → copy)
so the original caller object is not mutated while preserving the same
filtering, sorting, and h32ToString(JSON.stringify(...)) behavior.
---
Nitpick comments:
In `@fdm-data/src/measures/hash.test.ts`:
- Around line 35-41: Add a regression test in hash.test.ts that verifies
hashMeasure does not mutate its input: call hashMeasure with an object whose
hash property is null or set to a sentinel, capture a deep-cloned copy (or the
original), then after awaiting hashMeasure assert the original object's
properties (especially the hash field) are unchanged and equal to the pre-call
copy; reference the existing test case that uses baseMeasure and the hashMeasure
function so you extend that block to include these immutability assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2636e93c-073a-45ab-91c8-4dae6afa8418
📒 Files selected for processing (25)
.changeset/few-boxes-work.md.changeset/hip-oranges-nail.md.changeset/social-news-like.mddocker-compose.ymlfdm-app/.env.examplefdm-app/app/lib/fdm-migrate.server.jsfdm-app/app/routes/farm.create._index.tsxfdm-core/src/catalogues.test.tsfdm-core/src/catalogues.tsfdm-core/src/db/migrations/0028_spotty_greymalkin.sqlfdm-core/src/db/migrations/meta/0028_snapshot.jsonfdm-core/src/db/migrations/meta/_journal.jsonfdm-core/src/db/schema.tsfdm-core/src/farm.tsfdm-core/src/field.tsfdm-core/src/index.tsfdm-core/src/measure.test.tsfdm-core/src/measure.tsfdm-core/src/measure.types.tsfdm-data/src/index.tsfdm-data/src/measures/catalogues/bln.tsfdm-data/src/measures/d.tsfdm-data/src/measures/hash.test.tsfdm-data/src/measures/hash.tsfdm-data/src/measures/index.ts
gerardhros
reviewed
May 11, 2026
gerardhros
reviewed
May 11, 2026
gerardhros
reviewed
May 11, 2026
gerardhros
reviewed
May 11, 2026
| await fdm.transaction(async (tx) => { | ||
| await tx | ||
| .delete(schema.measureAdopting) | ||
| .where(eq(schema.measureAdopting.b_id_measure, b_id_measure)) |
Collaborator
There was a problem hiding this comment.
what happens if same measure is applied twice in a year with different time windows. is this where criteria then sufficient?
Collaborator
Author
There was a problem hiding this comment.
Good one, at 9df5dbf I added check for add and update measure to check whether the measure overlaps with same measure in the timeframe
gerardhros
approved these changes
May 11, 2026
Collaborator
gerardhros
left a comment
gerardhros
left a comment
There was a problem hiding this comment.
LGTM. See a few minor questions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests
Chores
Closes #573