Skip to content

Show duplicate applications on the same field separately in field and rotation tables#552

Merged
SvenVw merged 12 commits into
mainfrom
hotfix/FDM551
Apr 3, 2026
Merged

Show duplicate applications on the same field separately in field and rotation tables#552
SvenVw merged 12 commits into
mainfrom
hotfix/FDM551

Conversation

@BoraIneviNMI
Copy link
Copy Markdown
Collaborator

@BoraIneviNMI BoraIneviNMI commented Apr 3, 2026

Bug fixes

  • Users found the current fertilizer grouping to be confusing. It wasn't clear what "Aantal percelen" that is greater than the number of fields means, it turned out that those users had duplicate fertilizer applications.
    The changes in this PR ensure that if a field has equivalent fertilizer applications, each of those will be displayed on separate rows.
    Across multiple fields, first similar application on each field is grouped in a single row, then the second similar in each row if they exist on some in another row, then the third in each field in a third row, etc.

Closes #551

Summary by CodeRabbit

Release Notes

  • New Features

    • Fertilizer application table now displays equivalent applications as separate rows when application methods differ.
  • Bug Fixes

    • Fixed handling of missing field data in fertilizer application planning.
    • Improved type safety and test robustness.
  • Chores

    • Updated dependencies and build configuration for enhanced compatibility.

@BoraIneviNMI BoraIneviNMI self-assigned this Apr 3, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0586207a-ef43-460f-8a2e-3056d934e1f0

📥 Commits

Reviewing files that changed from the base of the PR and between a24cf86 and 8d62dca.

📒 Files selected for processing (13)
  • .github/workflows/typecheck.yml
  • fdm-agents/CHANGELOG.md
  • fdm-agents/package.json
  • fdm-agents/src/tools/fertilizer-planner/index.ts
  • fdm-app/CHANGELOG.md
  • fdm-app/package.json
  • fdm-calculator/CHANGELOG.md
  • fdm-calculator/package.json
  • fdm-calculator/src/norms/index.test.ts
  • fdm-core/CHANGELOG.md
  • fdm-core/package.json
  • fdm-core/src/farm.test.ts
  • fdm-core/src/invitation.test.ts
✅ Files skipped from review due to trivial changes (7)
  • fdm-calculator/package.json
  • fdm-core/CHANGELOG.md
  • fdm-core/package.json
  • fdm-agents/CHANGELOG.md
  • fdm-agents/package.json
  • fdm-calculator/CHANGELOG.md
  • fdm-app/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • fdm-app/package.json

📝 Walkthrough

Walkthrough

Patch version updates across multiple packages (fdm-app 0.29.0→0.29.1, fdm-agents 0.2.0→0.2.1, fdm-calculator 0.13.0→0.13.1, fdm-core 0.31.0→0.31.1) with a functional enhancement to fertilizer application table grouping to include application method in similarity calculations, along with test improvements for null safety and build configuration adjustments.

Changes

Cohort / File(s) Summary
Fertilizer Application Table Enhancement
fdm-app/app/components/blocks/fertilizer-applications/table.tsx
Enhanced similarity grouping key to include p_app_method and added new mapBySimilarAndOrder mapper that groups by method/date/amount with occurrence counters for distinct row ordering.
Package Version Updates
fdm-app/package.json, fdm-agents/package.json, fdm-calculator/package.json, fdm-core/package.json
Patch version increments across all packages; fdm-app also updated React Router (^7.14.0), PostHog, proj4, react-hook-form, and validator dependencies.
Build & Configuration
fdm-app/vite.config.ts, package.json, .github/workflows/typecheck.yml
Removed vite-tsconfig-paths plugin in favor of resolve.tsconfigPaths; added @nmi-agro/rvo-connector to pnpm allowlist; expanded typecheck workflow to include fdm-rvo and fdm-agents.
Test Improvements & Null Safety
fdm-agents/src/tools/fertilizer-planner/index.ts, fdm-calculator/src/norms/index.test.ts, fdm-core/src/farm.test.ts, fdm-core/src/invitation.test.ts
Added optional chaining for fieldMetrics in fertilizer planner; reordered TypeScript error-suppression directives; added defensive runtime guards and null checks for invitation queries in farm and invitation tests.
Changelog Updates
fdm-app/CHANGELOG.md, fdm-agents/CHANGELOG.md, fdm-calculator/CHANGELOG.md, fdm-core/CHANGELOG.md
Added corresponding patch release notes documenting feature improvements, bug fixes, and dependency version changes across all packages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

