Skip to content

Make shapefile upload less confusing#264

Closed
BoraIneviNMI wants to merge 3 commits into
developmentfrom
FDM248
Closed

Make shapefile upload less confusing#264
BoraIneviNMI wants to merge 3 commits into
developmentfrom
FDM248

Conversation

@BoraIneviNMI
Copy link
Copy Markdown
Collaborator

@BoraIneviNMI BoraIneviNMI commented Sep 16, 2025

  • Enhancements
    • When an user uploads their shapefile obtained from mijnpercelen, they are now redirected to a different year in case the farm doesn't have any fields from the current year still.
    • There is a new year selection menu in the farm creation fields page in case the farm has fields from multiple years.

Summary by CodeRabbit

  • New Features

    • Added a year selection dropdown on the farm creation fields page, shown when multiple years are available; defaults to the current year.
    • After shapefile upload, the app automatically redirects to the nearest available year if the selected year has no fields and informs the user; shows an error if no parcels were imported.
  • Chores

    • Bumped app and core packages to new minor versions.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Sep 16, 2025

🦋 Changeset detected

Latest commit: 021c257

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@svenvw/fdm-core Minor
@svenvw/fdm-app Minor
@svenvw/fdm-calculator Patch

Not sure what this means? Click here to learn what changesets are.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 16, 2025

Walkthrough

Adds calendar-year awareness across core and app: new getCalendarYears API in fdm-core and export; loader/UI changes to show a year dropdown on farm creation fields; upload flow records available years and redirects to a suitable year when needed; two changeset entries bump app and core.

Changes

Cohort / File(s) Summary
Release changesets
./.changeset/polite-falcons-sniff.md, ./.changeset/ten-days-joke.md
Adds minor bumps for @svenvw/fdm-core and @svenvw/fdm-app; documents UI year selector and upload-time year-redirect behavior.
Core: calendar years API
fdm-core/src/farm.ts, fdm-core/src/index.ts
Introduces and exports getCalendarYears(fdm, principal_id, b_id_farm): Promise<string[]>, querying distinct field start years with permission checks and error handling.
App route: fields view
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
Loader fetches calendarYears; UI renders a year Select when multiple years exist; selecting navigates to /farm/create/{b_id_farm}/{year}/fields. Loader return type includes calendarYears: string[].
App route: upload flow
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
Tracks imported parcel years; if none, redirects with error; if requested year missing, redirects to nearest available year with success message; otherwise proceeds with normal success redirect.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as Fields Page (App)
  participant Loader as Route Loader
  participant Core as fdm-core.getCalendarYears
  participant DB as DB

  UI->>Loader: load(b_id_farm, calendar)
  Loader->>Core: getCalendarYears(fdm, principal_id, b_id_farm)
  Core->>DB: SELECT DISTINCT b_start WHERE farm = b_id_farm
  DB-->>Core: dates[]
  Core-->>Loader: ["2022","2023","2024"]
  Loader-->>UI: { fields, calendar, calendarYears }
  Note over UI: If >1 year, render dropdown
  UI->>UI: On select year Y
  UI->>UI: navigate /farm/create/{b_id_farm}/{Y}/fields
Loading
sequenceDiagram
  autonumber
  participant User as User
  participant Upload as Upload Action
  participant Proc as Feature Processor
  participant Router as Redirect

  User->>Upload: Submit shapefile (calendar=C)
  Upload->>Proc: Parse features
  Proc-->>Upload: years found = {Ymin..Ymax}
  alt No parcels imported
    Upload->>Router: redirectWithError(/fields for C)
  else Requested C not in years
    Upload->>Upload: redirectCalendar = clamp(C, [Ymin,Ymax])
    Upload->>Router: redirectWithSuccess(/fields for redirectCalendar)
  else C available
    Upload->>Router: redirectWithSuccess(/fields for C)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

fdm-app, fdm-core, branch:development

Suggested reviewers

  • SvenVw
  • gerardhros

Poem

A rabbit flips the calendar with glee,
Fields hop from ’22 to ’23.
If the year you seek has gone astray,
I’ll nudge you to a nearby day.
Dropdown dreams and shapefile cheer—
We’re planting rows, year after year. 🌱🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Make shapefile upload less confusing" is concise and directly reflects the primary user-facing change (improving shapefile upload behavior and redirects), so it clearly communicates the main intent to reviewers; it does not mention the added year-selector UI but remains appropriately focused and readable for a PR title.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch FDM248

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 16, 2025

