Skip to content

Add default cultivation dates: schema fields, core util, app routes #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SvenVw merged 18 commits into from
Oct 16, 2025
Merged

Add default cultivation dates: schema fields, core util, app routes #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c3c96e3
refactor: use correct loop
SvenVw Oct 9, 2025
a226f7e
feat: adds b_lu_start_default and b_date_harvest_default
SvenVw Oct 9, 2025
97083dd
feat: adds `b_lu_start_default` and `b_date_harvest_default` to brp c…
SvenVw Oct 9, 2025
2f7b281
feat: adds getDefaultDatesOfCultivation as helper function to determi…
SvenVw Oct 9, 2025
250987e
tests: fixes
SvenVw Oct 9, 2025
d279d08
feat: for cultivations default dates based on the cultivation catalog…
SvenVw Oct 9, 2025
afc85b7
fix: winter crops should be sown previous year instead of harvest nex…
SvenVw Oct 10, 2025
945f752
refactor: combine changesets
SvenVw Oct 10, 2025
c3c8e41
fix: typo
SvenVw Oct 10, 2025
a4268f7
refactor: validate date format
SvenVw Oct 10, 2025
c35663a
refactor: use fixed default dates if not provided
SvenVw Oct 10, 2025
95309db
fix: add validation for year
SvenVw Oct 10, 2025
b90c1d2
refactor: add check to db for format of default dates
SvenVw Oct 10, 2025
cc91da2
nitpicks
SvenVw Oct 10, 2025
45c245d
Merge branch 'development' into FDM290
SvenVw Oct 10, 2025
a38d207
chore: remove migration to merge dev branch
SvenVw Oct 16, 2025
d2c501d
Merge remote-tracking branch 'origin/development' into FDM290
SvenVw Oct 16, 2025
d30a616
chore: add db migration
SvenVw Oct 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/big-cars-like.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@svenvw/fdm-data": minor
---

Adds `b_lu_start_default` and `b_date_harvest_default` to brp catalogue
5 changes: 5 additions & 0 deletions .changeset/cold-jars-marry.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@svenvw/fdm-core": minor
---

Adds `b_lu_start_default` and `b_date_harvest_default` to cultivations catalogue as the default start and harvest dates of a cultivation
5 changes: 5 additions & 0 deletions .changeset/lovely-mammals-sneeze.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@svenvw/fdm-core": minor
---

Adds getDefaultDatesOfCultivation as helper function to determine default dates for cultivations
5 changes: 5 additions & 0 deletions .changeset/proud-banks-train.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@svenvw/fdm-app": minor
---