bug, fdm-app, branch:main

Suggested reviewers

  • BoraIneviNMI
  • gerardhros

Poem

🐰 Tables now grouped by method so bright,
No more confusion in rows of delight,
Null guards stand firm where gaps appeared,
Versions all bumped and deps all cleared—
A patch for the fields, hooray hooray! 🥬

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: showing duplicate fertilizer applications separately in tables instead of grouping them together.
Linked Issues check ✅ Passed The PR successfully implements the core requirement from issue #551: duplicate fertilizer applications on the same field are now grouped separately by occurrence order instead of collapsed into single rows.
Out of Scope Changes check ✅ Passed While the PR includes dependency updates and configuration changes (Vite, React Router, package versions), these are supporting changes for compatibility and are reasonable given the scope of the main feature.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/FDM551

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@BoraIneviNMI BoraIneviNMI reopened this Apr 3, 2026
@sentry
Copy link
Copy Markdown

sentry Bot commented Apr 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/fertilizer-applications/table.tsx`:
- Around line 67-70: The row-key generation in mapBySimilarAndOrder (inside
createMapper) uses the full-array index `i`, which breaks grouping when similar
applications are interleaved; change the mapper to compute a
per-similar-occurrence index instead of using `i` — e.g., for each application
passed to the mapping function (reference createMapper, mapBySimilarAndOrder,
createDateKey, and application.p_app_amount), count how many prior items in the
same input array share the same similarity key
(`${createDateKey(application.p_app_date)}#${application.p_app_amount?.toPrecision(4)}`)
and use that count as the ordinal suffix, so keys become
`${dateKey}#${amountKey}#${occurrenceIndex}` ensuring consecutive similar items
get 0,1,2... even when interleaved.
🪄 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: 1b6b4ce4-113d-46e2-a6d4-e3f4329961e2

📥 Commits

Reviewing files that changed from the base of the PR and between 14c0805 and 914f921.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (3)
  • fdm-app/app/components/blocks/fertilizer-applications/table.tsx
  • fdm-rvo/package.json
  • package.json

Comment thread fdm-app/app/components/blocks/fertilizer-applications/table.tsx Outdated
@SvenVw SvenVw changed the base branch from development to main April 3, 2026 09:31
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
fdm-app/package.json (1)

32-33: Consider isolating dependency/toolchain bumps from the hotfix change.

This batch is substantial and unrelated to the fertilizer-table bugfix objective. Splitting these updates into a separate PR would make rollback and incident triage safer if production issues appear.

Also applies to: 63-64, 66-67, 71-71, 74-75, 85-85, 95-96, 109-109

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@fdm-app/package.json` around lines 32 - 33, This PR mixes
dependency/toolchain bumps with the fertilizer-table hotfix; revert the
unrelated package.json dependency changes (e.g., the bumped entries
"@react-router/node" and "@react-router/serve" and the other bumped deps
referenced at ranges 63-64, 66-67, 71, 74-75, 85, 95-96, 109) so the commit only
contains the fertilizer-table bugfix, then open a separate PR that contains the
dependency/toolchain updates (with a clear changelog and CI verification) to
allow isolated rollback and safer triage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@fdm-app/package.json`:
- Around line 32-33: This PR mixes dependency/toolchain bumps with the
fertilizer-table hotfix; revert the unrelated package.json dependency changes
(e.g., the bumped entries "@react-router/node" and "@react-router/serve" and the
other bumped deps referenced at ranges 63-64, 66-67, 71, 74-75, 85, 95-96, 109)
so the commit only contains the fertilizer-table bugfix, then open a separate PR
that contains the dependency/toolchain updates (with a clear changelog and CI
verification) to allow isolated rollback and safer triage.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3a019ae8-e52c-4a90-a194-6ece7c183e0e

📥 Commits

Reviewing files that changed from the base of the PR and between 24a2010 and a24cf86.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (4)
  • .changeset/quick-eggs-join.md
  • fdm-app/app/components/blocks/fertilizer-applications/table.tsx
  • fdm-app/package.json
  • fdm-app/vite.config.ts
✅ Files skipped from review due to trivial changes (1)
  • .changeset/quick-eggs-join.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • fdm-app/app/components/blocks/fertilizer-applications/table.tsx

@SvenVw SvenVw merged commit 0787dc5 into main Apr 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In the rotation fertilizer application table, duplicate applications on the same field should be grouped separately

2 participants