Codecov Report

❌ Patch coverage is 0% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.57%. Comparing base (e5e79f6) to head (021c257).

Files with missing lines Patch % Lines
fdm-core/src/farm.ts 0.00% 28 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #264      +/-   ##
===============================================
- Coverage        92.77%   92.57%   -0.20%     
===============================================
  Files               87       87              
  Lines            13030    13058      +28     
  Branches          1289     1289              
===============================================
  Hits             12089    12089              
- Misses             939      967      +28     
  Partials             2        2              
Flag Coverage Δ
fdm-calculator 94.15% <ø> (ø)
fdm-core 91.37% <0.00%> (-0.35%) ⬇️
fdm-data 94.37% <ø> (ø)

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.

@coderabbitai coderabbitai Bot added branch:development Issue only affecting development, not the main branch (yet) fdm-app fdm-core labels Sep 16, 2025
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: 2

🧹 Nitpick comments (7)
.changeset/ten-days-joke.md (1)

6-6: Fix wording and casing in the changeset note.

Grammatical nit and product name casing.

-When an user uploads their shapefile obtained from mijnpercelen, they are now redirected to a different year in case the farm doesn't have any fields from the current year still.
+When a user uploads a shapefile from MijnPercelen, the app redirects to a different year if the farm has no fields in the current year.
.changeset/polite-falcons-sniff.md (1)

6-6: Tighten phrasing and use consistent “dropdown”.

Pure copy edit.

-There is a new year selection drop-down in the farm creation fields page in case the farm has fields from multiple years.
+Adds a year selection dropdown on the farm creation fields page for farms with fields in multiple years.
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx (2)

177-181: Rename Select name to reflect value.

This isn’t a form submit, but if you keep name, prefer “calendar” over “role”.

-                                        <Select
-                                            defaultValue={loaderData.calendar}
-                                            name="role"
-                                            onValueChange={handleCalendarSelect}
-                                        >
+                                        <Select
+                                            defaultValue={loaderData.calendar}
+                                            name="calendar"
+                                            onValueChange={handleCalendarSelect}
+                                        >

189-199: Defensively dedupe and sort years to avoid duplicate options/keys.

If the API ever returns duplicates, this prevents UI duplication and React key warnings.

-                                                {loaderData.calendarYears.map(
-                                                    (year) => (
-                                                        <SelectItem
-                                                            key={year}
-                                                            value={year}
-                                                        >
-                                                            {year}
-                                                        </SelectItem>
-                                                    ),
-                                                )}
+                                                {Array.from(new Set(loaderData.calendarYears))
+                                                    .sort()
+                                                    .map((year) => (
+                                                        <SelectItem key={year} value={year}>
+                                                            {year}
+                                                        </SelectItem>
+                                                    ))}
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx (3)

183-186: Type the Set for clarity and correctness.

Makes intent explicit and prevents accidental mixing of strings/numbers.

-        const calendarYears = new Set()
+        const calendarYears = new Set<number>()
         let minCalendarYear = Number.POSITIVE_INFINITY
         let maxCalendarYear = 0

268-271: Use UTC year to avoid timezone off‑by‑one.

Also standardize the Set to numbers.

-            calendarYears.add(b_start.getFullYear().toString())
-            minCalendarYear = Math.min(minCalendarYear, b_start.getFullYear())
-            maxCalendarYear = Math.max(maxCalendarYear, b_start.getFullYear())
+            const y = b_start.getUTCFullYear()
+            calendarYears.add(y)
+            minCalendarYear = Math.min(minCalendarYear, y)
+            maxCalendarYear = Math.max(maxCalendarYear, y)

273-281: Likely unreachable branch.

Given earlier validation and loop, calendarYears.size === 0 seems impossible if features exist. Safe to keep, but consider removing to simplify.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5e79f6 and 021c257.

📒 Files selected for processing (6)
  • .changeset/polite-falcons-sniff.md (1 hunks)
  • .changeset/ten-days-joke.md (1 hunks)
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx (6 hunks)
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx (3 hunks)
  • fdm-core/src/farm.ts (1 hunks)
  • fdm-core/src/index.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
