Add balances to Organization#515
Merged
Merged
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds organization-level Nitrogen and Organic Matter balance pages, multi-farm input collection and batched per-field balance calculations, bulk catalogue APIs in fdm-core, sidebar/header nav for balances, a persisted organization-farm selection store, and new UI components for no-farms messaging and farm selection. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as fdm-app (UI)
participant Router as Route Loader
participant Calculator as fdm-calculator
participant Core as fdm-core
participant DB as Database
User->>App: Navigate to /organization/:slug/:calendar/balance/nitrogen
App->>Router: Invoke loader
Router->>Router: Validate params, parse farmIds
Router->>Core: Resolve organization by slug
Core->>DB: Query organizations
DB-->>Core: Organization
Router->>Core: Fetch accessible farms
Core->>DB: Query farms for org
DB-->>Core: Farm list
Router->>Calculator: collectInputForNitrogenBalanceForFarms(farmIds)
Calculator->>Core: getEnabledCultivationCataloguesForFarms(farmIds)
Core->>DB: Bulk query enabled cultivation catalogues (inArray)
DB-->>Core: Catalogue sources per farm
Calculator->>Core: getCultivationsFromCatalogues(dedupedIds)
Core->>DB: Query cultivations by catalogue IDs
DB-->>Core: Cultivation catalogue items
Calculator->>Core: getEnabledFertilizerCataloguesForFarms(farmIds)
Core->>DB: Bulk query enabled fertilizer catalogues (inArray)
DB-->>Core: Fertilizer sources per farm
Calculator->>Core: getFertilizersFromCatalogues(dedupedIds)
Core->>DB: Query fertilizers by catalogue IDs
DB-->>Core: Fertilizer catalogue items
Calculator->>Calculator: Flatten fields, batch calculate per-field (50)
Calculator-->>Router: Async per-farm results (with errors flagged)
Router-->>App: Resolve asyncData
App->>App: Render organization balance UI with metrics & farm list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
SvenVw
reviewed
Mar 18, 2026
| * @alpha | ||
| */ | ||
| export async function collectInputForNitrogenBalance( | ||
| export async function collectOnlyFieldInputForNitrogenBalance( |
Collaborator
There was a problem hiding this comment.
For consistency with collectInputForNitrogenBalanceForFarms rename it to collectInputForNitrogenBalanceForFarm
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Collaborator
Author
|
Maybe the farmIds should also be included in the Suspense key of the balance page components. Edit: Resolved here d53bc11 |
![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: 12
🧹 Nitpick comments (6)
fdm-core/src/cultivation.ts (1)
45-62: Short-circuit empty farm selections.The organization flow can legitimately end up with an empty subset. Returning
[]immediately makes that path explicit and avoids permission checks and query building for a known empty result.♻️ Suggested change
export async function getCultivationsOfFarmsFromCatalogue( fdm: FdmType, principal_id: PrincipalId, farmIds: schema.farmsTypeSelect["b_id_farm"][], ): Promise<CultivationCatalogue[]> { try { + if (farmIds.length === 0) { + return [] + } + await Promise.all( farmIds.map((b_id_farm) => checkPermission(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/cultivation.ts` around lines 45 - 62, In getCultivationsOfFarmsFromCatalogue, short-circuit when farmIds is empty by returning an empty array immediately before performing any permission checks or building queries; update the function (referencing getCultivationsOfFarmsFromCatalogue and the farmIds parameter) to check if farmIds.length === 0 (or equivalent) and return [] to avoid unnecessary work.fdm-core/src/cultivation.test.ts (1)
148-155: Assert the multi-farm behavior, not just that the call returned.This still passes if the implementation ignores every farm after the first one or accidentally returns
[]. Add a second farm with a distinct enabled catalogue and assert that catalogue is present in the result.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/cultivation.test.ts` around lines 148 - 155, The test currently only asserts that getCultivationsOfFarmsFromCatalogue returned a value; update the test for getCultivationsOfFarmsFromCatalogue in cultivation.test.ts to create a second farm (e.g., a new b_id_farm_2) with a distinct enabled catalogue, call getCultivationsOfFarmsFromCatalogue with both farm IDs, and assert that the returned cultivations include entries for both catalogues (or at least the distinct catalogue identifier for the second farm) to verify multi-farm behavior rather than a non-empty result.fdm-core/src/fertilizer.ts (1)
546-562: Error cause may be undefined.When remapping the error,
(err as Error).causecould be undefined if the original error doesn't have a cause property, which would passundefinedtohandleError.💡 Suggested defensive handling
if ( (err as Error)?.message?.startsWith( "Exception for getFertilizersOfFarms", ) ) { throw handleError( - (err as Error).cause, + (err as Error).cause ?? err, "Exception for getFertilizers", { b_id_farm, }, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/fertilizer.ts` around lines 546 - 562, The catch block remaps errors using (err as Error).cause which may be undefined; change it to derive a non-undefined cause (e.g. const cause = (err as any)?.cause ?? err) and pass that to handleError in the getFertilizersOfFarms error branch (reference handleError and the b_id_farm param) so handleError always receives a valid error/cause object.fdm-app/app/components/blocks/sidebar/organization-apps.tsx (1)
30-38: Ensurecalendaris never undefined in generated URLs.If both
params.calendarandstoredCalendarare undefined, the generated links would containundefinedin the URL path (e.g.,/organization/slug/undefined/balance/nitrogen). While the store initializescalendarto the current year, consider adding a defensive fallback.💡 Suggested defensive fallback
const storedCalendar = useCalendarStore((store) => store.calendar) -const calendar = params.calendar ?? storedCalendar +const calendar = params.calendar ?? storedCalendar ?? new Date().getFullYear().toString()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/components/blocks/sidebar/organization-apps.tsx` around lines 30 - 38, The generated URLs may contain "undefined" if both params.calendar and storedCalendar are undefined; update the assignment for calendar (used by nitrogenBalanceLink and omBalanceLink) to apply a defensive fallback (e.g., current year string) when params.calendar and storedCalendar are falsy, and then use that guaranteed string when building the links that reference params.slug, calendar, nitrogenBalanceLink and omBalanceLink so URLs never include "undefined".fdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsx (2)
342-343: Clarify comment about React 19usehook.The comment states "
useis not a React hook," but in React 19,useis a hook—it just has special semantics that allow conditional invocation (unlike other hooks). Consider clarifying this to avoid confusion.📝 Suggested comment update
-// `use` is not a React hook, therefore we can call it conditionally +// React 19's `use` hook has special semantics allowing conditional calls const asyncData = use(asyncDataPromise)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx around lines 342 - 343, Update the inline comment above the use(asyncDataPromise) call to correctly state that in React 19, use is indeed a React hook with special semantics that permit conditional invocation (unlike other hooks); replace the existing "use is not a React hook" phrasing with a succinct note that explains use is a hook in React 19 but is allowed to be called conditionally here (reference the asyncData = use(asyncDataPromise) line).
609-621: Array mutation and comparison could be cleaner.
farmIds.sort()mutates the original array and returns the same reference, sosortedPrevFarmIdsis just an alias. Using.find()to detect array differences is also non-idiomatic—.some()better expresses "does any element differ."♻️ Proposed cleaner implementation
-const sortedPrevFarmIds = - farmIds.sort() -selectedFarmIds.sort() -if ( - sortedPrevFarmIds.length !== - selectedFarmIds.length || - selectedFarmIds.find( - (selected_id, index) => - selected_id !== - sortedPrevFarmIds[ - index - ], - ) -) { +const sortedPrevFarmIds = [...farmIds].sort() +const sortedSelectedFarmIds = [...selectedFarmIds].sort() +const hasChanged = + sortedPrevFarmIds.length !== sortedSelectedFarmIds.length || + sortedSelectedFarmIds.some( + (id, index) => id !== sortedPrevFarmIds[index] + ) +if (hasChanged) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx around lines 609 - 621, Avoid mutating farmIds and use a clearer comparison: create sorted copies for both arrays (e.g., const sortedPrevFarmIds = [...farmIds].sort() and const sortedSelectedFarmIds = [...selectedFarmIds].sort()) and then check length and use .some((id, i) => id !== sortedPrevFarmIds[i]) (or .every) to detect differences instead of mutating with farmIds.sort() and using .find(); update the code paths that reference sortedPrevFarmIds and selectedFarmIds accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/components/blocks/header/organization.tsx`:
- Around line 43-50: The breadcrumb route check and generated balance URLs use a
wrong `.farms` segment; update the route id check in the farmBalanceRouteType
calculation (symbol: typesOfBalanceRoutes and farmBalanceRouteType) to use
`routes/organization.$slug.$calendar.balance.${type}._index` (remove `.farms`),
and update the three link generators that build balance URLs (they reference
selectedOrganizationSlug and params.calendar) to use
`/organization/${selectedOrganizationSlug}/${params.calendar}/balance/nitrogen`
and
`/organization/${selectedOrganizationSlug}/${params.calendar}/balance/organic-matter`
(remove `/farms/`) so the breadcrumb can activate and links point to the real
routes.
In `@fdm-app/app/lib/email.server.ts`:
- Around line 260-263: The sendEmail function was replaced with a noop that
writes sensitive HtmlBody to a shared "email.html" file; restore the real
transport by re-enabling the email client send path (use the original
client.sendEmail(email) call in sendEmail) and remove or guard the fs.writeFile
fallback behind an explicit dev-only flag (e.g., process.env.NODE_ENV ===
"development" or EMAIL_PREVIEW=true) so only local/dev runs write a preview to a
safe, non-shared location (or per-request file) and production continues to call
sendEmail on the real client; update references in sendEmail and remove
unconditional fs.writeFile usage.
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 545-548: DialogDescription currently states that selected farms
are "uitgesloten" (excluded) but the UI logic (the checkbox using
defaultChecked={!!currentValue} where currentValue comes from farmResults)
actually includes checked farms in the calculation; update the DialogDescription
text to reflect the actual behavior (e.g., say the selected farms are
included/inbegrepen/ingesloten) so the message matches the implementation,
referencing the DialogDescription component and the checkbox
defaultChecked/currentValue/farmResults flow.
- Around line 362-370: The conditional is using a truthiness check on
balanceResult.balance which treats 0 as falsy and shows CircleAlert; change the
check to explicitly detect null/undefined (e.g., balanceResult.balance == null
or typeof balanceResult.balance === "undefined") so that numeric 0 is handled by
the inner comparison and renders CircleCheck when balanceResult.balance <=
balanceResult.target; update the JSX around balanceResult.balance and the
CircleCheck/CircleX/CircleAlert branches accordingly.
- Around line 189-193: The loop over combinedResult.fields uses a dangerous
assertion: avoid casting fieldToFarmMap[result.b_id] as string; instead
defensively check that fieldToFarmMap has a value for the key before using it.
In the loop that computes b_id_farm and pushes into rawFarmResultsMap (symbols:
combinedResult.fields, fieldToFarmMap, b_id_farm, rawFarmResultsMap), skip or
log/collect any result whose b_id is missing from fieldToFarmMap and only push
when b_id_farm is a defined string to prevent grouping under "undefined" or
corrupting data.
- Around line 230-274: The try-catch is too late — it only wraps reportError and
the final return, so failures in calculateNitrogenBalancesFieldToFarm,
listPrincipalsForFarm, or getFields will escape; move the try-catch to wrap the
calls that can throw (the Promise.all that awaits
calculateNitrogenBalancesFieldToFarm, listPrincipalsForFarm, getFields and any
related awaits), then keep your existing error handling inside the catch: call
reportError (if desired) and return the same fallback nitrogenBalanceResult
object (hasErrors: true, errorMessage: ...) so the loader always returns the
expected shape (references: calculateNitrogenBalancesFieldToFarm,
listPrincipalsForFarm, getFields, reportError, nitrogenBalanceResult).
- Around line 561-574: The Checkbox inputs rendered inside the div lack
accessible labels; update the JSX around the Checkbox (component Checkbox) to
associate each input with its farm name by giving the Checkbox a unique id (use
b_id_farm or `farm.b_id_farm` to form the id) and either wrap the checkbox and
text in a <label> or render a <label htmlFor={id}>{farm.b_name_farm ??
'Onbekend'}</label>; ensure the Checkbox receives the id prop and keep existing
props (name={b_id_farm}, defaultChecked={!!currentValue}) so screen readers
announce the farm name and clicking the name toggles the checkbox.
In `@fdm-calculator/src/balance/nitrogen/input.test.ts`:
- Around line 513-518: The test creates duplicated fields as mockFieldsData2
(b_id values like "2-field-1"/"2-field-2") but mockDepositionSupplyMap only has
entries for "field-1" and "field-2", so lookups for the second farm return
undefined and break assertions; update mockDepositionSupplyMap to include
matching keys for the second-farm fields (e.g. "2-field-1" and "2-field-2") with
the same or appropriate deposition objects, or programmatically populate
deposition entries for each entry in mockFieldsData2 before assertions so that
the deposition lookup used by the test finds valid data for mockFieldsData2.
In `@fdm-calculator/src/balance/nitrogen/input.ts`:
- Around line 249-271: The JSDoc for collectInputForNitrogenBalance references
`@param` farmIds but the function signature uses b_id_farm; update the JSDoc to
use the exact parameter name b_id_farm (and adjust the description to indicate
it is a single farm ID string) so the docs match the function signature (check
the JSDoc block above collectInputForNitrogenBalance and replace farmIds with
b_id_farm and correct its type/description).
In `@fdm-core/src/fertilizer.test.ts`:
- Around line 432-438: The test currently asserts the opposite of the intended
behavior; update the assertion in the test for getFertilizers to verify that
when getFertilizersOfFarms throws, getFertilizers remaps the error message to
"Exception for getFertilizers" (i.e., assert that getFertilizers(fdm,
principal_id, invalidFarmId) rejects with or throws an error whose message
equals or matches "Exception for getFertilizers"). Reference the test invoking
getFertilizers and the underlying getFertilizersOfFarms remapping behavior to
locate where to change the expectation.
- Around line 64-74: The test creates a second farm (b_id_farm_2 via addFarm)
but never enables its fertilizer catalogue, so later steps that add fertilizers
to both farms may fail; update the setup to call enableFertilizerCatalogue for
the second farm as well (mirror the existing call enableFertilizerCatalogue(fdm,
principal_id, b_id_farm, b_id_farm) by invoking enableFertilizerCatalogue with
b_id_farm_2 for the same two business/farm id parameters), ensuring both
b_id_farm and b_id_farm_2 have catalogues enabled before the multi-farm test
runs.
In `@fdm-core/src/fertilizer.ts`:
- Around line 717-719: Remove the debug console.log that prints fertilizer
timing for a farm: delete the console.log(...) statement that references
b_id_farm, fertilizerStart and fertilizerEnd in the fertilizer-related code (the
snippet shown). If persistent debugging is needed, replace it with the
appropriate logger call (e.g., processLogger.debug or logger.debug) rather than
leaving a console.log in the production path.
---
Nitpick comments:
In `@fdm-app/app/components/blocks/sidebar/organization-apps.tsx`:
- Around line 30-38: The generated URLs may contain "undefined" if both
params.calendar and storedCalendar are undefined; update the assignment for
calendar (used by nitrogenBalanceLink and omBalanceLink) to apply a defensive
fallback (e.g., current year string) when params.calendar and storedCalendar are
falsy, and then use that guaranteed string when building the links that
reference params.slug, calendar, nitrogenBalanceLink and omBalanceLink so URLs
never include "undefined".
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 342-343: Update the inline comment above the use(asyncDataPromise)
call to correctly state that in React 19, use is indeed a React hook with
special semantics that permit conditional invocation (unlike other hooks);
replace the existing "use is not a React hook" phrasing with a succinct note
that explains use is a hook in React 19 but is allowed to be called
conditionally here (reference the asyncData = use(asyncDataPromise) line).
- Around line 609-621: Avoid mutating farmIds and use a clearer comparison:
create sorted copies for both arrays (e.g., const sortedPrevFarmIds =
[...farmIds].sort() and const sortedSelectedFarmIds =
[...selectedFarmIds].sort()) and then check length and use .some((id, i) => id
!== sortedPrevFarmIds[i]) (or .every) to detect differences instead of mutating
with farmIds.sort() and using .find(); update the code paths that reference
sortedPrevFarmIds and selectedFarmIds accordingly.
In `@fdm-core/src/cultivation.test.ts`:
- Around line 148-155: The test currently only asserts that
getCultivationsOfFarmsFromCatalogue returned a value; update the test for
getCultivationsOfFarmsFromCatalogue in cultivation.test.ts to create a second
farm (e.g., a new b_id_farm_2) with a distinct enabled catalogue, call
getCultivationsOfFarmsFromCatalogue with both farm IDs, and assert that the
returned cultivations include entries for both catalogues (or at least the
distinct catalogue identifier for the second farm) to verify multi-farm behavior
rather than a non-empty result.
In `@fdm-core/src/cultivation.ts`:
- Around line 45-62: In getCultivationsOfFarmsFromCatalogue, short-circuit when
farmIds is empty by returning an empty array immediately before performing any
permission checks or building queries; update the function (referencing
getCultivationsOfFarmsFromCatalogue and the farmIds parameter) to check if
farmIds.length === 0 (or equivalent) and return [] to avoid unnecessary work.
In `@fdm-core/src/fertilizer.ts`:
- Around line 546-562: The catch block remaps errors using (err as Error).cause
which may be undefined; change it to derive a non-undefined cause (e.g. const
cause = (err as any)?.cause ?? err) and pass that to handleError in the
getFertilizersOfFarms error branch (reference handleError and the b_id_farm
param) so handleError always receives a valid error/cause object.
🪄 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: 03a62baa-e262-4b9f-be94-3a03b74965d4
📒 Files selected for processing (18)
fdm-app/app/components/blocks/header/organization.tsxfdm-app/app/components/blocks/organization/no-farms-message.tsxfdm-app/app/components/blocks/sidebar/organization-apps.tsxfdm-app/app/lib/email.server.tsfdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsxfdm-app/app/routes/organization.$slug.$calendar.farms._index.tsxfdm-app/app/routes/organization.$slug._index.tsxfdm-app/app/routes/organization.tsxfdm-app/app/store/organization-farm-selection.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/balance/nitrogen/input.test.tsfdm-calculator/src/balance/nitrogen/input.tsfdm-calculator/src/index.tsfdm-core/src/cultivation.test.tsfdm-core/src/cultivation.tsfdm-core/src/fertilizer.test.tsfdm-core/src/fertilizer.tsfdm-core/src/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-calculator/src/balance/nitrogen/input.ts`:
- Around line 156-158: The code currently allows a provided b_id to be applied
across multiple farms (populating b_id_farm incorrectly); add a validation early
in the function that builds the NitrogenBalanceInput array (the function that
accepts b_id and returns Promise<NitrogenBalanceInput[]>), and if the input
indicates multiple farms will be processed (e.g., farms.length > 1 or a
multi-farm flag) and b_id is not undefined, immediately reject/throw a clear
Error stating "b_id cannot be provided when collecting inputs for multiple
farms"; apply the same validation in the other occurrence that handles b_id (the
block referenced around lines 185-195) so neither path ever attaches one field
ID to multiple b_id_farm results.
In `@fdm-calculator/src/balance/organic-matter/input.ts`:
- Around line 129-130: The code currently allows a single b_id to be passed when
farmIds contains multiple farms, causing one field ID to be reused across farms;
update the input validation to reject/throw when b_id is provided and farmIds
has more than one entry (check the b_id and farmIds variables where
organic-matter input is constructed—same places referenced around the b_id docs
and the later block at lines ~158-168), mirroring the nitrogen-path fix: detect
if (b_id && farmIds && farmIds.length > 1) and return/throw a clear validation
error (e.g., "b_id cannot be used when collecting input for multiple farms") and
add/update tests to cover this case.
In `@fdm-core/src/cultivation.ts`:
- Around line 157-164: The code is emitting undefined when a catalogue group is
missing; in the Object.fromEntries mapping using cataloguesByFarm and
enabledCatalogues, ensure the access catalogue[cat.b_lu_source] defaults to an
empty array so flatMap never yields undefined (e.g., replace occurrences of
catalogue[cat.b_lu_source] with a default like catalogue[cat.b_lu_source] ?? []
or similar) so downstream readers of cultivation.b_lu_catalogue always see an
array.
In `@fdm-core/src/fertilizer.ts`:
- Around line 65-72: The catch blocks in getFertilizersFromCatalogue (and the
second identical catch) are comparing err.message to the wrong helper exception
string ("Exception for getFertilizersFromCatalogueForFarms") so the unwrap
branch never runs and errors get double-wrapped; change the comparison to match
the helper's actual thrown message ("Exception for
getFertilizersFromCatalogues") or, better, replace the literal with the shared
constant (as used in cultivation.ts) and reuse it in both catch blocks so
handleError receives the original cause when appropriate.
🪄 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: 3bcabbf4-81fc-4ee9-aec3-418e54af92a9
📒 Files selected for processing (11)
fdm-calculator/src/balance/nitrogen/input.test.tsfdm-calculator/src/balance/nitrogen/input.tsfdm-calculator/src/balance/organic-matter/input.test.tsfdm-calculator/src/balance/organic-matter/input.tsfdm-core/src/bulk.tsfdm-core/src/cultivation.test.tsfdm-core/src/cultivation.tsfdm-core/src/fertilizer.d.tsfdm-core/src/fertilizer.test.tsfdm-core/src/fertilizer.tsfdm-core/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- fdm-core/src/bulk.ts
- fdm-calculator/src/balance/organic-matter/input.test.ts
- fdm-core/src/fertilizer.test.ts
- fdm-core/src/cultivation.test.ts
- fdm-core/src/fertilizer.d.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
fdm-calculator/src/balance/organic-matter/input.ts (1)
53-67:⚠️ Potential issue | 🟠 Major
b_idcan still attach the wrong field tob_id_farm.The
b_idbranch fetches a field by ID only, while the fallback branch enforcesb_id_farm+timeframescoping. If a caller passes a field from another accessible farm,getFieldwill return it with its actual farm ID, causing the multi-farm wrapper to tag it with the wrongb_id_farm.🛠️ Proposed fix
if (b_id) { - const field = await getField(tx, principal_id, b_id) + const field = ( + await getFields(tx, principal_id, b_id_farm, timeframe) + ).find((candidate) => candidate.b_id === b_id) if (!field) { - throw new Error(`Field not found: ${String(b_id)}`) + throw new Error( + `Field ${String(b_id)} not found in farm ${String( + b_id_farm, + )}`, + ) } farmFields = [field]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-calculator/src/balance/organic-matter/input.ts` around lines 53 - 67, The b_id branch uses getField(tx, principal_id, b_id) which can return a field from a different farm and then assigns it to farmFields, causing mis-tagging under b_id_farm; update the logic in the b_id branch (around getField, farmFields, b_id_farm) to enforce farm scoping by either calling a farm-scoped fetch (e.g., getField with b_id_farm) or validating the returned field's farm identifier matches b_id_farm and throw an error (or skip) if it does not; ensure principal_id, tx and timeframe handling remain unchanged and that getFields behavior is preserved for the fallback path.fdm-calculator/src/balance/nitrogen/input.ts (1)
53-65:⚠️ Potential issue | 🟠 Major
b_idbypasses farm-scoping and timeframe filtering.When
b_idis present, lines 54–58 fetch the field usinggetField()which performs field-level authorization only, without verifying that the field belongs to the requestedb_id_farmor respecting the timeframe constraint. This allows a field from another accessible farm to be included in the results and labeled as belonging tob_id_farm(line 220), causing the returned data to be incorrectly associated with the wrong farm.🛠️ Proposed fix
if (b_id) { - const field = await getField(tx, principal_id, b_id) + const field = ( + await getFields(tx, principal_id, b_id_farm, timeframe) + ).find((candidate) => candidate.b_id === b_id) if (!field) { - throw new Error(`Field not found: ${String(b_id)}`) + throw new Error( + `Field ${String(b_id)} not found in farm ${String( + b_id_farm, + )}`, + ) } farmFields = [field]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-calculator/src/balance/nitrogen/input.ts` around lines 53 - 65, When b_id is provided the code calls getField(tx, principal_id, b_id) which skips farm and timeframe filtering; instead ensure the returned farmFields are scoped to b_id_farm and respect timeframe by either (a) using getFields(tx, principal_id, b_id_farm, timeframe) and filtering the result for the matching b_id, or (b) after getField(...) validate the field's farm identifier matches b_id_farm and that the field falls within the requested timeframe before assigning farmFields = [field]; update the branch handling b_id to perform this validation so farmFields cannot include a field from another farm or outside the timeframe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-calculator/src/balance/nitrogen/input.ts`:
- Around line 190-245: Replace the current Promise.all farm-processing block
with Promise.allSettled so a failing farm doesn't reject the whole organization;
for each mapped promise (the async fn that calls
collectInputForNitrogenBalanceForFarm) convert the settled result into the
existing success shape ({ b_id_farm, fields, fertilizerDetails,
cultivationDetails, timeFrame }) on fulfilled or into a consistent error object
on rejected by calling handleNitrogenBalanceInputCollectionError(error,
b_id_farm) (i.e., preserve b_id_farm and timeframe and either the populated
fields/fertilizerDetails/cultivationDetails or the error wrapper), keeping the
same references to collectInputForNitrogenBalanceForFarm,
handleNitrogenBalanceInputCollectionError, b_id_farm, fertilizerDetails and
cultivationDetails so downstream code can aggregate successes and per-farm
failures.
In `@fdm-core/src/cultivation.test.ts`:
- Around line 248-253: The test calls the async function
enableCultivationCatalogue without awaiting it, causing a race where the
subsequent insert/read may see an empty object; update the test to await
enableCultivationCatalogue(principal_id, b_id_farm, "invalid-catalogue") (the
exact call shown) so the catalogue enablement completes before asserting on the
resulting state or performing the insert operation.
In `@fdm-core/src/cultivation.ts`:
- Around line 146-166: The result currently only iterates
Object.entries(cataloguesByFarm) so any requested farm that has zero
enabledCatalogues is omitted; change the final Object.fromEntries construction
to iterate the full set of requested farm IDs (derive the unique ids from the
original input/arguments or from enabledCatalogues.map(c => c.b_id_farm) if a
separate request list isn't available) and for each id return an array (empty
when cataloguesByFarm[id] is undefined); i.e., when building the map use the
full set of requested farm ids and populate each entry with
enabledCatalogues.flatMap(cat => catalogue[cat.b_lu_source] ?? []) or [] when
there are no enabled catalogues so requested-but-empty farms appear as []
instead of being dropped.
In `@fdm-core/src/fertilizer.ts`:
- Around line 137-157: The current mapping uses
Object.entries(enabledCataloguesByFarm) which drops farms that have zero enabled
catalog rows; change the return to iterate over the full set of requested farm
IDs (the input list of farms used to call getFertilizersFromCatalogues / build
enabledCataloguesByFarm) and for each farm id return an array (empty when there
are no enabled catalogues). Specifically, after computing
enabledCataloguesByFarm and catalogues (from getFertilizersFromCatalogues),
produce the result by iterating over the canonical list of farm IDs (e.g.,
requestedFarmIds or the original farm id collection passed into this function)
and for each b_id_farm build the array via
enabledCataloguesByFarm[b_id_farm]?.flatMap(({ p_source }) =>
catalogues[p_source] ?? []) ?? [] so farms with no enabledCatalogues are
included as empty arrays.
---
Outside diff comments:
In `@fdm-calculator/src/balance/nitrogen/input.ts`:
- Around line 53-65: When b_id is provided the code calls getField(tx,
principal_id, b_id) which skips farm and timeframe filtering; instead ensure the
returned farmFields are scoped to b_id_farm and respect timeframe by either (a)
using getFields(tx, principal_id, b_id_farm, timeframe) and filtering the result
for the matching b_id, or (b) after getField(...) validate the field's farm
identifier matches b_id_farm and that the field falls within the requested
timeframe before assigning farmFields = [field]; update the branch handling b_id
to perform this validation so farmFields cannot include a field from another
farm or outside the timeframe.
In `@fdm-calculator/src/balance/organic-matter/input.ts`:
- Around line 53-67: The b_id branch uses getField(tx, principal_id, b_id) which
can return a field from a different farm and then assigns it to farmFields,
causing mis-tagging under b_id_farm; update the logic in the b_id branch (around
getField, farmFields, b_id_farm) to enforce farm scoping by either calling a
farm-scoped fetch (e.g., getField with b_id_farm) or validating the returned
field's farm identifier matches b_id_farm and throw an error (or skip) if it
does not; ensure principal_id, tx and timeframe handling remain unchanged and
that getFields behavior is preserved for the fallback path.
🪄 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: 0426ab04-be14-49be-ae72-c39e26b59d60
📒 Files selected for processing (8)
fdm-calculator/src/balance/nitrogen/input.test.tsfdm-calculator/src/balance/nitrogen/input.tsfdm-calculator/src/balance/organic-matter/input.test.tsfdm-calculator/src/balance/organic-matter/input.tsfdm-core/src/cultivation.test.tsfdm-core/src/cultivation.tsfdm-core/src/fertilizer.test.tsfdm-core/src/fertilizer.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- fdm-core/src/fertilizer.test.ts
- fdm-calculator/src/balance/organic-matter/input.test.ts
…ultivationsFromCatalogueForFarms
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-core/src/fertilizer.ts (1)
669-771:⚠️ Potential issue | 🟠 MajorAdd
b_id_farmto theFertilizerinterface and restore the explicit return type.The function selects
b_id_farmfrom the database (line 686), but theFertilizerinterface does not include this field. Without an explicit return type annotation, the implicit type tracks the current query shape, leaking implementation details into the public API surface. Either addb_id_farmtoFertilizeras an intentional field and annotate the return type asPromise<Fertilizer[]>, or removeb_id_farmfrom the selection if it's not needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/fertilizer.ts` around lines 669 - 771, The DB query in getFertilizers selects b_id_farm but the public Fertilizer type doesn't include it and the function lacks an explicit return type; either add b_id_farm to the Fertilizer interface and annotate getFertilizers to return Promise<Fertilizer[]> (so callers see the intended field), or remove b_id_farm from the select list if it isn't part of the public shape—update the Fertilizer interface and the getFertilizers signature (and keep deriveFertilizerType usage intact).
🧹 Nitpick comments (2)
fdm-core/src/fertilizer.test.ts (1)
379-423: Trim the unused acquisition setup.
getFertilizersFromCatalogueForFarms()only reads catalogue/enabling tables, so theaddFertilizer()calls here do not affect the assertion. Removing them would make this test cheaper and easier to follow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/fertilizer.test.ts` around lines 379 - 423, The test currently calls addFertilizer via the helper addTestFertilizer but getFertilizersFromCatalogueForFarms only reads catalogue/enabling data, so remove the unnecessary acquisition setup: delete the addTestFertilizer helper usage and any addFertilizer calls, leaving only the catalogue population calls (addFertilizerToCatalogue / makeFertilizer) for farm_1_fert_1, farm_2_fert_1, farm_1_fert_2, and farm_2_fert_2 so the test exercises getFertilizersFromCatalogueForFarms without extra side effects from addFertilizer.fdm-core/src/cultivation.test.ts (1)
240-294: Consider renaming shadowed variables for clarity.The
b_id_farmvariable declared on lines 241 and 272 shadows the outer scope variable. While functional, this can be confusing. Consider using distinct names likeb_id_farm_empty_catalogueorb_id_farm_no_catalogue.♻️ Suggested improvement
it("should handle empty catalogues", async () => { - const b_id_farm = await addFarm( + const b_id_farm_empty = await addFarm( fdm, principal_id, "Test Farm No Cultivations In Catalogue", undefined, undefined, undefined, ) await enableCultivationCatalogue( fdm, principal_id, - b_id_farm, + b_id_farm_empty, "invalid-catalogue", ) expect( await getCultivationsFromCatalogue( fdm, principal_id, - b_id_farm, + b_id_farm_empty, ), ).toEqual([]) expect( await getCultivationsFromCatalogueForFarms(fdm, principal_id, [ - b_id_farm, + b_id_farm_empty, ]), ).toEqual({ - [b_id_farm]: [], + [b_id_farm_empty]: [], }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/cultivation.test.ts` around lines 240 - 294, The tests "should handle empty catalogues" and "should handle no enabled catalogues" declare a variable named b_id_farm that shadows an outer variable; rename these local variables (e.g., b_id_farm_empty_catalogue and b_id_farm_no_catalogue) to avoid shadowing and improve clarity where addFarm is called and later passed into enableCultivationCatalogue, getCultivationsFromCatalogue, and getCultivationsFromCatalogueForFarms so update all uses in those two it blocks accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 110-153: The loader currently trusts searchParamFarmIds and passes
them on; validate that any provided farmIds are contained in the organization's
accessible farms returned by getFarms(...) (compare searchParamFarmIds against
farms.map(f => f.b_id_farm)); if any requested id is unknown or unauthorized,
throw a 400 similar to the existing farmIds validation (use data("invalid:
farmIds", { status: 400, statusText: "invalid: farmIds" })), otherwise set
farmIds = searchParamFarmIds; do this before calling
collectInputForNitrogenBalanceForFarms(...) so bad URLs are rejected early.
- Around line 212-280: The loader is building farmResults for every
farmsExtended entry which causes unselected farms to produce fake error rows
when rawFarmResultsMap[farm.b_id_farm] is undefined; fix by filtering
farmsExtended before Promise.all to only include selected farms (use the
existing farmIds if present) or at least only those with
rawFarmResultsMap[farm.b_id_farm] defined. Update the code around
farmsExtended/map (symbols: farmsExtended, farmIds, rawFarmResultsMap,
farmResults, calculateNitrogenBalancesFieldToFarm) so you call Promise.all on
farmsExtended.filter(f => farmIds?.includes(f.b_id_farm) ||
rawFarmResultsMap[f.b_id_farm]) and then proceed to compute
calculateNitrogenBalancesFieldToFarm and listPrincipalsForFarm as before,
preventing the catch block from being triggered for unselected farms.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 415-417: The CardTitle currently reads "Balans (Bedrijf)" which
implies a single farm; update the selection-level label shown in CardTitle
inside organization.$slug.$calendar.balance.organic-matter._index.tsx to reflect
the aggregated organization selection (for example "Balans (Selectie)" or
"Balans (Organisatie)"), and if your app uses i18n replace the hardcoded string
with the appropriate translation key instead of the literal; locate the
CardTitle component instance to make this change.
- Around line 109-152: The code accepts query farmIds directly
(searchParamFarmIds → farmIds) without checking they belong to the reachable
farms; before calling collectInputForOrganicMatterBalanceForFarms(...) validate
searchParamFarmIds (if present) against the accessible farms list (farms.map(f
=> f.b_id_farm)) and if any id is missing throw data("invalid: farmIds", {
status: 400, statusText: "invalid: farmIds" }); otherwise proceed using the
validated farmIds variable (searchParamFarmIds or farms.map(...)).
In `@fdm-core/src/cultivation.test.ts`:
- Around line 296-348: Update the two test descriptions to reference the
cultivation functions rather than fertilizers: change the first it(...) title
that currently reads "(getCultivationsFromCatalogue) should rename the error if
getFertilizersFromCatalogues throws an error" to mention
getCultivationsFromCatalogue (matching the tested function), and change the
second it(...) title that mentions getFertilizersFromCatalogues to reference
getCultivationsFromCatalogueForFarms (matching
getCultivationsFromCatalogueForFarms). Keep the rest of the test logic and
assertions (functions: getCultivationsFromCatalogue,
getCultivationsFromCatalogueForFarms, mockFdmThatThrowsOnSelectFrom) unchanged.
---
Outside diff comments:
In `@fdm-core/src/fertilizer.ts`:
- Around line 669-771: The DB query in getFertilizers selects b_id_farm but the
public Fertilizer type doesn't include it and the function lacks an explicit
return type; either add b_id_farm to the Fertilizer interface and annotate
getFertilizers to return Promise<Fertilizer[]> (so callers see the intended
field), or remove b_id_farm from the select list if it isn't part of the public
shape—update the Fertilizer interface and the getFertilizers signature (and keep
deriveFertilizerType usage intact).
---
Nitpick comments:
In `@fdm-core/src/cultivation.test.ts`:
- Around line 240-294: The tests "should handle empty catalogues" and "should
handle no enabled catalogues" declare a variable named b_id_farm that shadows an
outer variable; rename these local variables (e.g., b_id_farm_empty_catalogue
and b_id_farm_no_catalogue) to avoid shadowing and improve clarity where addFarm
is called and later passed into enableCultivationCatalogue,
getCultivationsFromCatalogue, and getCultivationsFromCatalogueForFarms so update
all uses in those two it blocks accordingly.
In `@fdm-core/src/fertilizer.test.ts`:
- Around line 379-423: The test currently calls addFertilizer via the helper
addTestFertilizer but getFertilizersFromCatalogueForFarms only reads
catalogue/enabling data, so remove the unnecessary acquisition setup: delete the
addTestFertilizer helper usage and any addFertilizer calls, leaving only the
catalogue population calls (addFertilizerToCatalogue / makeFertilizer) for
farm_1_fert_1, farm_2_fert_1, farm_1_fert_2, and farm_2_fert_2 so the test
exercises getFertilizersFromCatalogueForFarms without extra side effects from
addFertilizer.
🪄 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: b9e3865e-90bc-4c8a-a15e-bfb57dea85ea
📒 Files selected for processing (7)
fdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsxfdm-core/src/cultivation.test.tsfdm-core/src/cultivation.tsfdm-core/src/fertilizer.test.tsfdm-core/src/fertilizer.tsfdm-core/src/test-util.ts
Comment on lines
+110
to
+153
| let searchParamFarmIds: string[] | undefined | ||
| if (url.searchParams.has("farmIds")) { | ||
| searchParamFarmIds = url.searchParams | ||
| .get("farmIds") | ||
| ?.split(",") | ||
| .filter(Boolean) | ||
| if (!searchParamFarmIds || searchParamFarmIds.length === 0) { | ||
| throw data("invalid: farmIds", { | ||
| status: 400, | ||
| statusText: "invalid: farmIds", | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Get timeframe from calendar store | ||
| const timeframe = getTimeframe(params) | ||
|
|
||
| // Get the user's session too (for error reporting) | ||
| const session = await getSession(request) | ||
|
|
||
| const allOrganizations = await auth.api.listOrganizations({ | ||
| headers: request.headers, | ||
| }) | ||
| const organization = allOrganizations.find((org) => org.slug === slug) | ||
| if (!organization) { | ||
| throw data(`not found: ${slug}`, { | ||
| status: 404, | ||
| statusText: `not found: ${slug}`, | ||
| }) | ||
| } | ||
|
|
||
| const farms = await getFarms(fdm, organization.id) | ||
|
|
||
| // If the organization has no access to any farms, render the empty message | ||
| if (farms.length === 0) { | ||
| return { | ||
| farmIds: [], | ||
| organization: organization, | ||
| noFarms: true, | ||
| } | ||
| } | ||
|
|
||
| const farmIds = | ||
| searchParamFarmIds ?? farms.map((farm) => farm.b_id_farm) |
Contributor
There was a problem hiding this comment.
Reject farmIds that are outside the organization's accessible farm set.
The loader accepts the query selection verbatim. A request with an unknown or unauthorized farm ID reaches collectInputForNitrogenBalanceForFarms(...), which turns a bad URL into a loader failure instead of a clean 400. Validate the requested IDs against getFarms(...) before starting the calculation.
Suggested fix
const farms = await getFarms(fdm, organization.id)
+ const accessibleFarmIds = new Set(
+ farms.map((farm) => farm.b_id_farm),
+ )
// If the organization has no access to any farms, render the empty message
if (farms.length === 0) {
return {
@@
- const farmIds =
- searchParamFarmIds ?? farms.map((farm) => farm.b_id_farm)
+ const farmIds = searchParamFarmIds
+ ? [...new Set(searchParamFarmIds)]
+ : [...accessibleFarmIds]
+
+ if (farmIds.some((farmId) => !accessibleFarmIds.has(farmId))) {
+ throw data("invalid: farmIds", {
+ status: 400,
+ statusText: "invalid: farmIds",
+ })
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx
around lines 110 - 153, The loader currently trusts searchParamFarmIds and
passes them on; validate that any provided farmIds are contained in the
organization's accessible farms returned by getFarms(...) (compare
searchParamFarmIds against farms.map(f => f.b_id_farm)); if any requested id is
unknown or unauthorized, throw a 400 similar to the existing farmIds validation
(use data("invalid: farmIds", { status: 400, statusText: "invalid: farmIds" })),
otherwise set farmIds = searchParamFarmIds; do this before calling
collectInputForNitrogenBalanceForFarms(...) so bad URLs are rejected early.
Comment on lines
+109
to
+152
| let searchParamFarmIds: string[] | undefined | ||
| if (url.searchParams.has("farmIds")) { | ||
| searchParamFarmIds = url.searchParams | ||
| .get("farmIds") | ||
| ?.split(",") | ||
| .filter(Boolean) | ||
| if (!searchParamFarmIds || searchParamFarmIds.length === 0) { | ||
| throw data("invalid: farmIds", { | ||
| status: 400, | ||
| statusText: "invalid: farmIds", | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Get timeframe from calendar store | ||
| const timeframe = getTimeframe(params) | ||
|
|
||
| // Get the user's session too (for error reporting) | ||
| const session = await getSession(request) | ||
|
|
||
| const allOrganizations = await auth.api.listOrganizations({ | ||
| headers: request.headers, | ||
| }) | ||
| const organization = allOrganizations.find((org) => org.slug === slug) | ||
| if (!organization) { | ||
| throw data(`not found: ${slug}`, { | ||
| status: 404, | ||
| statusText: `not found: ${slug}`, | ||
| }) | ||
| } | ||
|
|
||
| const farms = await getFarms(fdm, organization.id) | ||
|
|
||
| // If the organization has no access to any farms, render the empty message | ||
| if (farms.length === 0) { | ||
| return { | ||
| organization: organization, | ||
| noFarms: true, | ||
| farmIds: [], | ||
| } | ||
| } | ||
|
|
||
| const farmIds = | ||
| searchParamFarmIds ?? farms.map((farm) => farm.b_id_farm) |
Contributor
There was a problem hiding this comment.
Validate farmIds against the accessible farm list before calling the batched OM collector.
This route has the same query-param gap as the nitrogen page: a stale or tampered farmIds value is passed straight into collectInputForOrganicMatterBalanceForFarms(...). That should be rejected up front as 400, not left to fail deeper in the loader.
Suggested fix
const farms = await getFarms(fdm, organization.id)
+ const accessibleFarmIds = new Set(
+ farms.map((farm) => farm.b_id_farm),
+ )
// If the organization has no access to any farms, render the empty message
if (farms.length === 0) {
return {
@@
- const farmIds =
- searchParamFarmIds ?? farms.map((farm) => farm.b_id_farm)
+ const farmIds = searchParamFarmIds
+ ? [...new Set(searchParamFarmIds)]
+ : [...accessibleFarmIds]
+
+ if (farmIds.some((farmId) => !accessibleFarmIds.has(farmId))) {
+ throw data("invalid: farmIds", {
+ status: 400,
+ statusText: "invalid: farmIds",
+ })
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx
around lines 109 - 152, The code accepts query farmIds directly
(searchParamFarmIds → farmIds) without checking they belong to the reachable
farms; before calling collectInputForOrganicMatterBalanceForFarms(...) validate
searchParamFarmIds (if present) against the accessible farms list (farms.map(f
=> f.b_id_farm)) and if any id is missing throw data("invalid: farmIds", {
status: 400, statusText: "invalid: farmIds" }); otherwise proceed using the
validated farmIds variable (searchParamFarmIds or farms.map(...)).
Comment on lines
+415
to
+417
| <CardTitle className="text-sm font-medium"> | ||
| Balans (Bedrijf) | ||
| </CardTitle> |
Contributor
There was a problem hiding this comment.
Use a selection-level label here instead of a single-farm label.
This card shows the aggregated result for the current organization selection, but the title says Balans (Bedrijf). That reads as one farm's balance and is easy to misinterpret.
Suggested fix
- Balans (Bedrijf)
+ Balans (Geselecteerde Bedrijven)📝 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.
Suggested change
| <CardTitle className="text-sm font-medium"> | |
| Balans (Bedrijf) | |
| </CardTitle> | |
| <CardTitle className="text-sm font-medium"> | |
| Balans (Geselecteerde Bedrijven) | |
| </CardTitle> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx
around lines 415 - 417, The CardTitle currently reads "Balans (Bedrijf)" which
implies a single farm; update the selection-level label shown in CardTitle
inside organization.$slug.$calendar.balance.organic-matter._index.tsx to reflect
the aggregated organization selection (for example "Balans (Selectie)" or
"Balans (Organisatie)"), and if your app uses i18n replace the hardcoded string
with the appropriate translation key instead of the literal; locate the
CardTitle component instance to make this change.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
fdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsx (1)
109-114:⚠️ Potential issue | 🟠 MajorDeduplicate
farmIdsbefore building the OM inputs.A URL like
?farmIds=A,Ais still accepted here, socollectInputForOrganicMatterBalanceForFarms(...)can receive the same farm twice and count it twice in the organization result.🛠️ Suggested fix
- const farmIds = - searchParamFarmIds ?? farms.map((farm) => farm.b_id_farm) + const farmIds = searchParamFarmIds + ? [...new Set(searchParamFarmIds)] + : farms.map((farm) => farm.b_id_farm)Also applies to: 151-156
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx around lines 109 - 114, The URL-parsed farmIds may contain duplicates, so before building OM inputs deduplicate the parsed array (searchParamFarmIds) and any other parsed farmIds array used later (the other farmIds parsing block) by converting to a Set and back (or using a unique filter) and then pass the deduplicated array to collectInputForOrganicMatterBalanceForFarms(...) so the same farm ID isn’t counted twice; ensure you handle undefined/empty cases the same way as before.fdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsx (1)
110-115:⚠️ Potential issue | 🟠 MajorDeduplicate
farmIdsbefore calling the batched calculator.A URL like
?farmIds=A,Apasses validation here, butcollectInputForNitrogenBalanceForFarms(...)will collect the same farm twice. That double-counts both the organization aggregate and the per-farm result forA.🛠️ Suggested fix
- const farmIds = - searchParamFarmIds ?? farms.map((farm) => farm.b_id_farm) + const farmIds = searchParamFarmIds + ? [...new Set(searchParamFarmIds)] + : farms.map((farm) => farm.b_id_farm)Also applies to: 152-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx around lines 110 - 115, The parsed farmIds from the URL (searchParamFarmIds) must be deduplicated before being passed to collectInputForNitrogenBalanceForFarms to avoid double-counting; modify the handling where you build searchParamFarmIds (and the similar block at the other occurrence) to replace the raw array with a deduplicated array (e.g., Array.from(new Set(...)) or equivalent) while preserving undefined when no param is present, then pass that deduplicated array into collectInputForNitrogenBalanceForFarms and any other downstream calls that use searchParamFarmIds.
🧹 Nitpick comments (1)
fdm-core/src/cultivation.test.ts (1)
106-145: Scope the second-farm fixture to only multi-farm tests.This setup now runs before every test in the suite, including tests that never use
b_id_farm_2/b_id_2, which adds avoidable DB work and coupling.♻️ Suggested refactor
- b_id_farm_2 = await addFarm(...) - b_id_2 = await addField(...) - b_lu_source_2 = "custom-2" - await enableCultivationCatalogue(...) ... - await addCultivation(... b_lu_catalogue_2, b_id_2, b_lu_start) + // Keep core single-farm fixture in global beforeEach. + // Move second-farm setup into a helper and call it only in multi-farm tests. + // Example: + // const secondFarm = await setupSecondFarmFixture() + // await addCultivation(... secondFarm.b_lu_catalogue_2, secondFarm.b_id_2, b_lu_start)Also applies to: 195-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/cultivation.test.ts` around lines 106 - 145, The test fixture that creates b_id_farm_2, b_id_2 and enables cultivation (calls to addFarm, addField, enableCultivationCatalogue and the variables b_lu_source_2) is currently executed for every test; move this setup out of the global beforeEach and into a narrower scope that only runs for multi-farm tests (e.g., a describe(...) or a nested beforeEach that wraps tests needing multiple farms) so that only tests that reference b_id_farm_2 / b_id_2 perform the DB work; ensure tests that rely on these variables still declare/consume them from that scoped setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 110-115: The parsed farmIds from the URL (searchParamFarmIds) must
be deduplicated before being passed to collectInputForNitrogenBalanceForFarms to
avoid double-counting; modify the handling where you build searchParamFarmIds
(and the similar block at the other occurrence) to replace the raw array with a
deduplicated array (e.g., Array.from(new Set(...)) or equivalent) while
preserving undefined when no param is present, then pass that deduplicated array
into collectInputForNitrogenBalanceForFarms and any other downstream calls that
use searchParamFarmIds.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 109-114: The URL-parsed farmIds may contain duplicates, so before
building OM inputs deduplicate the parsed array (searchParamFarmIds) and any
other parsed farmIds array used later (the other farmIds parsing block) by
converting to a Set and back (or using a unique filter) and then pass the
deduplicated array to collectInputForOrganicMatterBalanceForFarms(...) so the
same farm ID isn’t counted twice; ensure you handle undefined/empty cases the
same way as before.
---
Nitpick comments:
In `@fdm-core/src/cultivation.test.ts`:
- Around line 106-145: The test fixture that creates b_id_farm_2, b_id_2 and
enables cultivation (calls to addFarm, addField, enableCultivationCatalogue and
the variables b_lu_source_2) is currently executed for every test; move this
setup out of the global beforeEach and into a narrower scope that only runs for
multi-farm tests (e.g., a describe(...) or a nested beforeEach that wraps tests
needing multiple farms) so that only tests that reference b_id_farm_2 / b_id_2
perform the DB work; ensure tests that rely on these variables still
declare/consume them from that scoped setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1bb45fc2-088d-4d77-8e6f-fb5506cd1470
📒 Files selected for processing (7)
.changeset/breezy-spies-smoke.md.changeset/green-pumas-flash.md.changeset/thin-steaks-sneeze.mdfdm-app/app/components/blocks/balance/farm-select-dialog.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsxfdm-core/src/cultivation.test.ts
✅ Files skipped from review due to trivial changes (4)
- .changeset/green-pumas-flash.md
- .changeset/thin-steaks-sneeze.md
- .changeset/breezy-spies-smoke.md
- fdm-app/app/components/blocks/balance/farm-select-dialog.tsx
SvenVw
approved these changes
Mar 31, 2026
Collaborator
SvenVw
left a comment
SvenVw
left a comment
There was a problem hiding this comment.
I refactored the functions at fdm-core and fdm-calculator. Other than that, everything looks good
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
fdm-calculator/src/balance/nitrogen/input.ts (1)
205-265:⚠️ Potential issue | 🟠 MajorOne bad farm still aborts the entire batch.
This is the same failure mode raised earlier: rethrowing inside
Promise.allmakes a single farm-level collection error reject the full organization request, so broken farms cannot be excluded or flagged independently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-calculator/src/balance/nitrogen/input.ts` around lines 205 - 265, The current farm loop uses Promise.all over farmIds with a try/catch that rethrows via handleNitrogenBalanceInputCollectionError, which causes a single farm error to reject the whole Promise.all; change to per-farm failure-tolerant handling by using Promise.allSettled or having each farm-mapped async return a result object (e.g., { b_id_farm, success: true, payload: {...} } or { b_id_farm, success: false, error: ... }) instead of throwing from inside the map; locate the collectInputForNitrogenBalanceForFarm call and the catch that calls handleNitrogenBalanceInputCollectionError and replace the rethrow with returning an error marker so the caller can filter or report failed farms without aborting the entire batch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 376-417: createFarmRow currently shows only an icon and the raw
farm balance; update it to compute and render the farm's delta vs the
selection's area-weighted organization average (use the available organization
average variable or compute it from farmResults and totalArea if not present),
e.g. delta = balanceResult.balance - orgAverage, format to one decimal and
prepend +/−, and display this delta next to the balance (green for
negative/improvement, red for positive/worse, orange if non-finite) while
preserving the existing error link behavior; modify the JSX inside createFarmRow
(referencing farmResult, balanceResult, farmResult.totalArea and
params.calendar) to show the delta with proper styling and fallbacks when
balance is non-finite or balanceResult.hasErrors.
- Around line 242-291: The owner lookup via listPrincipalsForFarm is turning any
failure into a full loader error that clobbers organicMatterBalanceResult; make
the owner lookup best-effort by isolating it into its own try/catch (or by
catching errors around listPrincipalsForFarm and setting owner = undefined on
failure) so that failures in listPrincipalsForFarm do not set
organicMatterBalanceResult.hasErrors — keep the existing try/catch for the main
loader logic and only handle metadata lookup errors locally when calling
listPrincipalsForFarm/setting owner.
In `@fdm-calculator/src/balance/nitrogen/input.ts`:
- Around line 165-202: collectInputForNitrogenBalanceForFarms currently uses the
incoming farmIds array directly, so duplicate IDs produce duplicated inputs;
deduplicate farmIds at the start (e.g. const uniqueFarmIds = [...new
Set(farmIds)]) and use uniqueFarmIds everywhere in this function (replace uses
of farmIds in getEnabledCultivationCataloguesForFarms,
getEnabledFertilizerCataloguesForFarms, and the later mapping/aggregation) so
each farm is processed exactly once while leaving the rest of the logic and
variable names (farmCultivationCatalogues, farmFertilizerCatalogues,
uniqueCultivationSources, uniqueFertilizerSources, allCultivations,
allFertilizers) unchanged.
In `@fdm-calculator/src/balance/organic-matter/input.ts`:
- Around line 158-179: The function collectInputForOrganicMatterBalanceForFarms
currently runs batch queries with the incoming farmIds array which may contain
duplicates; deduplicate farmIds at the start of the transaction (e.g. produce
uniqueFarmIds from farmIds) and use uniqueFarmIds for the batch calls to
getEnabledCultivationCataloguesForFarms and
getEnabledFertilizerCataloguesForFarms (and any other farm-based batch lookups
performed on tx) so each farm is queried once; then iterate over uniqueFarmIds
for subsequent processing and emit one OrganicMatterBalanceInput per unique
b_id_farm (or map results back to the original farmIds if you intentionally must
preserve count elsewhere) to avoid duplicate outputs affecting averages.
In `@fdm-core/src/fertilizer.ts`:
- Around line 62-98: The exported function getFertilizersFromCatalogues
currently accepts arbitrary catalogueIds and can expose farm-scoped sources
(p_source = b_id_farm) without authorization; change it to require a principal
(e.g., add principal_id parameter) or move the function to internal-only, and
enforce authorization before running the DB query: validate or filter
catalogueIds against the principal's allowed sources (reject or omit
unauthorized farm IDs) prior to calling fdm.select(...). Keep the existing
return shape and error handling via handleError, and preserve use of
deriveFertilizerType and casting of p_app_method_options when mapping results.
---
Duplicate comments:
In `@fdm-calculator/src/balance/nitrogen/input.ts`:
- Around line 205-265: The current farm loop uses Promise.all over farmIds with
a try/catch that rethrows via handleNitrogenBalanceInputCollectionError, which
causes a single farm error to reject the whole Promise.all; change to per-farm
failure-tolerant handling by using Promise.allSettled or having each farm-mapped
async return a result object (e.g., { b_id_farm, success: true, payload: {...} }
or { b_id_farm, success: false, error: ... }) instead of throwing from inside
the map; locate the collectInputForNitrogenBalanceForFarm call and the catch
that calls handleNitrogenBalanceInputCollectionError and replace the rethrow
with returning an error marker so the caller can filter or report failed farms
without aborting the entire batch.
🪄 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: 5022a2d8-0082-49f9-809c-7fe0b068c2c6
📒 Files selected for processing (17)
.changeset/breezy-spies-smoke.md.changeset/thin-steaks-sneeze.mdfdm-app/app/routes/about.whats-new._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsxfdm-calculator/src/balance/nitrogen/input.test.tsfdm-calculator/src/balance/nitrogen/input.tsfdm-calculator/src/balance/organic-matter/input.test.tsfdm-calculator/src/balance/organic-matter/input.tsfdm-core/src/catalogues.tsfdm-core/src/cultivation.test.tsfdm-core/src/cultivation.tsfdm-core/src/fertilizer.test.tsfdm-core/src/fertilizer.tsfdm-core/src/index.ts
✅ Files skipped from review due to trivial changes (4)
- .changeset/thin-steaks-sneeze.md
- fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter._index.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- .changeset/breezy-spies-smoke.md
- fdm-core/src/index.ts
- fdm-calculator/src/balance/organic-matter/input.test.ts
- fdm-core/src/fertilizer.test.ts
- fdm-core/src/cultivation.test.ts
- fdm-calculator/src/balance/nitrogen/input.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
fdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsx (1)
224-230:⚠️ Potential issue | 🟠 MajorSelected farms can silently disappear from the overview.
This only keeps farms that got at least one entry in
rawFarmResultsMap. If a selected farm has zero fields or the calculator yields no field rows for it, the farm drops out of the list entirely andhasErrorsnever reflects that missing result. Iterate the selected farms instead and return an explicit no-data/error row when a selected farm has no result set.Also applies to: 374-376
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx around lines 224 - 230, The current mapping filters farmsExtended by rawFarmResultsMap and thus silently drops selected farms that have zero fields or no calculator rows; update the logic that builds farmResults (and the analogous block around the other use at 374-376) to iterate over the selected farmsExtended list directly, check rawFarmResultsMap[farm.b_id_farm], and when absent or empty push/return an explicit no-data/error row object (and set hasErrors accordingly) instead of omitting the farm; reference the farmResults variable and rawFarmResultsMap and ensure the returned item for missing data carries a clear flag or message so the UI and hasErrors can detect and render missing-result farms.
🧹 Nitpick comments (2)
fdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsx (1)
53-55: Drop the owner lookup until this page actually renders it.
ownernever gets read in this route, but every selected farm still triggerslistPrincipalsForFarm(...)and ships that data inasyncData. Removing it here trims loader latency and the deferred payload.Also applies to: 242-254
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx around lines 53 - 55, The FarmResult type includes an owner field that eagerly calls listPrincipalsForFarm and populates asyncData even though the route never reads owner; remove the owner property from FarmResult and any code paths that call listPrincipalsForFarm for this route (e.g., the logic that builds asyncData for selected farms) so the loader no longer performs the principals lookup; keep listPrincipalsForFarm calls only where the UI actually renders owner (add them later when rendering requires it).fdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsx (1)
55-57: Drop the owner lookup until this page actually renders it.
ownernever gets read in this route, but every selected farm still triggerslistPrincipalsForFarm(...)and ships that data inasyncData. Removing it here trims loader latency and the deferred payload.Also applies to: 243-255
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx around lines 55 - 57, The FarmResult type includes an owner property that forces calling listPrincipalsForFarm(...) even though the route never reads owner; remove the owner field from the FarmResult type and stop calling listPrincipalsForFarm(...) when building the asyncData/deferred payload (also remove any mapping that awaits or includes owner for each farm in the code path that constructs asyncData) so the loader no longer fetches or ships principal data until the page actually needs to render owners.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/routes/about.whats-new._index.tsx`:
- Line 87: Update the Dutch changelog sentence string in
about.whats-new._index.tsx: replace the incorrect fragment "een nieuw Bouwplan
pagina" with the grammatically correct "een nieuwe Bouwplan-pagina" so the full
string reads: "Deze update introduceert de Organische stofbalans voor inzicht in
bodemgezondheid, een nieuwe Bouwplan-pagina voor efficiënt gewasbeheer, en voegt
nitraatuitspoeling toe aan de stikstofbalans." Locate and edit the string
literal shown in the diff to apply this change.
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 370-373: resolvedNitrogenBalanceResult is being force-cast to a
shape with removal:number (farmChartBalanceData) though actual data has
removal.total; instead of casting, map
combinedResult/resolvedNitrogenBalanceResult into the exact props
NitrogenBalanceChart expects: read resolvedNitrogenBalanceResult.removal.total
(and other numeric fields) and produce an object matching NitrogenBalanceChart’s
input type (or adjust NitrogenBalanceChart to accept NitrogenBalanceNumeric if
intended), replacing the "as unknown as { balance: number; removal: number } &
NitrogenBalanceNumeric" cast with an explicit normalization step that extracts
removal.total into removal and ensures all numeric fields match
NitrogenBalanceChart.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 400-405: The delta color logic in the deltaClass assignment is
inverted for organic matter: change the ternary so higher-than-average deltas
render as "text-green-600" and lower-than-average as "text-red-600"; update the
deltaClass expression (the block that currently reads delta === undefined ?
"text-orange-500" : delta < 0 ? "text-green-600" : "text-red-600") to use delta
< 0 ? "text-red-600" : "text-green-600" (keeping the undefined case as
"text-orange-500"), and make the same change in the other identical deltaClass
occurrence in this file.
- Around line 223-229: The current implementation filters farmsExtended by
presence in rawFarmResultsMap, causing selected farms with zero fields or no
calculator rows to disappear; change the logic in the farmsExtended ->
farmResults Promise.all construction (and the similar block around the other
occurrence at the referenced lines) to iterate over the selected farmsExtended
array directly, lookup rawFarmResultsMap[farm.b_id_farm], and when missing or
empty return an explicit no-data/error row for that farm (and set/propagate
hasErrors accordingly) instead of skipping it so all selected farms remain in
farmResults.
In `@fdm-calculator/src/balance/organic-matter/input.test.ts`:
- Around line 19-35: The test uses Vitest mocking (vi.mock/vi.spyOn) and lacks
per-test reset causing shared state; remove the vi.mock(...) block and any
vi.spyOn usage and instead create a plain manual mock object for the fdm core
functions (getFields, getCultivations, getHarvests, getSoilAnalyses,
getFertilizerApplications, getCultivationsFromCatalogue,
getFertilizersFromCatalogue, getEnabledCultivationCataloguesForFarms,
getEnabledFertilizerCataloguesForFarms, getCultivationsFromCatalogues,
getFertilizersFromCatalogues) and assign those implementations directly in a
beforeEach so each test gets a fresh implementation, then restore or re-require
the module (or explicitly reset the assigned functions) in afterEach to avoid
leakage; replace all vi.spyOn(...).mockResolvedValue(...) usages with direct
assignments on that manual mock object and remove calls to vi.resetAllMocks().
In `@fdm-core/src/cultivation.ts`:
- Around line 67-100: getCultivationsFromCatalogues currently accepts arbitrary
catalogueIds and can be used to bypass farm-level gating; fix it by adding a
principal-based filter (or make the function internal): update the signature of
getCultivationsFromCatalogues to accept a principalId (e.g., principalId: string
| null) and include a where clause filtering
schema.cultivationsCatalogue.b_principal_id to that principalId (or only include
the filter when principalId is non-null), mirroring how
getFertilizersFromCatalogues enforces principal_id, and adjust callers to pass
the current principal or replace the export to keep the helper internal if you
prefer not to expose it.
In `@fdm-core/src/fertilizer.test.ts`:
- Around line 346-379: The generator makeFertilizer sometimes emits p_type =
"other", which is not accepted by addFertilizerToCatalogue (allowed: "manure" |
"mineral" | "compost" | null); update makeFertilizer to choose p_type only from
the supported set (e.g., ["manure","mineral","compost", null]) instead of
["manure","mineral","compost","other"], remove the unsafe cast workaround, and
ensure the produced object type matches Parameters<typeof
addFertilizerToCatalogue>[3] so tests exercise only valid input paths.
---
Duplicate comments:
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 224-230: The current mapping filters farmsExtended by
rawFarmResultsMap and thus silently drops selected farms that have zero fields
or no calculator rows; update the logic that builds farmResults (and the
analogous block around the other use at 374-376) to iterate over the selected
farmsExtended list directly, check rawFarmResultsMap[farm.b_id_farm], and when
absent or empty push/return an explicit no-data/error row object (and set
hasErrors accordingly) instead of omitting the farm; reference the farmResults
variable and rawFarmResultsMap and ensure the returned item for missing data
carries a clear flag or message so the UI and hasErrors can detect and render
missing-result farms.
---
Nitpick comments:
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 55-57: The FarmResult type includes an owner property that forces
calling listPrincipalsForFarm(...) even though the route never reads owner;
remove the owner field from the FarmResult type and stop calling
listPrincipalsForFarm(...) when building the asyncData/deferred payload (also
remove any mapping that awaits or includes owner for each farm in the code path
that constructs asyncData) so the loader no longer fetches or ships principal
data until the page actually needs to render owners.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 53-55: The FarmResult type includes an owner field that eagerly
calls listPrincipalsForFarm and populates asyncData even though the route never
reads owner; remove the owner property from FarmResult and any code paths that
call listPrincipalsForFarm for this route (e.g., the logic that builds asyncData
for selected farms) so the loader no longer performs the principals lookup; keep
listPrincipalsForFarm calls only where the UI actually renders owner (add them
later when rendering requires it).
🪄 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: 9ab22a45-d483-431f-b8c2-9830caa3f82e
📒 Files selected for processing (18)
.changeset/breezy-spies-smoke.md.changeset/thin-steaks-sneeze.mdfdm-app/app/routes/about.whats-new._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsxfdm-calculator/src/balance/nitrogen/input.test.tsfdm-calculator/src/balance/nitrogen/input.tsfdm-calculator/src/balance/organic-matter/input.test.tsfdm-calculator/src/balance/organic-matter/input.tsfdm-core/src/catalogues.tsfdm-core/src/cultivation.test.tsfdm-core/src/cultivation.tsfdm-core/src/fertilizer.test.tsfdm-core/src/fertilizer.tsfdm-core/src/index.ts
✅ Files skipped from review due to trivial changes (6)
- .changeset/thin-steaks-sneeze.md
- fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.$b_id.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter._index.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter.tsx
- .changeset/breezy-spies-smoke.md
- fdm-calculator/src/balance/organic-matter/input.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- fdm-core/src/cultivation.test.ts
- fdm-core/src/catalogues.ts
- fdm-calculator/src/balance/nitrogen/input.ts
| title: "Nieuw: Bouwplan & OS Balans. Oogstregistratie is verbeterd", | ||
| description: | ||
| "Deze update introduceert de Organische Stof Balans voor inzicht in bodemgezondheid, een nieuw Bouwplan pagina voor efficiënt gewasbeheer, en voegt nitraatuitspoeling toe aan de stikstofbalans.", | ||
| "Deze update introduceert de Organische stofbalans voor inzicht in bodemgezondheid, een nieuw Bouwplan pagina voor efficiënt gewasbeheer, en voegt nitraatuitspoeling toe aan de stikstofbalans.", |
Contributor
There was a problem hiding this comment.
Correct Dutch wording in the changelog sentence.
Line 87: "een nieuw Bouwplan pagina" should be "een nieuwe Bouwplan-pagina" for correct grammar/readability.
✍️ Suggested copy fix
- "Deze update introduceert de Organische stofbalans voor inzicht in bodemgezondheid, een nieuw Bouwplan pagina voor efficiënt gewasbeheer, en voegt nitraatuitspoeling toe aan de stikstofbalans.",
+ "Deze update introduceert de Organische stofbalans voor inzicht in bodemgezondheid, een nieuwe Bouwplan-pagina voor efficiënt gewasbeheer, en voegt nitraatuitspoeling toe aan de stikstofbalans.",📝 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.
Suggested change
| "Deze update introduceert de Organische stofbalans voor inzicht in bodemgezondheid, een nieuw Bouwplan pagina voor efficiënt gewasbeheer, en voegt nitraatuitspoeling toe aan de stikstofbalans.", | |
| "Deze update introduceert de Organische stofbalans voor inzicht in bodemgezondheid, een nieuwe Bouwplan-pagina voor efficiënt gewasbeheer, en voegt nitraatuitspoeling toe aan de stikstofbalans.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fdm-app/app/routes/about.whats-new._index.tsx` at line 87, Update the Dutch
changelog sentence string in about.whats-new._index.tsx: replace the incorrect
fragment "een nieuw Bouwplan pagina" with the grammatically correct "een nieuwe
Bouwplan-pagina" so the full string reads: "Deze update introduceert de
Organische stofbalans voor inzicht in bodemgezondheid, een nieuwe
Bouwplan-pagina voor efficiënt gewasbeheer, en voegt nitraatuitspoeling toe aan
de stikstofbalans." Locate and edit the string literal shown in the diff to
apply this change.
Comment on lines
+19
to
35
| // Mock the @nmi-agro/fdm-core module | ||
| vi.mock("@nmi-agro/fdm-core", async () => { | ||
| const original = await vi.importActual("@nmi-agro/fdm-core") | ||
| return { | ||
| ...original, | ||
| getFields: vi.fn(), | ||
| getField: vi.fn(), | ||
| getCultivations: vi.fn(), | ||
| getHarvests: vi.fn(), | ||
| getSoilAnalyses: vi.fn(), | ||
| getFertilizerApplications: vi.fn(), | ||
| getFertilizers: vi.fn(), | ||
| getCultivationsFromCatalogue: vi.fn(), | ||
| getFertilizersFromCatalogue: vi.fn(), | ||
| getEnabledCultivationCataloguesForFarms: vi.fn(), | ||
| getEnabledFertilizerCataloguesForFarms: vi.fn(), | ||
| getCultivationsFromCatalogues: vi.fn(), | ||
| getFertilizersFromCatalogues: vi.fn(), | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="fdm-calculator/src/balance/organic-matter/input.test.ts"
rg -n 'describe\(|beforeEach\(|vi\.mock\(|vi\.spyOn\(' "$FILE"Repository: nmi-agro/fdm
Length of output: 2399
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="fdm-calculator/src/balance/organic-matter/input.test.ts"
# Get a broader view of the test structure
echo "=== Test structure overview ==="
rg -n 'describe\(|beforeEach\(|afterEach\(|afterAll\(' "$FILE" | head -20
echo ""
echo "=== First describe block (lines 199-230) ==="
sed -n '199,230p' "$FILE"
echo ""
echo "=== Check for beforeEach in first describe (lines 199-327) ==="
sed -n '199,327p' "$FILE" | grep -E 'beforeEach|afterEach' || echo "No beforeEach/afterEach in first describe block"Repository: nmi-agro/fdm
Length of output: 1722
Refactor away from Vitest mocking utilities in this test.
This test file uses vi.mock() and vi.spyOn() extensively, but the fdm project has established practices to avoid Vitest's mocking utilities due to implementation problems. The first describe block lacks a beforeEach(() => vi.resetAllMocks()), causing shared mock state to leak between test cases, but the deeper issue is the mocking strategy itself.
Refactor to use manual mock objects with direct function implementations instead of vi.mock(), vi.spyOn(), and vi.resetAllMocks(). For example, replace vi.spyOn(fdmCore, "getFields").mockResolvedValue(...) with direct method assignments on a plain mock object.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fdm-calculator/src/balance/organic-matter/input.test.ts` around lines 19 -
35, The test uses Vitest mocking (vi.mock/vi.spyOn) and lacks per-test reset
causing shared state; remove the vi.mock(...) block and any vi.spyOn usage and
instead create a plain manual mock object for the fdm core functions (getFields,
getCultivations, getHarvests, getSoilAnalyses, getFertilizerApplications,
getCultivationsFromCatalogue, getFertilizersFromCatalogue,
getEnabledCultivationCataloguesForFarms, getEnabledFertilizerCataloguesForFarms,
getCultivationsFromCatalogues, getFertilizersFromCatalogues) and assign those
implementations directly in a beforeEach so each test gets a fresh
implementation, then restore or re-require the module (or explicitly reset the
assigned functions) in afterEach to avoid leakage; replace all
vi.spyOn(...).mockResolvedValue(...) usages with direct assignments on that
manual mock object and remove calls to vi.resetAllMocks().
Comment on lines
+67
to
100
| /** | ||
| * Retrieves cultivations available in the given list of catalogues. | ||
| * | ||
| * No permission checks are performed. If a catalogue permission system is implemented in the future this may change. | ||
| * | ||
| * @param fdm The FDM instance providing the connection to the database. The instance can be created with {@link createFdmServer}. | ||
| * @param catalogueIds The source IDs of the catalogues to retrieve cultivations from, such as "brp". | ||
| * @returns A Promise that resolves with a flat array of cultivation catalogue entries across all given catalogues. | ||
| * @alpha | ||
| */ | ||
| export async function getCultivationsFromCatalogues( | ||
| fdm: FdmType, | ||
| catalogueIds: schema.cultivationCatalogueSelectingTypeSelect["b_lu_source"][], | ||
| ): Promise<CultivationCatalogue[]> { | ||
| try { | ||
| if (catalogueIds.length === 0) { | ||
| return [] | ||
| } | ||
|
|
||
| // Get cultivations from enabled catalogues | ||
| const cultivationsCatalogue = await fdm | ||
| return fdm | ||
| .select() | ||
| .from(schema.cultivationsCatalogue) | ||
| .where( | ||
| inArray( | ||
| schema.cultivationsCatalogue.b_lu_source, | ||
| enabledCatalogues.map( | ||
| (c: { b_lu_source: string }) => c.b_lu_source, | ||
| ), | ||
| ), | ||
| inArray(schema.cultivationsCatalogue.b_lu_source, catalogueIds), | ||
| ) | ||
| .orderBy( | ||
| asc(schema.cultivationsCatalogue.b_lu_source), | ||
| asc(schema.cultivationsCatalogue.b_lu_name), | ||
| ) | ||
|
|
||
| return cultivationsCatalogue | ||
| } catch (err) { | ||
| throw handleError(err, "Exception for getCultivationsFromCatalogue", { | ||
| principal_id, | ||
| b_id_farm, | ||
| throw handleError(err, "Exception for getCultivationsFromCatalogues", { | ||
| catalogueIds, | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Don't expose a raw cultivation-catalogue lookup without authorization.
getCultivationsFromCatalogue(...) is still farm-gated via getEnabledCultivationCatalogues(...), but this new helper accepts arbitrary catalogueIds and queries them directly. Because it is re-exported from fdm-core/src/index.ts, callers can bypass the farm-level gate entirely and read any matching b_lu_source rows if non-public sources exist. Please either add a principal_id-based source filter like getFertilizersFromCatalogues(...) or keep this helper internal.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fdm-core/src/cultivation.ts` around lines 67 - 100,
getCultivationsFromCatalogues currently accepts arbitrary catalogueIds and can
be used to bypass farm-level gating; fix it by adding a principal-based filter
(or make the function internal): update the signature of
getCultivationsFromCatalogues to accept a principalId (e.g., principalId: string
| null) and include a where clause filtering
schema.cultivationsCatalogue.b_principal_id to that principalId (or only include
the filter when principalId is non-null), mirroring how
getFertilizersFromCatalogues enforces principal_id, and adjust callers to pass
the current principal or replace the export to keep the helper internal if you
prefer not to expose it.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
fdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsx (2)
150-152:⚠️ Potential issue | 🟠 MajorDeduplicate
farmIdsbefore running calculations.At Line 150, repeated query IDs (e.g.
farmIds=a,a) are accepted and passed into the collector at Line 165, which can double-count farms in organization aggregates.💡 Suggested fix
- const farmIds = - searchParamFarmIds ?? farms.map((farm) => farm.b_id_farm) + const farmIds = searchParamFarmIds + ? [...new Set(searchParamFarmIds)] + : farms.map((farm) => farm.b_id_farm)Also applies to: 165-170
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx around lines 150 - 152, The farmIds array (constructed as searchParamFarmIds ?? farms.map(farm => farm.b_id_farm)) can contain duplicates and is passed into the downstream collector/aggregation logic, causing double-counting; before any calculations or passing to the collector, replace farmIds with a deduplicated version (preserve the existing fallback behavior for searchParamFarmIds) — e.g., compute a unique list from searchParamFarmIds or farms.map(...) using a Set and assign back to farmIds so subsequent uses (the collector/aggregation calls) operate on unique farm IDs only.
222-227:⚠️ Potential issue | 🟠 MajorCompute
farmResultsfrom selected farms only.At Line 223, results are generated for all accessible farms, but calculations were executed only for selected
farmIds. Unselected farms then fall into the “Geen veldgegevens beschikbaar” path, inflating error state (Line 372) and showing misleading rows.💡 Suggested fix
+ const selectedFarmIds = new Set(farmIds) const farmResults = await Promise.all( farmsExtended + .filter((farm) => selectedFarmIds.has(farm.b_id_farm)) .map(async (farm) => { const fieldResults = rawFarmResultsMap[farm.b_id_farm]Also applies to: 372-375
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx around lines 222 - 227, The code computes farmResults over all accessible farms (farmsExtended) but only ran calculations for the selected farmIds, causing unselected farms to hit the "Geen veldgegevens beschikbaar" branch; update the farmResults creation to iterate only over the selected farms (e.g., filter farmsExtended by farmIds or check farm.b_id_farm ∈ farmIds before mapping) so rawFarmResultsMap lookups and the subsequent logic (the block using rawFarmResultsMap and the "Geen veldgegevens beschikbaar" handling) only run for selected farms; ensure any downstream code that expects farmResults still receives the same shape (skip or exclude unselected farms rather than producing empty entries).fdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsx (1)
223-237:⚠️ Potential issue | 🟠 MajorOnly build
farmResultsfor the selected farms.This still maps over every accessible farm. When
farmIdsis a subset, the unselected farms fall into the"Geen veldgegevens beschikbaar"branch, show up as fake error rows, and incorrectly flip the page-level warning.🛡️ Suggested fix
+ const selectedFarmIds = new Set(farmIds) const farmResults = await Promise.all( - farmsExtended.map(async (farm) => { + farmsExtended + .filter((farm) => selectedFarmIds.has(farm.b_id_farm)) + .map(async (farm) => { const fieldResults = rawFarmResultsMap[farm.b_id_farm] if (!fieldResults || fieldResults.length === 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx around lines 223 - 237, The code builds farmResults by mapping over farmsExtended which includes all accessible farms instead of restricting to the user-selected farmIds, causing unselected farms to produce bogus "Geen veldgegevens beschikbaar" errors; update the farmResults construction (where farmsExtended is iterated and farmResults is created) to first filter farmsExtended by the selected farmIds (e.g., farmsExtended.filter(f => farmIds.includes(f.b_id_farm))) or iterate over farmIds to only process selected farms, then use rawFarmResultsMap[farm.b_id_farm] as before so only selected farms produce results and the page-level warning logic remains correct.
🧹 Nitpick comments (2)
fdm-core/src/fertilizer.test.ts (1)
466-484: Consider a more descriptive test name.The test name "should handle empty catalogues" may be misleading. This test validates behavior when an enabled catalogue ID doesn't correspond to any actual catalogue entries (i.e., an invalid or non-existent catalogue source). A clearer name would better communicate the edge case:
♻️ Suggested rename
- it("should handle empty catalogues", async () => { + it("should return empty array when enabled catalogue source has no entries", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-core/src/fertilizer.test.ts` around lines 466 - 484, Rename the test case to a clearer, more specific name that reflects the edge case being validated: replace the it(...) title "should handle empty catalogues" with something like "should return empty array when enabled catalogue ID is invalid or non-existent" in the test that uses addFarm, enableFertilizerCatalogue, and getFertilizersFromCatalogue so the intent (enabled catalogue ID with no entries) is explicit.fdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsx (1)
367-371: Remove the unsafe double-cast for chart data.At Line 368, casting to a type that includes
removal(not part of organic-matter result) weakens type safety and can hide model drift.♻️ Suggested cleanup
- const farmChartBalanceData = - resolvedOrganicMatterBalanceResult as unknown as { - supply: number - removal: number - } & OrganicMatterBalanceNumeric @@ <OrganicMatterBalanceChart - supply={farmChartBalanceData.supply} - degradation={farmChartBalanceData.degradation} + supply={resolvedOrganicMatterBalanceResult.supply} + degradation={resolvedOrganicMatterBalanceResult.degradation} />Also applies to: 543-546
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx around lines 367 - 371, The code currently performs an unsafe double-cast on resolvedOrganicMatterBalanceResult into a type that includes supply/removal (see farmChartBalanceData and OrganicMatterBalanceNumeric), which hides mismatches; instead explicitly construct a properly-typed object by extracting numeric fields from resolvedOrganicMatterBalanceResult and computing/assigning supply and removal (or use a narrow type guard) before assigning to farmChartBalanceData—e.g., validate/rescue the numeric fields on resolvedOrganicMatterBalanceResult, then create a new object { ...validatedNumeric, supply: computedSupply, removal: computedRemoval } and use that object as the chart data; apply the same pattern to the other occurrence around lines 543–546.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 395-403: The icon rendering should honor balanceResult.hasErrors
first: update the conditional around the CircleCheck/CircleX/CircleAlert so that
if balanceResult.hasErrors is true you render the warning icon (CircleAlert)
regardless of whether balanceResult.balance is numeric; otherwise fall back to
Number.isFinite(balanceResult.balance) and then render CircleCheck when balance
<= target or CircleX when > target. Locate the JSX that references balanceResult
(from calculateNitrogenBalancesFieldToFarm) and adjust the branch to check
balanceResult.hasErrors before the numeric comparisons so the row body ("Bekijk
foutmelding") and icon are consistent.
---
Duplicate comments:
In `@fdm-app/app/routes/organization`.$slug.$calendar.balance.nitrogen._index.tsx:
- Around line 223-237: The code builds farmResults by mapping over farmsExtended
which includes all accessible farms instead of restricting to the user-selected
farmIds, causing unselected farms to produce bogus "Geen veldgegevens
beschikbaar" errors; update the farmResults construction (where farmsExtended is
iterated and farmResults is created) to first filter farmsExtended by the
selected farmIds (e.g., farmsExtended.filter(f =>
farmIds.includes(f.b_id_farm))) or iterate over farmIds to only process selected
farms, then use rawFarmResultsMap[farm.b_id_farm] as before so only selected
farms produce results and the page-level warning logic remains correct.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 150-152: The farmIds array (constructed as searchParamFarmIds ??
farms.map(farm => farm.b_id_farm)) can contain duplicates and is passed into the
downstream collector/aggregation logic, causing double-counting; before any
calculations or passing to the collector, replace farmIds with a deduplicated
version (preserve the existing fallback behavior for searchParamFarmIds) — e.g.,
compute a unique list from searchParamFarmIds or farms.map(...) using a Set and
assign back to farmIds so subsequent uses (the collector/aggregation calls)
operate on unique farm IDs only.
- Around line 222-227: The code computes farmResults over all accessible farms
(farmsExtended) but only ran calculations for the selected farmIds, causing
unselected farms to hit the "Geen veldgegevens beschikbaar" branch; update the
farmResults creation to iterate only over the selected farms (e.g., filter
farmsExtended by farmIds or check farm.b_id_farm ∈ farmIds before mapping) so
rawFarmResultsMap lookups and the subsequent logic (the block using
rawFarmResultsMap and the "Geen veldgegevens beschikbaar" handling) only run for
selected farms; ensure any downstream code that expects farmResults still
receives the same shape (skip or exclude unselected farms rather than producing
empty entries).
---
Nitpick comments:
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 367-371: The code currently performs an unsafe double-cast on
resolvedOrganicMatterBalanceResult into a type that includes supply/removal (see
farmChartBalanceData and OrganicMatterBalanceNumeric), which hides mismatches;
instead explicitly construct a properly-typed object by extracting numeric
fields from resolvedOrganicMatterBalanceResult and computing/assigning supply
and removal (or use a narrow type guard) before assigning to
farmChartBalanceData—e.g., validate/rescue the numeric fields on
resolvedOrganicMatterBalanceResult, then create a new object {
...validatedNumeric, supply: computedSupply, removal: computedRemoval } and use
that object as the chart data; apply the same pattern to the other occurrence
around lines 543–546.
In `@fdm-core/src/fertilizer.test.ts`:
- Around line 466-484: Rename the test case to a clearer, more specific name
that reflects the edge case being validated: replace the it(...) title "should
handle empty catalogues" with something like "should return empty array when
enabled catalogue ID is invalid or non-existent" in the test that uses addFarm,
enableFertilizerCatalogue, and getFertilizersFromCatalogue so the intent
(enabled catalogue ID with no entries) is explicit.
🪄 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: 1006b049-d8a1-4dfc-9bdf-bdb07e1216d4
📒 Files selected for processing (4)
fdm-app/app/components/blocks/balance/nitrogen-chart.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.nitrogen._index.tsxfdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsxfdm-core/src/fertilizer.test.ts
This was referenced Apr 2, 2026
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.
Enhancements
Closes #493
Summary by CodeRabbit
New Features
Bug Fixes & Improvements
Tests
Documentation