For cultivations default dates based on the cultivation catalogue date are used for b_lu_start and b_lu_end if available when adding a field with a cultivation
20 changes: 13 additions & 7 deletions fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
addField,
addSoilAnalysis,
getCultivationsFromCatalogue,
getDefaultDatesOfCultivation,
getFarm,
getFarms,
getFields,
Expand Down Expand Up @@ -417,13 +418,18 @@ export async function action({ request, params }: ActionFunctionArgs) {
const b_geometry = JSON.parse(
JSON.parse(String(formValues.b_geometry)),
) as Polygon
const currentYear = new Date().getFullYear()
const defaultDate = timeframe.start
? timeframe.start
: `${currentYear}-01-01`
const b_start = defaultDate
const b_lu_start = defaultDate
const b_lu_end = undefined
const currentYear = Number(calendar)

const cultivationDefaultDates = await getDefaultDatesOfCultivation(
fdm,
session.principal_id,
b_id_farm,
b_lu_catalogue,
currentYear,
)
const b_start = new Date(`${currentYear}-01-01`)
const b_lu_start = cultivationDefaultDates.b_lu_start
const b_lu_end = cultivationDefaultDates.b_lu_end
const b_end = undefined
Comment on lines +421 to 433
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add guards for calendar year and invalid default dates

  • Validate Number(calendar) yields an integer.
  • Ensure b_lu_start/b_lu_end are valid Dates before calling addCultivation.

Apply this diff:

-        const currentYear = Number(calendar)
+        const currentYear = Number(calendar)
+        if (!Number.isInteger(currentYear)) {
+            throw new Error(`invalid calendar year: ${calendar}`)
+        }

         const cultivationDefaultDates = await getDefaultDatesOfCultivation(
             fdm,
             session.principal_id,
             b_id_farm,
             b_lu_catalogue,
             currentYear,
         )
         const b_start = new Date(`${currentYear}-01-01`)
-        const b_lu_start = cultivationDefaultDates.b_lu_start
-        const b_lu_end = cultivationDefaultDates.b_lu_end
+        const b_lu_start = cultivationDefaultDates.b_lu_start
+        const b_lu_end = cultivationDefaultDates.b_lu_end
+        if (Number.isNaN(b_lu_start.getTime())) {
+            throw new Error(`Invalid default start date for cultivation ${b_lu_catalogue}`)
+        }
+        if (b_lu_end && Number.isNaN(b_lu_end.getTime())) {
+            throw new Error(`Invalid default end date for cultivation ${b_lu_catalogue}`)
+        }
📝 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
const currentYear = Number(calendar)
const cultivationDefaultDates = await getDefaultDatesOfCultivation(
fdm,
session.principal_id,
b_id_farm,
b_lu_catalogue,
currentYear,
)
const b_start = new Date(`${currentYear}-01-01`)
const b_lu_start = cultivationDefaultDates.b_lu_start
const b_lu_end = cultivationDefaultDates.b_lu_end
const b_end = undefined
const currentYear = Number(calendar)
if (!Number.isInteger(currentYear)) {
throw new Error(`invalid calendar year: ${calendar}`)
}
const cultivationDefaultDates = await getDefaultDatesOfCultivation(
fdm,
session.principal_id,
b_id_farm,
b_lu_catalogue,
currentYear,
)
const b_start = new Date(`${currentYear}-01-01`)
const b_lu_start = cultivationDefaultDates.b_lu_start
const b_lu_end = cultivationDefaultDates.b_lu_end
if (Number.isNaN(b_lu_start.getTime())) {
throw new Error(`Invalid default start date for cultivation ${b_lu_catalogue}`)
}
if (b_lu_end && Number.isNaN(b_lu_end.getTime())) {
throw new Error(`Invalid default end date for cultivation ${b_lu_catalogue}`)
}
const b_end = undefined

Copy link
Copy Markdown
Collaborator Author

@SvenVw SvenVw Oct 10, 2025

Choose a reason for hiding this comment

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

Why? The function getCalendar should return a calendar year and getDefaultDatesOfCultivation will return date objects

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 10, 2025

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and examine the getCalendar function implementation
echo "=== Searching for getCalendar function ==="
ast-grep --pattern 'export function getCalendar($$$) { $$$ }'

echo -e "\n=== Searching for getCalendar with alternate patterns ==="
rg -nP --type=ts -C5 'function getCalendar'

echo -e "\n=== Finding getCalendar in imports ==="
rg -nP --type=ts 'import.*getCalendar'

Length of output: 5433

🏁 Script executed:

#!/bin/bash
# Examine the getDefaultDatesOfCultivation function implementation
echo "=== Examining getDefaultDatesOfCultivation implementation ==="
ast-grep --pattern 'export async function getDefaultDatesOfCultivation($$$) { $$$ }'

echo -e "\n=== Alternative search for getDefaultDatesOfCultivation ==="
rg -nP --type=ts -A30 'async function getDefaultDatesOfCultivation'

Length of output: 2258

🏁 Script executed:

#!/bin/bash
# Get the complete implementation of getDefaultDatesOfCultivation
echo "=== Full getDefaultDatesOfCultivation implementation ==="
rg -nP --type=ts -A100 'export async function getDefaultDatesOfCultivation' fdm-core/src/cultivation.ts | head -120

Length of output: 4152

🏁 Script executed:

#!/bin/bash
# Check the CultivationDefaultDates type definition
echo "=== CultivationDefaultDates type definition ==="
rg -nP --type=ts -C3 'type CultivationDefaultDates'

Length of output: 447

@SvenVw, I understand your point. Let me clarify what I found by examining the actual implementations:

getCalendar currently just returns the route parameter as-is:

export function getCalendar(params: Params): string {
    const calendar = params.calendar as string
    return calendar
}

It performs no validation that the value is actually a valid year. So Number(calendar) could theoretically produce NaN or a non-integer if the route param is malformed.

getDefaultDatesOfCultivation does construct Date objects:

cultivationDefaultDates.b_lu_start = new Date(
    `${year}-${cultivationsCatalogue[0].b_lu_start_default}`
)

However, it doesn't validate that the resulting Date is valid. JavaScript's new Date() with invalid input (like "2025-13-32") creates an Invalid Date object where getTime() returns NaN.

So while your assumptions about what these functions should do are reasonable, the current implementations don't actually enforce those guarantees. The validation catches:

  1. Non-numeric or malformed calendar params
  2. Invalid Date objects from malformed catalogue data

That said, if you're confident the route params and catalogue data are always well-formed in practice, you can skip these checks. The decision is yours!

🧬 Code Graph Analysis Results

fdm-app/app/lib/calendar.ts

  • Snippet: getCalendar

    • File: fdm-app/app/lib/calendar.ts
    • Range: 7-11
    • Content: 
      export function getCalendar(params: Params): string {
    const calendar = params.calendar as string

    return calendar
}
    • Relevance: Shows that getCalendar simply returns the calendar value from route params (string). This supports evaluating whether it should return a year number instead of a string.

  • Snippet: getTimeframe

    • File: fdm-app/app/lib/calendar.ts
    • Range: 13-37
    • Content: 
      export function getTimeframe(params: Params): Timeframe {
    const calendar = getCalendar(params)

    const timeframe = {
        start: new Date(`${yearStart}-01-01T00:00:00.000Z`),
        end: new Date(`${yearEnd}-12-31T23:59:59.999Z`),
    }

    // Check if calendar is year and create a timeframe
    if (calendar) {
        // Try to coerce to year
        const year = Number(calendar)
        if (!Number.isNaN(year)) {
            // Check if year is supported
            if (year < yearStart || year > yearEnd) {
                throw new Error(`Unsupported year: ${calendar}`)
            }
            // Set start and end date
            timeframe.start = new Date(`${year}-01-01T00:00:00.000Z`)
            timeframe.end = new Date(`${year}-12-31T23:59:59.999Z`)
        }
    }

    return timeframe
}
    • Relevance: Indicates that only if calendar coerces to a numeric year is the timeframe adjusted to that year. This is central to the user’s expectation that the calendar parameter represents a year.

fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx

  • Snippet: usage of calendar value as current year

    • File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx
    • Range: 0-0 (context shown in action)
    • Content (relevant excerpt): 
      const currentYear = Number(calendar)
    • Relevance: Demonstrates how the code currently treats the calendar value as a year number when deriving default cultivation dates. Supports the idea that getCalendar should yield a value representing a year.

  • Snippet: getDefaultDatesOfCultivation usage

    • File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx
    • Range: (see surrounding code where cultivationDefaultDates is computed)
    • Content (summarized description):
      • Calling getDefaultDatesOfCultivation with parameters:
        • fdm, session.principal_id, b_id_farm, b_lu_catalogue, currentYear
      • Then using cultivationDefaultDates.b_lu_start and cultivationDefaultDates.b_lu_end
    • Relevance: Illustrates that the default dates function returns date objects used to initialize cultivation spans for a new field, aligning with the user’s expectation that the function returns actual Date objects.

fdm-core/src/cultivation.ts

  • Doc: getDefaultDatesOfCultivation
    • File: fdm-core/src/cultivation.ts
    • Range: 191-289
    • Description:
      • Function signature:
        • getDefaultDatesOfCultivation(fdm, principal_id, b_id_farm, b_lu_catalogue, year) -> Promise
      • Returns:
        • CultivationDefaultDates with:
          • b_lu_start: Date
          • b_lu_end: Date | undefined
        • computed using enabled catalogues and catalogue defaults (b_lu_start_default, b_date_harvest_default)
      • Behavior highlights:
        • Validates permissions
        • Retrieves enabled cultivation catalogues for farm
        • Looks up catalogue entry for b_lu_catalogue
        • Constructs default start date from year and b_lu_start_default
        • If harvestable type is "once" and a default harvest date exists, sets b_lu_end accordingly
        • If end date is before start date, adjusts start date to previous year
      • Relevant error handling:
        • Throws errors if catalogue entry not found, or on permission/DB issues
    • Relevance: Directly addresses the user concern that getDefaultDatesOfCultivation should return Date objects representing default cultivation dates. This description clarifies expected return type and behavior.

Copy link
Copy Markdown
Collaborator Author

@SvenVw SvenVw Oct 10, 2025

Choose a reason for hiding this comment

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

Resolved in c35663a and 95309db

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 10, 2025

Choose a reason for hiding this comment

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

@SvenVw, thank you for addressing this! I appreciate you taking the time to implement the validation guards. 🎉

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

const b_acquiring_method = "unknown"

Expand Down
22 changes: 15 additions & 7 deletions fdm-app/app/routes/farm.create.$b_id_farm.$calendar.atlas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
addField,
addSoilAnalysis,
getCultivations,
getDefaultDatesOfCultivation,
getFarm,
getFields,
} from "@svenvw/fdm-core"
Expand Down Expand Up @@ -428,13 +429,20 @@ export async function action({ request, params }: ActionFunctionArgs) {
const b_id_source = field.properties.b_id_source
const b_lu_catalogue = field.properties.b_lu_catalogue
const b_geometry = field.geometry
const currentYear = new Date().getFullYear()
const defaultDate = timeframe.start
? timeframe.start
: `${currentYear}-01-01`
const b_start = defaultDate
const b_lu_start = defaultDate
const b_lu_end = undefined

const currentYear = Number(calendar)
const cultivationDefaultDates =
await getDefaultDatesOfCultivation(
fdm,
session.principal_id,
b_id_farm,
b_lu_catalogue,
currentYear,
)

const b_start = new Date(`${currentYear}-01-01`)
const b_lu_start = cultivationDefaultDates.b_lu_start
const b_lu_end = cultivationDefaultDates.b_lu_end
const b_end = undefined
Comment thread
SvenVw marked this conversation as resolved.
const b_acquiring_method = "unknown"

Expand Down
14 changes: 12 additions & 2 deletions fdm-app/app/routes/farm.create.$b_id_farm.$calendar.upload.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
addCultivation,
addField,
addSoilAnalysis,
getDefaultDatesOfCultivation,
getFarm,
} from "@svenvw/fdm-core"
import * as turf from "@turf/turf"
Expand Down Expand Up @@ -233,13 +234,22 @@ export async function action({ request, params }: ActionFunctionArgs) {
b_end,
)

const cultivationDefaultDates = await getDefaultDatesOfCultivation(
fdm,
session.principal_id,
b_id_farm,
b_lu_catalogue,
Number(calendar),
)
const b_lu_start = cultivationDefaultDates.b_lu_start
const b_lu_end = cultivationDefaultDates.b_lu_end
await addCultivation(
Comment thread
SvenVw marked this conversation as resolved.
fdm,
session.principal_id,
b_lu_catalogue,
fieldId,
new Date(`${calendar}-01-01`),
undefined,
b_lu_start,
b_lu_end,
)

if (nmiApiKey) {
Expand Down
5 changes: 5 additions & 0 deletions fdm-core/src/cultivation.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,8 @@ export interface CultivationPlan {
}

export type CultivationCatalogue = schema.cultivationsCatalogueTypeSelect

export type CultivationDefaultDates = {
b_lu_start: Date
b_lu_end: Date | undefined
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Loading