Skip to content

Store SECTORID as b_id_source in RVO uploads#372

Merged
SvenVw merged 2 commits into
mainfrom
hotfix/FDM371
Dec 10, 2025
Merged

Store SECTORID as b_id_source in RVO uploads#372
SvenVw merged 2 commits into
mainfrom
hotfix/FDM371

Conversation

@SvenVw
Copy link
Copy Markdown
Collaborator

@SvenVw SvenVw commented Dec 9, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Corrected field source identifier handling in farm shapefile uploads. SECTORID values are now properly stored as field source references when uploading RVO MijnPercelen data.

✏️ Tip: You can customize this high-level summary in your review settings.

closes #371

@SvenVw SvenVw self-assigned this Dec 9, 2025
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Dec 9, 2025

⚠️ No Changeset found

Latest commit: a04a372

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 9, 2025

👋 Hotfix Branch PR Detected!

Before merging this Pull Request into main, please ensure you have finalized the hotfix by manually running the 'Release' workflow on this hotfix/FDM371 branch.

This will:

  1. Bump package versions.
  2. Generate changelogs.
  3. Create Git tags.

You can trigger the workflow from the 'Actions' tab, selecting the 'Release' workflow, and choosing this hotfix/FDM371 branch.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 9, 2025

Walkthrough

This PR updates the RVO Mijn Percelen shapefile upload to store the SECTORID attribute as b_id_source, enabling future synchronization with the RVO webservice. The SECTORID type is changed from number to string, and the field creation now uses this identifier instead of null.

Changes

Cohort / File(s) Summary
Upload Field Identifier Mapping
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
Changed RvoProperties SECTORID type from number to string; updated field creation to pass b_id_source as SECTORID instead of null
Version & Documentation Updates
fdm-app/package.json
fdm-app/CHANGELOG.md
Version bumped from 0.25.2 to 0.25.3; CHANGELOG entry added documenting SECTORID storage as b_id_source for RVO MijnPercelen shapefile uploads

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Type change from number to string is straightforward
  • Single parameter modification in field creation call
  • Version and documentation updates are trivial
  • Changes are focused and cohesive around one feature

Areas for extra attention:

  • Verify SECTORID values from RVO Mijn Percelen shapefile are consistently strings
  • Confirm downstream code using b_id_source handles string values correctly

Possibly related PRs

Suggested reviewers

  • gerardhros

Poem

🐰 A shapefile's secret revealed today,
SECTORID finds its rightful way,
From null to source, the ID takes flight,
Future sync now burns so bright,
Numbers become strings—hop, skip, delight! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: storing SECTORID as b_id_source in RVO uploads, matching the primary objective.
Linked Issues check ✅ Passed The code changes directly implement the requirement from issue #371 by changing SECTORID type from number to string and using b_id_source to store the SECTORID value during RVO Mijn Percelen shapefile uploads [#371].
Out of Scope Changes check ✅ Passed All changes are within scope: SECTORID type modification, b_id_source integration, CHANGELOG update, and version bump are all directly related to the issue objective.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch hotfix/FDM371

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 598a3b6 and a04a372.

📒 Files selected for processing (2)
  • fdm-app/CHANGELOG.md (1 hunks)
  • fdm-app/package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: .changeset/yellow-sheep-wash.md:1-5
Timestamp: 2025-01-09T16:06:08.547Z
Learning: The `b_sector` column has been removed from the farms table and replaced with `b_businessid_farm`, `b_address_farm`, and `b_postalcode_farm` columns for better business information management.
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.

Applied to files:

  • fdm-app/package.json
  • fdm-app/CHANGELOG.md
📚 Learning: 2024-11-25T12:42:32.783Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.

Applied to files:

  • fdm-app/package.json
📚 Learning: 2025-08-13T10:33:05.313Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-08-13T10:33:05.313Z
Learning: In the fdm project, fdm-calculator integration for new features like b_lu_variety is handled in separate updates from the core data model changes. When fdm-core functions are updated to support new fields, fdm-calculator can consume these enhanced APIs without requiring changes in the same PR that introduces the core functionality.

Applied to files:

  • fdm-app/package.json
  • fdm-app/CHANGELOG.md
📚 Learning: 2025-01-31T15:05:14.310Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 67
File: fdm-app/app/routes/farm.create.$b_id_farm.fields.$b_id.tsx:601-610
Timestamp: 2025-01-31T15:05:14.310Z
Learning: The `updateField` function in fdm-core has optional parameters after `fdm` and `b_id`. The TypeScript definitions might show 8 required parameters due to a potential version mismatch.

Applied to files:

  • fdm-app/package.json
🔇 Additional comments (3)
fdm-app/package.json (1)

3-3: Version bump appropriate for patch release.

Incrementing the patch version is correct for a bug fix that stores SECTORID as b_id_source.

fdm-app/CHANGELOG.md (2)

3-7: Changelog entry is clear and well-formatted.

The patch entry accurately summarizes the SECTORID storage improvement for RVO Mijn Percelen uploads.


1-117: No verification issues found with the SECTORID implementation in this PR.

The changelog entry accurately describes the code changes: the shapefile upload route (farm.create.$b_id_farm.$calendar.upload.tsx) stores SECTORID as b_id_source when creating fields. The implementation is correct:

  • SECTORID is already defined as string type in the RvoProperties interface (line 92)
  • The b_id_source parameter in the addField API accepts text/string values
  • SECTORID is properly assigned to b_id_source and passed to addField (line 228, 235)

The addField function in fdm-core creates the field and all required relations (field data, acquiring info, discarding info) within a single transaction, as expected.


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.

@coderabbitai coderabbitai Bot changed the title @coderabbitai Store SECTORID as b_id_source in RVO uploads Dec 9, 2025
@coderabbitai coderabbitai Bot added branch:main An issue, affecting the main branch, that requires an hotfix bug Something isn't working fdm-app labels Dec 9, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.62%. Comparing base (6a8a7e9) to head (598a3b6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #372   +/-   ##
=======================================
  Coverage   87.62%   87.62%           
=======================================
  Files          79       79           
  Lines        3959     3959           
  Branches     1145     1145           
=======================================
  Hits         3469     3469           
  Misses        490      490           
Flag Coverage Δ
fdm-calculator 87.81% <ø> (ø)
fdm-core 87.08% <ø> (ø)
fdm-data 92.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SvenVw SvenVw requested a review from gerardhros December 9, 2025 14:15
Copy link
Copy Markdown
Collaborator

@gerardhros gerardhros left a comment

Choose a reason for hiding this comment

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

LGTM.

@SvenVw SvenVw merged commit 8e788fe into main Dec 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch:main An issue, affecting the main branch, that requires an hotfix bug Something isn't working fdm-app

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants