Show specific error messages for balance calculations on empty farms#546
Merged
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
fdm-calculator/src/balance/organic-matter/index.test.ts (1)
49-63: Expand this test to validatehasErrorsand top-levelerrorMessage.The new UI path depends on
errorMessage, so validating onlyfieldErrorMessagesleaves a regression gap.💡 Suggested test update
- it("should return correct error message for no fields in input", async () => { - expect( - ( - await calculateOrganicMatterBalance(mockFdm, { - fertilizerDetails: [], - cultivationDetails: [], - fields: [], - timeFrame: { - start: new Date("2023-01-01"), - end: new Date("2023-12-31"), - }, - }) - ).fieldErrorMessages, - ).toContain("No fields in input") - }) + it("should return correct error payload for no fields in input", async () => { + const result = await calculateOrganicMatterBalance(mockFdm, { + fertilizerDetails: [], + cultivationDetails: [], + fields: [], + timeFrame: { + start: new Date("2023-01-01"), + end: new Date("2023-12-31"), + }, + }) + + expect(result.hasErrors).toBe(true) + expect(result.errorMessage).toBe("No fields in input") + expect(result.fieldErrorMessages).toContain("No fields in input") + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-calculator/src/balance/organic-matter/index.test.ts` around lines 49 - 63, The test for calculateOrganicMatterBalance that currently asserts only fieldErrorMessages should be expanded to also assert the top-level error flags: call calculateOrganicMatterBalance with empty fields (as in the diff) and assert that the returned object's hasErrors property is true and that the top-level errorMessage contains or equals "No fields in input" (in addition to the existing assertion on fieldErrorMessages) so the UI path that reads errorMessage is covered.fdm-calculator/src/balance/nitrogen/index.test.ts (1)
252-266: Strengthen this empty-input test to cover the full error contract.This case now drives UI behavior via top-level
errorMessage; asserthasErrorsanderrorMessagein addition tofieldErrorMessagesto prevent regressions.💡 Suggested test update
- it("should return correct error message for no fields in input", async () => { - expect( - ( - await calculateNitrogenBalance(mockFdm, { - fertilizerDetails: [], - cultivationDetails: [], - fields: [], - timeFrame: { - start: new Date("2023-01-01"), - end: new Date("2023-12-31"), - }, - }) - ).fieldErrorMessages, - ).toContain("No fields in input") - }) + it("should return correct error payload for no fields in input", async () => { + const result = await calculateNitrogenBalance(mockFdm, { + fertilizerDetails: [], + cultivationDetails: [], + fields: [], + timeFrame: { + start: new Date("2023-01-01"), + end: new Date("2023-12-31"), + }, + }) + + expect(result.hasErrors).toBe(true) + expect(result.errorMessage).toBe("No fields in input") + expect(result.fieldErrorMessages).toContain("No fields in input") + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fdm-calculator/src/balance/nitrogen/index.test.ts` around lines 252 - 266, The test for empty input should assert the full error contract: after calling calculateNitrogenBalance(mockFdm, {...}) verify that the returned object has hasErrors set to true and errorMessage equals the expected top-level message (in addition to the existing assertion on fieldErrorMessages), so update the spec to check response.hasErrors and response.errorMessage alongside response.fieldErrorMessages to prevent regressions in UI behavior.fdm-app/app/routes/organization.$slug.$calendar.balance.organic-matter._index.tsx (1)
248-248: Consider extracting the magic string to a shared constant.The error message
"No fields in input"is used as both a value (line 248) and a comparison key (lines 465-466). Since this string is also used in the nitrogen route and potentially infdm-calculator, extracting it to a shared constant would improve maintainability and reduce the risk of typos causing silent failures.💡 Example approach
Create a shared constant (e.g., in a constants file or import from
@nmi-agro/fdm-calculatorif defined there):// In a shared constants file or exported from fdm-calculator export const ERROR_NO_FIELDS_IN_INPUT = "No fields in input"Then use it in both locations for consistency.
Also applies to: 465-468
🤖 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 at line 248, Extract the literal "No fields in input" into a shared exported constant (for example ERROR_NO_FIELDS_IN_INPUT) and replace all occurrences instead of using the magic string; update the place that sets errorMessage (symbol: errorMessage) and the places that compare against that value (the comparison in this route and the similar check in the nitrogen route / fdm-calculator usage) to import and use the shared constant so both assignment and comparisons reference the same identifier.
🤖 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:
- Line 338: The Suspense/key prop string contains an extra closing brace in the
template literal used for the key; update the key value that references
loaderData.organization.id and location.search (the
key={`${loaderData.organization.id},${location.search}}`} usage) to remove the
stray `}` so it becomes a well-formed template literal combining those two
values.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Line 343: Fix the extra closing brace in the Suspense/key template literal:
locate the key prop that currently reads
key={`${loaderData.organization.id},${location.search}}`} (in the component
rendering Suspense) and remove the trailing `}` so the template becomes
`${loaderData.organization.id},${location.search}` producing
key={`${loaderData.organization.id},${location.search}`}.
- Around line 312-314: The returned farmIds are built from the unsorted farms
array causing order mismatch with the area-sorted farmsExtended used earlier;
update the return to use the already-computed farmIds (or map/filter from
farmsExtended) instead of filtering/mapping farms so asyncData.farmIds preserves
the display order (refer to the farmIds variable and farmsExtended used around
lines where balance calculations occur).
In `@fdm-calculator/src/balance/nitrogen/index.ts`:
- Around line 82-100: The fallback for empty input currently creates a synthetic
placeholder field (with b_id: "" etc.) and passes it to
calculateNitrogenBalancesFieldToFarm, which can leak a phantom field downstream;
instead, detect fieldInputs.length === 0 and call
calculateNitrogenBalancesFieldToFarm with an empty array ([]) so aggregation
runs over no fields, preserving the errorMessage return; update the return to
stop injecting any fake field and pass [] to
calculateNitrogenBalancesFieldToFarm while keeping the existing errorMessage and
flag parameters.
In `@fdm-calculator/src/balance/organic-matter/index.ts`:
- Around line 87-105: The current early-return creates a fake field object
(b_id: "") which can leak into downstream UIs; instead, return an empty fields
array and surface the error via the existing error signaling used by the
aggregator. Replace the block that checks fieldInputs.length === 0 so it returns
{ errorMessage, ...calculateOrganicMatterBalancesFieldToFarm([], true,
[errorMessage]) } (i.e. pass an empty array to
calculateOrganicMatterBalancesFieldToFarm and preserve
errorMessage/fieldErrorMessages) rather than constructing a placeholder field;
ensure you reference fieldInputs, errorMessage, and
calculateOrganicMatterBalancesFieldToFarm when making the change.
---
Nitpick comments:
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Line 248: Extract the literal "No fields in input" into a shared exported
constant (for example ERROR_NO_FIELDS_IN_INPUT) and replace all occurrences
instead of using the magic string; update the place that sets errorMessage
(symbol: errorMessage) and the places that compare against that value (the
comparison in this route and the similar check in the nitrogen route /
fdm-calculator usage) to import and use the shared constant so both assignment
and comparisons reference the same identifier.
In `@fdm-calculator/src/balance/nitrogen/index.test.ts`:
- Around line 252-266: The test for empty input should assert the full error
contract: after calling calculateNitrogenBalance(mockFdm, {...}) verify that the
returned object has hasErrors set to true and errorMessage equals the expected
top-level message (in addition to the existing assertion on fieldErrorMessages),
so update the spec to check response.hasErrors and response.errorMessage
alongside response.fieldErrorMessages to prevent regressions in UI behavior.
In `@fdm-calculator/src/balance/organic-matter/index.test.ts`:
- Around line 49-63: The test for calculateOrganicMatterBalance that currently
asserts only fieldErrorMessages should be expanded to also assert the top-level
error flags: call calculateOrganicMatterBalance with empty fields (as in the
diff) and assert that the returned object's hasErrors property is true and that
the top-level errorMessage contains or equals "No fields in input" (in addition
to the existing assertion on fieldErrorMessages) so the UI path that reads
errorMessage is covered.
🪄 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: 6340979f-cbd7-4bee-8e8b-a65e07ba19c4
📒 Files selected for processing (10)
fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.balance.organic-matter._index.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/index.test.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/balance/nitrogen/types.d.tsfdm-calculator/src/balance/organic-matter/index.test.tsfdm-calculator/src/balance/organic-matter/index.tsfdm-calculator/src/balance/organic-matter/types.ts
Collaborator
Author
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 191-202: The current default-selection logic can yield farmIds =
[] (if farmIdsSet stays empty), which hides the empty-input state; after
computing farmIds from farmIdsSet/farmsExtended, add an explicit check for
farmIds.length === 0 and handle it by setting/returning the "No fields in input"
error state (or a dedicated flag) so the rendering code that reads farmResults
will show the empty-input UI instead of proceeding with a normal calculation.
Ensure you update the code paths that use farmResults to honor this new
flag/error and reference farmIdsSet, farmsExtended, farmIds and farmResults when
making the change.
In
`@fdm-app/app/routes/organization`.$slug.$calendar.balance.organic-matter._index.tsx:
- Around line 191-201: The current logic yields farmIds = [] when no query is
present and every accessible farm has zero fields, causing downstream
farmResults = [] and hiding the "empty-input" error; detect this case right
after computing farmIdsSet/farmsExtended (when farmIdsSet.size === 0 &&
farmsExtended.length > 0) and short-circuit by returning an explicit error/flag
instead of producing an empty farmIds array—e.g., throw or return a structured
error Response (with a clear message like "organization has no fields") or set
farmIds to a sentinel (null/undefined plus an error flag) so consumers (the code
that reads farmResults) surface the empty-input error instead of rendering a
blank list. Ensure you update the code paths that reference farmIds/farmResults
to handle this error sentinel/Response accordingly.
🪄 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: cdf54646-33c9-470e-b67d-459e2aa978a2
📒 Files selected for processing (6)
fdm-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/index.test.tsfdm-calculator/src/balance/nitrogen/index.tsfdm-calculator/src/balance/organic-matter/index.test.tsfdm-calculator/src/balance/organic-matter/index.ts
✅ Files skipped from review due to trivial changes (3)
- fdm-calculator/src/balance/nitrogen/index.ts
- fdm-calculator/src/balance/organic-matter/index.test.ts
- fdm-calculator/src/balance/organic-matter/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-calculator/src/balance/nitrogen/index.test.ts
Comment on lines
+191
to
+202
| // Update farmIds selection if it was missing | ||
| if (farmIdsSet.size === 0) { | ||
| for (const farm of farmsExtended) { | ||
| if (farm.hasFields) { | ||
| farmIdsSet.add(farm.b_id_farm) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const farmIds = farmsExtended | ||
| .filter((farm) => farmIdsSet.has(farm.b_id_farm)) | ||
| .map((farm) => farm.b_id_farm) |
Contributor
There was a problem hiding this comment.
Don't let the default filter hide the empty-input error.
If every accessible farm is empty, the default-selection path resolves to farmIds = []. The rest of the page then renders with no farmResults, so the nitrogen organization overview looks like a normal calculation instead of exposing the calculator's "No fields in input" state. Please handle the zero-selected-farms case explicitly.
🤖 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 191 - 202, The current default-selection logic can yield farmIds =
[] (if farmIdsSet stays empty), which hides the empty-input state; after
computing farmIds from farmIdsSet/farmsExtended, add an explicit check for
farmIds.length === 0 and handle it by setting/returning the "No fields in input"
error state (or a dedicated flag) so the rendering code that reads farmResults
will show the empty-input UI instead of proceeding with a normal calculation.
Ensure you update the code paths that use farmResults to honor this new
flag/error and reference farmIdsSet, farmsExtended, farmIds and farmResults when
making the change.
Comment on lines
+191
to
+201
| if (farmIdsSet.size === 0) { | ||
| for (const farm of farmsExtended) { | ||
| if (farm.hasFields) { | ||
| farmIdsSet.add(farm.b_id_farm) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const farmIds = farmsExtended | ||
| .filter((farm) => farmIdsSet.has(farm.b_id_farm)) | ||
| .map((farm) => farm.b_id_farm) |
Contributor
There was a problem hiding this comment.
Handle the all-empty organization case explicitly.
When no farmIds query is present and every accessible farm has zero fields, this branch produces farmIds = []. Downstream that means farmResults = [], so the page falls through to a blank farm list and the organization cards/chart never surface the new empty-input error. Please short-circuit this state here, or make the consumer treat the combined result error as authoritative.
🤖 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 191 - 201, The current logic yields farmIds = [] when no query is
present and every accessible farm has zero fields, causing downstream
farmResults = [] and hiding the "empty-input" error; detect this case right
after computing farmIdsSet/farmsExtended (when farmIdsSet.size === 0 &&
farmsExtended.length > 0) and short-circuit by returning an explicit error/flag
instead of producing an empty farmIds array—e.g., throw or return a structured
error Response (with a clear message like "organization has no fields") or set
farmIds to a sentinel (null/undefined plus an error flag) so consumers (the code
that reads farmResults) surface the empty-input error instead of rendering a
blank list. Ensure you update the code paths that reference farmIds/farmResults
to handle this error sentinel/Response accordingly.
SvenVw
approved these changes
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
Summary by CodeRabbit
Bug Fixes