PR: SvenVw/fdm#42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A shared layout component `FarmLayoutBase` has been created in `components/custom/farm-layout-base.tsx` to maintain consistency across farm-related pages. The component handles farm selection dropdown, breadcrumb navigation, and provides a common layout structure.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
PR: SvenVw/fdm#233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
PR: SvenVw/fdm#124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
PR: SvenVw/fdm#16
File: fdm-app/app/routes/app.addfarm.$b_id_farm.cultivations.$b_lu_catalogue.fertilizers.tsx:1-1
Timestamp: 2024-12-16T10:56:07.561Z
Learning: The project uses `react-router` v7, and the `data` function is exported and used for error handling in loaders and actions.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
PR: SvenVw/fdm#42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The `FarmLayout` component in `components/custom/farm-layout.tsx` provides a reusable layout structure for farm-related pages, with support for farm selection dropdown, customizable breadcrumb titles, and flexible content rendering through either children or Outlet components.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
PR: SvenVw/fdm#138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
PR: SvenVw/fdm#138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
PR: SvenVw/fdm#138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx
  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
PR: SvenVw/fdm#151
File: fdm-app/app/routes/signin._index.tsx:101-101
Timestamp: 2025-06-02T10:31:27.097Z
Learning: In fdm-app/app/routes/signin._index.tsx, the redirect destinations are intentionally inconsistent by design: the component defaults new sign-ins to "/welcome" (line 101) while the loader redirects authenticated users to "/farm" (line 80) and the action uses "/farm" as fallback (line 434). This creates appropriate user flows where new users complete their profile via the welcome page, while existing authenticated users bypass it and go directly to the main application.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2025-01-14T16:06:24.294Z
Learnt from: SvenVw
PR: SvenVw/fdm#45
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:1-1
Timestamp: 2025-01-14T16:06:24.294Z
Learning: In the fdm-app codebase, the `redirect` function should be imported from `react-router`, not `react-router-dom`.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
📚 Learning: 2025-01-14T16:06:21.832Z
Learnt from: SvenVw
PR: SvenVw/fdm#45
File: fdm-app/app/routes/farm.$b_id_farm.settings._index.tsx:1-1
Timestamp: 2025-01-14T16:06:21.832Z
Learning: In the fdm project, `redirect` and other routing utilities should be imported from `react-router` instead of `react-router-dom`.

Applied to files:

  • fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
🧬 Code graph analysis (2)
fdm-core/src/farm.ts (2)
fdm-core/src/index.ts (2)
  • getCalendarYears (60-60)
  • FdmType (70-70)
fdm-core/src/authorization.ts (1)
  • checkPermission (156-240)
fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fields.tsx (2)
fdm-core/src/farm.ts (1)
  • getCalendarYears (87-116)
fdm-core/src/index.ts (1)
  • getCalendarYears (60-60)
🔇 Additional comments (1)
fdm-core/src/index.ts (1)

60-60: LGTM: public export added.

Comment on lines +282 to 296
if (!calendarYears.has(calendar)) {
try {
const redirectCalendar = Math.max(
minCalendarYear,
Math.min(maxCalendarYear, Number.parseInt(calendar)),
)

return redirectWithSuccess(
`/farm/create/${b_id_farm}/${redirectCalendar}/fields`,
{
message: `Geen percelen in ${calendar} zijn geïmporteerd. Je bekijkt nu de percelen in ${redirectCalendar}.`,
},
)
} catch (_) {}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Clamping picks unavailable years; choose the nearest available year instead.

Current logic clamps to [min,max] but may still select a year that has no parcels (e.g., available {2018,2020,2022} and requested 2021 → 2021). Pick the closest available year.

Apply this diff:

-        if (!calendarYears.has(calendar)) {
-            try {
-                const redirectCalendar = Math.max(
-                    minCalendarYear,
-                    Math.min(maxCalendarYear, Number.parseInt(calendar)),
-                )
-
-                return redirectWithSuccess(
-                    `/farm/create/${b_id_farm}/${redirectCalendar}/fields`,
-                    {
-                        message: `Geen percelen in ${calendar} zijn geïmporteerd. Je bekijkt nu de percelen in ${redirectCalendar}.`,
-                    },
-                )
-            } catch (_) {}
-        }
+        if (!calendarYears.has(Number.parseInt(calendar, 10))) {
+            const years = Array.from(calendarYears).sort((a, b) => a - b)
+            const req = Number.parseInt(calendar, 10)
+            // Prefer the latest year <= requested; otherwise the earliest >= requested; fallback to most recent
+            const lower = years.filter((y) => y <= req).pop()
+            const higher = years.find((y) => y >= req)
+            const redirectCalendar = lower ?? higher ?? years[years.length - 1]
+
+            return redirectWithSuccess(
+                `/farm/create/${b_id_farm}/${redirectCalendar}/fields`,
+                {
+                    message: `Geen percelen in ${calendar} zijn geïmporteerd. Je bekijkt nu de percelen in ${redirectCalendar}.`,
+                },
+            )
+        }
📝 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
if (!calendarYears.has(calendar)) {
try {
const redirectCalendar = Math.max(
minCalendarYear,
Math.min(maxCalendarYear, Number.parseInt(calendar)),
)
return redirectWithSuccess(
`/farm/create/${b_id_farm}/${redirectCalendar}/fields`,
{
message: `Geen percelen in ${calendar} zijn geïmporteerd. Je bekijkt nu de percelen in ${redirectCalendar}.`,
},
)
} catch (_) {}
}
if (!calendarYears.has(Number.parseInt(calendar, 10))) {
const years = Array.from(calendarYears).sort((a, b) => a - b)
const req = Number.parseInt(calendar, 10)
// Prefer the latest year <= requested; otherwise the earliest >= requested; fallback to most recent
const lower = years.filter((y) => y <= req).pop()
const higher = years.find((y) => y >= req)
const redirectCalendar = lower ?? higher ?? years[years.length - 1]
return redirectWithSuccess(
`/farm/create/${b_id_farm}/${redirectCalendar}/fields`,
{
message: `Geen percelen in ${calendar} zijn geïmporteerd. Je bekijkt nu de percelen in ${redirectCalendar}.`,
},
)
}
🤖 Prompt for AI Agents
In fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx around lines
282-296, the code currently clamps the requested year to min/max which can still
select a year that has no parcels; instead compute the numeric requested year,
iterate the available calendarYears to find the single year with the smallest
absolute difference (tie-breaker: prefer the larger or smaller year? pick the
nearer; if equal choose the lower or higher consistently — pick the lower for
determinism), use that nearest available year as redirectCalendar, and return
redirectWithSuccess to `/farm/create/${b_id_farm}/${redirectCalendar}/fields`
with the same message; if calendarYears is empty fallback to the existing clamp
behavior or abort gracefully; remove the silent empty catch (or at least log the
error) so failures are visible.

Comment thread fdm-core/src/farm.ts
Comment on lines +75 to +116
/**
* Retrieves the calendar years during which the farm has fields, after verifying that the requesting principal has read access.
*
* This function checks the principal's permissions before querying the database for the farm identified by the provided ID.
*
* @param fdm The FDM instance providing the connection to the database. The instance can be created with {@link createFdmServer}.
* @param principal_id - The identifier of the principal making the request.
* @param b_id_farm - The unique identifier of the farm to retrieve.
* @returns A Promise that resolves with a string array of calendar years.
* @throws {Error} If permission checks fail or if an error occurs while retrieving the calendar years.
* @alpha
*/
export async function getCalendarYears(
fdm: FdmType,
principal_id: string,
b_id_farm: string,
): Promise<string[]> {
try {
return await fdm.transaction(async (tx: FdmType) => {
await checkPermission(
tx,
"farm",
"read",
b_id_farm,
principal_id,
"getCalendarYears",
)

const results = await tx
.selectDistinct({ b_start: schema.fieldAcquiring.b_start })
.from(schema.fieldAcquiring)
.where(eq(schema.fieldAcquiring.b_id_farm, b_id_farm))
.orderBy(asc(schema.fieldAcquiring.b_start))

return results.map(({ b_start }) =>
b_start.getFullYear().toString(),
)
})
} catch (err) {
throw handleError(err, "Exception for getCalendarYears")
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure unique, sorted years and avoid timezone drift.

Current query selects distinct b_start timestamps, then maps to years. Different dates within the same year will produce duplicate years; also .getFullYear() is locale‑dependent. Use UTC and dedupe/sort years before returning.

Apply this diff:

-            const results = await tx
-                .selectDistinct({ b_start: schema.fieldAcquiring.b_start })
-                .from(schema.fieldAcquiring)
-                .where(eq(schema.fieldAcquiring.b_id_farm, b_id_farm))
-                .orderBy(asc(schema.fieldAcquiring.b_start))
-
-            return results.map(({ b_start }) =>
-                b_start.getFullYear().toString(),
-            )
+            const results = await tx
+                .selectDistinct({ b_start: schema.fieldAcquiring.b_start })
+                .from(schema.fieldAcquiring)
+                .where(eq(schema.fieldAcquiring.b_id_farm, b_id_farm))
+
+            const years = Array.from(
+                new Set(results.map(({ b_start }) => b_start.getUTCFullYear())),
+            ).sort((a, b) => a - b)
+
+            return years.map(String)

Also add minimal context to the error for parity with other methods:

-    } catch (err) {
-        throw handleError(err, "Exception for getCalendarYears")
-    }
+    } catch (err) {
+        throw handleError(err, "Exception for getCalendarYears", {
+            b_id_farm,
+            principal_id,
+        })
+    }
📝 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
/**
* Retrieves the calendar years during which the farm has fields, after verifying that the requesting principal has read access.
*
* This function checks the principal's permissions before querying the database for the farm identified by the provided ID.
*
* @param fdm The FDM instance providing the connection to the database. The instance can be created with {@link createFdmServer}.
* @param principal_id - The identifier of the principal making the request.
* @param b_id_farm - The unique identifier of the farm to retrieve.
* @returns A Promise that resolves with a string array of calendar years.
* @throws {Error} If permission checks fail or if an error occurs while retrieving the calendar years.
* @alpha
*/
export async function getCalendarYears(
fdm: FdmType,
principal_id: string,
b_id_farm: string,
): Promise<string[]> {
try {
return await fdm.transaction(async (tx: FdmType) => {
await checkPermission(
tx,
"farm",
"read",
b_id_farm,
principal_id,
"getCalendarYears",
)
const results = await tx
.selectDistinct({ b_start: schema.fieldAcquiring.b_start })
.from(schema.fieldAcquiring)
.where(eq(schema.fieldAcquiring.b_id_farm, b_id_farm))
.orderBy(asc(schema.fieldAcquiring.b_start))
return results.map(({ b_start }) =>
b_start.getFullYear().toString(),
)
})
} catch (err) {
throw handleError(err, "Exception for getCalendarYears")
}
}
/**
* Retrieves the calendar years during which the farm has fields, after verifying that the requesting principal has read access.
*
* This function checks the principal's permissions before querying the database for the farm identified by the provided ID.
*
* @param fdm The FDM instance providing the connection to the database. The instance can be created with {@link createFdmServer}.
* @param principal_id - The identifier of the principal making the request.
* @param b_id_farm - The unique identifier of the farm to retrieve.
* @returns A Promise that resolves with a string array of calendar years.
* @throws {Error} If permission checks fail or if an error occurs while retrieving the calendar years.
* @alpha
*/
export async function getCalendarYears(
fdm: FdmType,
principal_id: string,
b_id_farm: string,
): Promise<string[]> {
try {
return await fdm.transaction(async (tx: FdmType) => {
await checkPermission(
tx,
"farm",
"read",
b_id_farm,
principal_id,
"getCalendarYears",
)
const results = await tx
.selectDistinct({ b_start: schema.fieldAcquiring.b_start })
.from(schema.fieldAcquiring)
.where(eq(schema.fieldAcquiring.b_id_farm, b_id_farm))
const years = Array.from(
new Set(results.map(({ b_start }) => b_start.getUTCFullYear())),
).sort((a, b) => a - b)
return years.map(String)
})
} catch (err) {
throw handleError(err, "Exception for getCalendarYears", {
b_id_farm,
principal_id,
})
}
}
🤖 Prompt for AI Agents
In fdm-core/src/farm.ts around lines 75 to 116, the function returns distinct
b_start timestamps which can map to duplicate years and .getFullYear() can be
affected by local timezone; change the logic to extract the UTC year (e.g.,
using getUTCFullYear or equivalent) from each b_start, collect the years into a
Set to dedupe, convert to an array, sort ascending, and return that array of
unique string years; also include minimal context in the thrown error (e.g.,
"Exception for getCalendarYears: ") when rethrowing via handleError so it
parallels other methods.

@BoraIneviNMI
Copy link
Copy Markdown
Collaborator Author

This was not the problem. Shapefile upload has to add cultivations to existing fields if possible. The getFields query might be wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch:development Issue only affecting development, not the main branch (yet) fdm-app fdm-core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After uploading shapefile with many fields, not all fields look to be present in fields overview at farm create page

1 participant