Disable buttons in the user interface based on the user's farm permissions#344
Conversation
🦋 Changeset detectedLatest commit: d975484 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
WalkthroughLoaders now call runtime permission checks (checkPermission / hasPermission) and return booleans/maps; many UI components receive new optional props (editable, canAddItem, canModify*, has_write_permission) and use them to disable/hide create/edit/delete controls and inputs. Changes
Sequence Diagram(s)sequenceDiagram
participant Loader as Route Loader
participant Auth as checkPermission / hasPermission
participant UI as Component (Card/List/Form)
participant User as User
participant Server as Fetcher/Form
Loader->>Auth: query write permission(s) (resource/id/path)
Auth-->>Loader: boolean or per-item permission map
Loader->>UI: pass editable / canCreate / canModify / canAddItem props
alt editable / canModify true
UI->>User: render enabled controls (buttons, inputs)
User->>UI: interact (submit/update/delete)
UI->>Server: perform write action (fetcher/form)
else editable / canModify false
UI->>User: render disabled/hidden controls (read-only)
User->>UI: no write interactions available
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
fdm-core/src/authorization.ts (1)
163-309: Align hasPermission JSDoc with the actual boolean return typeThe new factoring around
getPermissionlooks good and keepscheckPermission/hasPermissionbehavior consistent. One thing to fix: the JSDoc forhasPermission(Lines 211–222) still says “Gets the granting resource type and ID…”, which is misleading now that the function returns a bareboolean. The summary and@returnsdescription should clearly state that it only returnstrue/false, and that callers needing the granting resource must use a different API.fdm-app/app/components/blocks/cultivation/card-list.tsx (1)
13-23: Empty-state add button ignoreseditableand breaks type expectationsTwo related issues here:
- In the empty-state branch you’re not passing
editable:<CultivationAddFormDialog options={cultivationsCatalogueOptions} />so a user without
fieldWritePermissionwill still see an enabled “Gewas toevoegen” button whenevercultivations.length === 0, which defeats the permission-based UI goal.
- With
CultivationsFormPropscurrently requiringeditable, this call also violates the component’s props type.Propagate
editableconsistently:- <CultivationAddFormDialog - options={cultivationsCatalogueOptions} - /> + <CultivationAddFormDialog + options={cultivationsCatalogueOptions} + editable={editable} + />This keeps the UI behavior and typing consistent across both branches.
Also applies to: 30-36, 54-56
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx (1)
1-11: Avoid unusedfieldWritePermissioncall
fieldWritePermissionis computed viahasPermissionbut never returned or used in the component, so the permission check still runs but its result is discarded.Consider either:
- returning it from the loader and using it in the UI, or
- removing the call entirely if field-level write permission is not needed here.
A cleaner pattern would be:
- const fieldWritePermission = hasPermission( - fdm, - "field", - "write", - b_id, - session.principal_id, - ) - - const cultivationWritePermission = hasPermission( + const cultivationWritePermission = hasPermission( fdm, "cultivation", "write", b_lu, session.principal_id, )Also applies to: 166-198, 213-226
fdm-app/app/components/blocks/cultivation/card-harvests.tsx (1)
9-19:editablecurrently overrides harvest rules and allows extra harvestsRight now:
editabledefaults totrue, andif (typeof editable !== "undefined") { canAddHarvest = editable }means
canAddHarvestis always forced totruewheneditableistrue, even forb_lu_harvestable === "once"with existing harvests or"none". That’s a behavior change from the previous logic where those cases correctly disabled “Oogst toevoegen”.To keep the original harvest rules and still allow permissions to turn actions off, only override when
editableis explicitlyfalse:-export function CultivationHarvestsCard({ - harvests, - b_lu_harvestable, - harvestParameters, - editable = true, -}: { +export function CultivationHarvestsCard({ + harvests, + b_lu_harvestable, + harvestParameters, + editable = true, +}: { @@ - if (b_lu_harvestable === "multiple") { - canAddHarvest = true - } - if (typeof editable !== "undefined") { - canAddHarvest = editable - } + if (b_lu_harvestable === "multiple") { + canAddHarvest = true + } + // Permission can only further restrict adding a harvest + if (editable === false) { + canAddHarvest = false + }This preserves the domain rule (max one harvest for
"once") while still honoringeditable = falsefor permission-based disabling.Also applies to: 27-29
🧹 Nitpick comments (9)
fdm-app/app/components/blocks/cultivation/form-add.tsx (1)
20-39: Decouple dialog’seditableconcern from the inner form propsThe runtime behavior (disabling the “Gewas toevoegen” button via
editable) is good. The type coupling is less ideal:
CultivationAddFormDialogneedseditable.CultivationAddFormonly needsoptionsbut currently depends onCultivationsFormProps, draggingeditableinto its props type even though it’s unused and not passed.After relaxing or splitting
CultivationsFormProps(see types.tsx comment), consider tightening the inner form signature, e.g.:-type CultivationAddFormProps = CultivationsFormProps & { onSuccess?: () => void } +type CultivationAddFormProps = { + options: CultivationOption[] + onSuccess?: () => void +}so only the dialog layer is permission-aware.
fdm-app/app/components/custom/date-picker.tsx (1)
72-78:disabledshould also cover the calendar trigger, not just the text inputAdding
disabledto the input is a good start, but with the current implementation a user can still change the date via the calendar button/popup whendisabledis true.To make
disabledbehave like a true read-only/disabled control (important for permission-driven UIs), also disable the trigger button (and optionally block opening the popover), for example:- <Popover open={open} onOpenChange={setOpen}> + <Popover open={open} onOpenChange={(nextOpen) => { + if (!disabled) { + setOpen(nextOpen) + } + }}> <PopoverTrigger asChild> <Button id={`${field.name}-picker`} variant="ghost" className="absolute top-1/2 right-2 size-6 -translate-y-1/2" + disabled={disabled} >This way, when
disabledis true, neither typing nor calendar interactions can change the value.Also applies to: 80-87, 122-129, 154-165
fdm-app/app/components/blocks/fertilizer-applications/list.tsx (1)
35-36: Per-application permission handling looks consistent
canModifyFertilizerApplicationwith a{[p_app_id]: boolean}map plus theeditableflag wiring (disabling edit and hiding delete) matches the PR goal and keeps old callers working via the?? truedefault. Just be aware that any missing IDs in the map will be treated as editable, so loaders should ideally populate the map for all rendered applications.Also applies to: 45-46, 62-66, 87-100, 127-169
fdm-app/app/components/blocks/fertilizer-applications/card.tsx (1)
22-29: Consider checking permissions when restoring saved editsThe create/modify props are wired correctly: the add button is disabled via
canCreateFertilizerApplication, and per-application permissions are passed down to the list.One small edge case: when restoring
savedFormValuesyou only check thatapplicationToEditexists, not that it’s still modifiable according tocanModifyFertilizerApplication. If a user’s permissions change between saving and returning, the dialog could reopen in an edit state for an application they can no longer modify.You could defensively guard this by also checking the permission map:
- const applicationToEdit = savedFormValues?.p_app_id - ? fertilizerApplications.find( - (app) => app.p_app_id === savedFormValues.p_app_id, - ) - : null + const applicationToEdit = savedFormValues?.p_app_id + ? fertilizerApplications.find((app) => { + const canModify = + canModifyFertilizerApplication[app.p_app_id] ?? true + return app.p_app_id === savedFormValues.p_app_id && canModify + }) + : nullAlso applies to: 37-41, 85-123, 182-191
fdm-app/app/components/blocks/cultivation/card-details.tsx (1)
30-38: Editable handling in cultivation details is consistentUsing
editableto:
- disable the underlying form (
disabled: !editable),- hide the delete and update actions,
- and disable individual inputs
gives a clean read-only mode when permissions are missing. If you want to be extra defensive, you could also add
|| !editableto the<fieldset disabled={...}>expression, but functionally the current implementation already prevents edits.Also applies to: 40-52, 72-97, 109-123, 126-171, 206-226
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx (1)
1-8: Soil permissions and per-analysis gating look goodThe loader’s use of
hasPermissionfor:
- field-level writes (
fieldWritePermission), and- per-analysis writes (
soilAnalysisWritePermissions)and the way those are wired into:
- the “Bodemanalyse toevoegen” buttons, and
SoilAnalysesList.canModifySoilAnalysismatches the intended behavior and keeps the UI aligned with permissions. The Promise-based pattern is fine; if you ever touch this again, you could also write
const fieldWritePermission = await hasPermission(...)for slightly clearer intent.Also applies to: 96-131, 159-171, 176-193, 205-211
fdm-app/app/components/blocks/soil/list.tsx (1)
9-20: Avoid nesting<Button>inside<NavLink>; useButton asChildinstead
NavLinkwrapping aButtonproduces an<a><button>…</button></a>structure, which is invalid HTML and may cause accessibility and event-handling quirks. Also,asChildonNavLinkhas no effect.You can keep the same visuals while fixing the structure by inverting the wrapper:
- <div className="space-x-4"> - <NavLink - to={`./analysis/${analysis.a_id}`} - asChild - className={cn( - "pointer-events-auto", - analysis.a_source === "nl-other-nmi" - ? "pointer-events-none" - : "", - )} - > - <Button - variant="default" - disabled={ - fetcher.state === - "submitting" || - analysis.a_source === - "nl-other-nmi" - } - > - Bewerk - </Button> - </NavLink> + <div className="space-x-4"> + <Button + asChild + variant="default" + disabled={ + fetcher.state === "submitting" || + analysis.a_source === + "nl-other-nmi" + } + > + <NavLink + to={`./analysis/${analysis.a_id}`} + className={cn( + "pointer-events-auto", + analysis.a_source === + "nl-other-nmi" + ? "pointer-events-none" + : "", + )} + > + Bewerk + </NavLink> + </Button>The rest of the per-analysis permission gating via
canModifySoilAnalysis[analysis.a_id] ?? truelooks solid.Also applies to: 29-33, 52-76, 77-96
fdm-app/app/components/blocks/harvest/form.tsx (1)
38-56: Aligneditablebehavior across harvest form variantsThe introduction of
editableand makingactionoptional are sensible, andHarvestFormDialogalready hides the delete/save buttons wheneditableis false. Two small consistency tweaks would make the behavior clearer:
- Disable the dialog form when read-only
Right now the
<FieldSet>inHarvestFormDialogonly checksform.formState.isSubmitting. You can also tie ineditableso inputs are visually/read-only disabled:- <FieldSet disabled={form.formState.isSubmitting}> + <FieldSet + disabled={ + form.formState.isSubmitting || editable === false + } + >
- Respect
editablein the non-dialogHarvestForm
HarvestFormaccepts the same props type but always renders active delete/save buttons. If you intend to support read-only mode there as well, mirror the dialog logic:-export function HarvestForm(props: HarvestFormDialogProps) { - const { b_lu_harvest_date, action } = props +export function HarvestForm(props: HarvestFormDialogProps) { + const { b_lu_harvest_date, action, editable = true } = props @@ - <fieldset - disabled={form.formState.isSubmitting} - className="space-y-8" - > + <fieldset + disabled={ + form.formState.isSubmitting || editable === false + } + className="space-y-8" + > @@ - <div className="grid grid-cols-2 items"> - <Button - type="button" - variant="destructive" - onClick={handleDeleteHarvest} - disabled={ - form.formState.isSubmitting || - fetcher.state === "submitting" - } - className={cn( - "mr-auto", - !isHarvestUpdate ? "invisible" : "", - )} - > + <div className="grid grid-cols-2 items"> + {editable && ( + <Button + type="button" + variant="destructive" + onClick={handleDeleteHarvest} + disabled={ + form.formState.isSubmitting || + fetcher.state === "submitting" + } + className={cn( + "mr-auto", + !isHarvestUpdate ? "invisible" : "", + )} + > @@ - Verwijderen - </Button> - <Button type="submit" className="ml-auto"> + Verwijderen + </Button> + )} + <Button + type="submit" + className="ml-auto" + disabled={ + form.formState.isSubmitting || + editable === false + } + >This keeps both dialog and non-dialog harvest forms in sync with the new permission model.
Also applies to: 58-79, 490-498, 512-513, 526-528, 530-553, 564-583, 594-603, 616-661
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx (1)
169-195: Permission checks for field and applications are structured well; consider hardening resource type usageThe pattern of:
- computing
fieldWritePermissiononce, and- computing per-application write permissions in parallel via
Promise.allandObject.fromEntriesis sound and efficient for the current loader shape.
One minor improvement would be to ensure the
"field"and"fertilizer_application"resource type strings are not duplicated as magic strings across the codebase (e.g., centralize them in a shared enum/constant exported from fdm-core or a local module) to avoid subtle typos breaking permission checks.Please double-check that:
- the
hasPermission(fdm, resource, action, id, principal_id)argument order here matches the current fdm-core signature, and"field"/"fertilizer_application"exactly match the resource identifiers expected by fdm-core (you can confirm by grepping for otherhasPermissionusages in the repo).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
fdm-app/app/components/blocks/cultivation/card-details.tsx(6 hunks)fdm-app/app/components/blocks/cultivation/card-harvests.tsx(2 hunks)fdm-app/app/components/blocks/cultivation/card-list.tsx(2 hunks)fdm-app/app/components/blocks/cultivation/form-add.tsx(1 hunks)fdm-app/app/components/blocks/cultivation/types.tsx(1 hunks)fdm-app/app/components/blocks/fertilizer-applications/card.tsx(4 hunks)fdm-app/app/components/blocks/fertilizer-applications/list.tsx(5 hunks)fdm-app/app/components/blocks/harvest/form.tsx(5 hunks)fdm-app/app/components/blocks/soil/list.tsx(2 hunks)fdm-app/app/components/custom/date-picker.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx(5 hunks)fdm-core/src/authorization.ts(3 hunks)fdm-core/src/index.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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/components/blocks/cultivation/types.tsxfdm-app/app/components/blocks/cultivation/form-add.tsxfdm-app/app/components/blocks/cultivation/card-harvests.tsxfdm-app/app/components/blocks/cultivation/card-details.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/components/blocks/cultivation/card-list.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/components/blocks/cultivation/types.tsx
📚 Learning: 2025-04-18T14:20:40.975Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:70-76
Timestamp: 2025-04-18T14:20:40.975Z
Learning: The schema defined in fdm-core/src/db/schema-authn.ts follows better-auth's structure and requirements. While the schema is defined in the application code, modifications to it should maintain compatibility with better-auth's expectations.
Applied to files:
fdm-core/src/index.ts
📚 Learning: 2025-04-18T14:20:40.975Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:70-76
Timestamp: 2025-04-18T14:20:40.975Z
Learning: The organization schema in fdm-core/src/db/schema-authn.ts is managed by better-auth, and modifications to field constraints (like making the slug field non-nullable) should maintain compatibility with better-auth's expectations, even if application code assumes non-null values.
Applied to files:
fdm-core/src/index.ts
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/list.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/components/blocks/fertilizer-applications/card.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsxfdm-core/src/authorization.ts
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/components/blocks/cultivation/card-details.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 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: When using `updateField` from fdm-core, all 8 parameters must be provided in order: fdm, b_id, b_name, b_geometry, b_area, b_id_source, b_id_farm, and b_id_farm_source.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/components/blocks/harvest/form.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 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 requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
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:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
🧬 Code graph analysis (8)
fdm-app/app/components/blocks/cultivation/form-add.tsx (1)
fdm-app/app/components/blocks/cultivation/types.tsx (1)
CultivationsFormProps(15-18)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx (2)
fdm-core/src/index.ts (1)
hasPermission(25-25)fdm-core/src/db/schema-authn.ts (1)
session(29-41)
fdm-app/app/components/blocks/harvest/form.tsx (1)
fdm-app/app/components/blocks/harvest/schema.tsx (1)
FormSchema(3-236)
fdm-app/app/components/blocks/cultivation/card-details.tsx (4)
fdm-app/app/components/blocks/cultivation/types.tsx (1)
Cultivation(1-8)fdm-core/src/index.ts (1)
Cultivation(56-56)fdm-app/app/components/blocks/cultivation/schema.ts (1)
CultivationDetailsFormSchema(3-40)fdm-app/app/components/custom/date-picker.tsx (1)
DatePicker(80-202)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx (1)
fdm-core/src/index.ts (1)
hasPermission(25-25)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx (2)
fdm-core/src/index.ts (1)
hasPermission(25-25)fdm-app/app/components/blocks/cultivation/card-harvests.tsx (1)
CultivationHarvestsCard(9-65)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx (1)
fdm-core/src/index.ts (1)
hasPermission(25-25)
fdm-app/app/components/blocks/cultivation/card-list.tsx (1)
fdm-app/app/components/blocks/cultivation/types.tsx (2)
CultivationOption(10-13)Cultivation(1-8)
🔇 Additional comments (6)
fdm-core/src/index.ts (1)
25-25: Exporting checkPermission/hasPermission from the root is fineRe-exporting
checkPermissionandhasPermissionfrom the package root is a clean, backwards-compatible way to expose authorization helpers. No issues here.fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx (1)
5-8: Harvest write permission wiring looks correctUsing
hasPermissionto computeharvestingWritePermissionand passing it aseditabletoHarvestFormDialogis a good way to gate the UI without duplicating logic. The “start promise then await at return” pattern is also fine here.Just ensure that the underlying
updateHarvest/removeHarvestimplementations in@svenvw/fdm-corestill perform server-side authorization checks so this remains a UI convenience, not the only guard.Also applies to: 91-99, 132-133, 169-170
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx (1)
1-9: Field write permission wiring toCultivationListCardis consistentThe loader’s use of
hasPermissionto computefieldWritePermissionand then passing it through aseditabletoCultivationListCardaligns well with the permission-driven UI pattern in this PR. The separation between:
- loader: deriving a boolean for UI state, and
- action: delegating actual writes to
addCultivation/removeCultivationin@svenvw/fdm-corelooks good, assuming those core functions continue to enforce authorization on the server side.
Also applies to: 75-87, 136-143, 160-169
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx (3)
1-11: ImportinghasPermissionis appropriate for the new loader logicThe added
hasPermissionimport from@svenvw/fdm-coreis consistent with the new permission checks later in the loader and keeps the dependency surface clear.
197-210: Returning resolved permission flags from the loader aligns with UI needsExposing
fieldWritePermission(as a resolved boolean) andfertilizerApplicationWritePermissions(as a plainRecord<string, boolean>) on the loader result cleanly matches the props expected by the UI and keeps permission state colocated with the rest of the field/application data.
234-250: Wiring loader permissions intoFertilizerApplicationCardlooks correctPassing:
canCreateFertilizerApplication={loaderData.fieldWritePermission}andcanModifyFertilizerApplication={loaderData.fertilizerApplicationWritePermissions}is consistent with the loader’s output and with the intended behavior of disabling creation when the field is not writable and controlling per-application edit/delete via the permissions map.
Please verify in
FertilizerApplicationCardandFertilizerApplicationsListthat:
- they gracefully handle missing keys in
canModifyFertilizerApplication(e.g., defaulting tofalse), and- they don’t expect these props to be Promises (you’re now returning plain values from the loader).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development #344 +/- ##
===============================================
+ Coverage 87.60% 87.62% +0.02%
===============================================
Files 79 79
Lines 3952 3959 +7
Branches 1140 1145 +5
===============================================
+ Hits 3462 3469 +7
Misses 490 490
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Few issues found, unrelated to the PR goals:
To be made issues later, maybe. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/components/blocks/harvest/form.tsx (1)
619-661: Consider consistent button visibility handling.In
HarvestForm, the delete (line 629) and submit (line 650) buttons remain visible but are disabled via the fieldset wheneditableis false. However, inHarvestFormDialog, both buttons are conditionally rendered and completely hidden when not editable (lines 532-555, 566-585).This creates an inconsistent user experience between the dialog and inline form variants. Consider wrapping these buttons in conditional rendering for consistency:
<div className="grid grid-cols-2 items"> + {editable && ( <Button type="button" variant="destructive" onClick={handleDeleteHarvest} disabled={ form.formState.isSubmitting || fetcher.state === "submitting" } className={cn( "mr-auto", !isHarvestUpdate ? "invisible" : "", )} > {form.formState.isSubmitting || fetcher.state === "submitting" ? ( <div className="flex items-center space-x-2"> <LoadingSpinner /> </div> ) : null} Verwijderen </Button> + )} + {editable && ( <Button type="submit" className="ml-auto"> {form.formState.isSubmitting ? ( <div className="flex items-center space-x-2"> <LoadingSpinner /> <span>Opslaan...</span> </div> ) : isHarvestUpdate ? ( "Bijwerken" ) : ( "Toevoegen" )} </Button> + )} </div>Alternatively, if the visibility difference is intentional, this can be safely ignored.
🧹 Nitpick comments (3)
fdm-app/app/components/blocks/fertilizer-applications/card.tsx (1)
22-41: Permission-aware gating looks good; include permission props in effect depsThe new
canCreateFertilizerApplicationandcanModifyFertilizerApplicationchecks correctly:
- Disable the “Toevoegen” button when creation isn’t allowed.
- Prevent auto‑opening the dialog for an existing application when per‑item modify permission is false.
- Avoid opening a new‑application dialog when creation is not allowed.
One improvement: the
useEffectthat auto‑opens the dialog based onsavedFormValues(lines 111‑128) now depends logically oncanCreateFertilizerApplicationandcanModifyFertilizerApplication, but they’re not in the dependency array. If these permissions could ever change without a full remount, the effect would use stale values.You can make this robust with:
- useEffect(() => { + useEffect(() => { if (savedFormValues && !isDialogOpen) { if (savedFormValues.p_app_id) { // Do not open the form if there is a risk it will create a new application if ( applicationToEdit && (canModifyFertilizerApplication[ applicationToEdit.p_app_id ] ?? true) ) { setIsDialogOpen(true) } } else if (canCreateFertilizerApplication) { setIsDialogOpen(true) } } - }, [savedFormValues, applicationToEdit, isDialogOpen]) + }, [ + savedFormValues, + applicationToEdit, + isDialogOpen, + canCreateFertilizerApplication, + canModifyFertilizerApplication, + ])Also applies to: 85-102, 111-128, 156-159, 188-197
fdm-app/app/components/custom/date-picker.tsx (1)
114-118: Minor optimization opportunity for the useEffect.The effect runs on every
openstate change due to the dependency array, but it only needs to act whendisabledbecomestrue. This is functionally correct but slightly inefficient.Consider using a ref to track the previous disabled state, or simplify to only depend on
disabled:React.useEffect(() => { - if (disabled && open) { + if (disabled) { setOpen(false) } - }, [disabled, open]) + }, [disabled])This is safe because
setOpen(false)is idempotent whenopenis alreadyfalse.fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx (1)
179-185: Consider computing farm permission concurrently with field data.The
farmWritePermissioncheck runs sequentially after all field data is fetched. For better performance, consider starting it earlier to run concurrently.+ // Start farm permission check early for concurrent execution + const farmWritePermissionPromise = hasPermission( + fdm, + "farm", + "write", + b_id_farm, + session.principal_id, + ) + const fieldsExtended = await Promise.all( fields.map(async (field) => { // ... existing code }), ) - const farmWritePermission = await hasPermission( - fdm, - "farm", - "write", - b_id_farm, - session.principal_id, - ) + const farmWritePermission = await farmWritePermissionPromise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
fdm-app/app/components/blocks/cultivation/card-details.tsx(7 hunks)fdm-app/app/components/blocks/cultivation/card-list.tsx(1 hunks)fdm-app/app/components/blocks/cultivation/form-add.tsx(3 hunks)fdm-app/app/components/blocks/fertilizer-applications/card.tsx(5 hunks)fdm-app/app/components/blocks/fertilizer/table.tsx(2 hunks)fdm-app/app/components/blocks/fields/columns.tsx(3 hunks)fdm-app/app/components/blocks/fields/table.tsx(2 hunks)fdm-app/app/components/blocks/harvest/form.tsx(8 hunks)fdm-app/app/components/blocks/rotation/table.tsx(2 hunks)fdm-app/app/components/blocks/soil/form.tsx(4 hunks)fdm-app/app/components/blocks/soil/list.tsx(3 hunks)fdm-app/app/components/custom/date-picker-v2.tsx(2 hunks)fdm-app/app/components/custom/date-picker.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsx(3 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm._index.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- fdm-app/app/components/blocks/cultivation/card-list.tsx
- fdm-app/app/components/blocks/soil/list.tsx
- fdm-app/app/components/blocks/cultivation/form-add.tsx
🧰 Additional context used
🧠 Learnings (32)
📓 Common learnings
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
📚 Learning: 2025-01-23T15:18:57.212Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/src/db/schema.ts:407-426
Timestamp: 2025-01-23T15:18:57.212Z
Learning: In the farm data model, each cultivation (b_lu) can have only one termination date but can have multiple harvest dates. This is enforced through the database schema where cultivationTerminating uses b_lu as primary key while cultivationHarvesting uses a composite primary key of b_id_harvestable and b_lu.
Applied to files:
fdm-app/app/components/blocks/fields/columns.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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/components/blocks/cultivation/card-details.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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/components/blocks/cultivation/card-details.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2025-04-04T14:27:39.518Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 116
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsx:111-154
Timestamp: 2025-04-04T14:27:39.518Z
Learning: In the FDM application, cultivation retrieval logic should be centralized in utility functions rather than duplicated across loader and action functions to improve maintainability and ensure consistent behavior.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/components/blocks/fields/table.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/components/blocks/fertilizer-applications/card.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 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: When using `updateField` from fdm-core, all 8 parameters must be provided in order: fdm, b_id, b_name, b_geometry, b_area, b_id_source, b_id_farm, and b_id_farm_source.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsxfdm-app/app/components/blocks/rotation/table.tsxfdm-app/app/components/blocks/fields/table.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 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/app/routes/farm.$b_id_farm.fertilizers._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
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:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 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 requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-14T16:06:21.832Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-14T16:06:24.294Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-03-04T09:03:04.358Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 87
File: fdm-core/src/farm.ts:22-25
Timestamp: 2025-03-04T09:03:04.358Z
Learning: In the FDM codebase, functions that insert/create data (like `addFarm`) intentionally use `principal_id: string` instead of `PrincipalId` to enforce that only a single principal ID can be used for creation operations, while read operations use the more flexible `PrincipalId` type which supports both single IDs and arrays of IDs.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm._index.tsx
📚 Learning: 2025-02-13T08:35:59.306Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.$b_id_farm.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx:114-124
Timestamp: 2025-02-13T08:35:59.306Z
Learning: The HarvestForm component in fdm-app expects undefined (not 0) for b_lu_yield when no yield information is available, as 0 would incorrectly imply that yield data exists.
Applied to files:
fdm-app/app/components/blocks/harvest/form.tsx
🧬 Code graph analysis (11)
fdm-app/app/components/blocks/cultivation/card-details.tsx (2)
fdm-app/app/components/blocks/cultivation/types.tsx (1)
Cultivation(1-8)fdm-app/app/components/custom/date-picker.tsx (1)
DatePicker(80-209)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx (2)
fdm-core/src/index.ts (1)
hasPermission(25-25)fdm-app/app/components/blocks/cultivation/card-harvests.tsx (1)
CultivationHarvestsCard(9-65)
fdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsx (1)
fdm-core/src/index.ts (1)
hasPermission(25-25)
fdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsx (1)
fdm-core/src/db/schema-authn.ts (1)
session(29-41)
fdm-app/app/components/blocks/rotation/table.tsx (1)
fdm-app/app/components/blocks/rotation/columns.tsx (1)
RotationExtended(22-50)
fdm-app/app/components/blocks/fields/table.tsx (1)
fdm-app/app/components/blocks/fields/columns.tsx (1)
FieldExtended(24-41)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsx (2)
fdm-core/src/index.ts (1)
hasPermission(25-25)fdm-core/src/db/schema.ts (1)
soilAnalysis(721-764)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx (1)
fdm-core/src/index.ts (1)
hasPermission(25-25)
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (1)
fdm-core/src/db/schema-authn.ts (1)
session(29-41)
fdm-app/app/routes/farm.$b_id_farm._index.tsx (3)
fdm-core/src/index.ts (2)
hasPermission(25-25)isAllowedToDeleteFarm(71-71)fdm-core/src/db/schema-authn.ts (1)
session(29-41)fdm-core/src/farm.ts (1)
isAllowedToDeleteFarm(569-587)
fdm-app/app/components/blocks/harvest/form.tsx (1)
fdm-app/app/components/blocks/harvest/schema.tsx (1)
FormSchema(3-236)
🔇 Additional comments (34)
fdm-app/app/components/blocks/fields/columns.tsx (2)
269-275: Backend authorization for field deletion is properly implemented.The
removeFieldfunction infdm-core/src/field.ts(lines 471-484) includes server-side authorization checks before deletion occurs. It callscheckPermission(fdm, "field", "write", b_id, principal_id)and throws an error if the principal lacks write permission. The authorization check executes before the database transaction, ensuring unauthorized deletions are rejected at the backend level regardless of client-side UI state.
40-40: All data sources correctly populatehas_write_permission.The verification confirms that the primary loader in
farm.$b_id_farm.$calendar.field._index.tsxproperly constructsFieldExtendedobjects with thehas_write_permissionfield by callinghasPermission()and including it in the returned object. TypeScript's generic type constraint<TData extends FieldExtended>on theDataTablecomponent enforces that all data passed to it must conform to the type, preventing any accidental omission of the field at compile time.fdm-app/app/components/blocks/harvest/form.tsx (5)
53-55: LGTM! Props updated for permission-based UI control.The addition of the optional
editableprop and makingactionoptional aligns with the PR objectives to gate UI based on user permissions.
73-78: LGTM! Hook correctly implements permission-based form disabling.The
editableparameter defaults totruefor backward compatibility, and the form'sdisabledstate is correctly derived from it.
491-514: LGTM! Dialog correctly implements permission-based form gating.The
editableprop is properly defaulted totrue, and theFieldSetdisabled state correctly considers both the editable flag and submission state.
532-555: LGTM! Delete button correctly hidden when not editable.The conditional rendering ensures the delete button is completely removed from the UI when the user lacks edit permissions, which is both good UX and a security best practice.
566-585: LGTM! Submit button correctly hidden when not editable.The conditional rendering ensures the submit button is completely removed from the UI when the user lacks edit permissions, maintaining consistency with the delete button handling.
fdm-app/app/components/blocks/soil/form.tsx (2)
75-75: Approved: fieldset correctly disabled based on permissions.The fieldset is appropriately disabled when the user lacks write permission or during form submission, preventing unauthorized or concurrent edits.
229-240: Approved: save button correctly hidden when not editable.Conditionally rendering the save button based on the
editableflag ensures users without write permission cannot submit the form, aligning UI with backend permissions.fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsx (3)
146-146: Approved: editable prop correctly wired to permission check.The
editableprop is properly set based on the loader's permission check, ensuring the form respects user permissions.
165-212: The original review comment is based on an incorrect assumption.The
updateSoilAnalysisfunction in fdm-core/src/soil.ts (lines 120–127) already performs authorization validation by callingcheckPermissionwith "write" permission check for the soil analysis resource. The review's concern about missing authorization checks is unfounded. No additional explicit permission check is needed in the action function—the underlyingupdateSoilAnalysiscall already validates that the principal has write access before performing any updates.Likely an incorrect or invalid review comment.
5-5: No action required —hasPermissioncorrectly returns a boolean.The
hasPermissionfunction from@svenvw/fdm-coreis async and returns a Promise that resolves to a boolean via the!!operator. At line 102, it is properly awaited in the loader function, and the boolean result is passed to theeditableprop ofSoilAnalysisFormat line 146. The function never returnsundefinedornull; on error, it throws an exception instead.fdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsx (1)
8-10: Editing now correctly depends on farm write permissionThe extra
hasPermissioncheck cleanly tightenseditableto “fertilizer belongs to this farm and user has farm:write”, which matches the intended UI gating on edit controls without changing existing action logic.Also applies to: 109-125
fdm-app/app/components/blocks/fertilizer/table.tsx (1)
25-35: NewcanAddItemprop cleanly gates the add-fertilizer buttonMaking
canAddItemrequired onDataTableand conditionally rendering the “Meststof toevoegen” block behind it is straightforward and keeps permission concerns at the call site.Also applies to: 70-79
fdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsx (1)
6-7: Farm write permission is wired consistently into the fertilizers overviewComputing
farmWritePermissionviahasPermissionin the loader and passing it through ascanAddItemtoDataTablegives a single, clear control point for showing the “add fertilizer” action.Also applies to: 93-105, 106-107, 109-115, 147-151
fdm-app/app/components/blocks/rotation/table.tsx (1)
51-55: Rotation actions now correctly hide when add-permission is absentThe new
canAddItemprop and conditional wrapper around the Bemesting/Oogst buttons neatly ensure rotation actions only appear when the caller grants permission, without disturbing existing selection/tooltip behavior.Also applies to: 57-61, 281-355
fdm-app/app/components/custom/date-picker-v2.tsx (1)
66-71: Date picker now correctly respects disabled stateAutomatically closing the popover when
field.disabledbecomes true and disabling the trigger button keeps the date picker in sync with the parent form’s read‑only/permission state.Also applies to: 118-125
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsx (1)
1-1: Field delete navigation is now gated by write permissionUsing
hasPermissionto computefieldWritePermissionand only appending the “Verwijderen” sidebar item when it’s true prevents users without field:write permission from seeing the delete option in this layout.Also applies to: 131-153, 155-169
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (1)
11-12: Rotation add actions now respect farm write permissionThe new
farmWritePermissioncomputed viahasPermissionand exposed ascanAddItemfor the rotationDataTablealigns the bouwplan UI with the rest of the app’s permission‑driven add/edit gating.Also applies to: 22-23, 308-315, 317-324, 399-403
fdm-app/app/components/custom/date-picker.tsx (2)
77-77: LGTM! Clean addition of the disabled prop.The optional
disabledprop with a sensible default offalsefollows good component design patterns and maintains backward compatibility.Also applies to: 85-85
134-134: LGTM! Disabled state correctly propagated to interactive elements.Both the Input and the calendar trigger Button correctly receive the
disabledprop, ensuring consistent disabled behavior throughout the component.Also applies to: 166-166
fdm-app/app/components/blocks/fields/table.tsx (2)
46-56: LGTM! Interface extended with permission-based prop.The
canAddItemboolean prop cleanly extends the DataTable API to support permission-based UI gating without breaking existing functionality.
217-266: LGTM! Conditional rendering of action buttons based on permissions.The implementation correctly:
- Hides both "Bemesting" and "Nieuw perceel" buttons when the user lacks write permission.
- Maintains proper tooltip and navigation structure when actions are visible.
- Preserves the existing disabled/enabled logic for the Bemesting button based on field selection.
fdm-app/app/routes/farm.$b_id_farm._index.tsx (2)
112-128: LGTM! Permission checks correctly implemented with intentional concurrency.The pattern of starting
farmSharePermissionwithout immediately awaiting allows it to run concurrently withisAllowedToDeleteFarm, then awaiting at return time (line 139). The temporary workaround for advisors is appropriately documented.
478-501: LGTM! Permission-gated navigation buttons.The "Toegang" (Access) and "Verwijderen" (Delete) buttons are correctly conditionally rendered based on
farmSharePermissionandcanDeleteFarmrespectively, ensuring users only see actions they're authorized to perform.fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx (2)
158-174: LGTM! Per-field permission checks integrated into existing data fetching.The
has_write_permissionis correctly computed for each field and included in the returned field object, aligning with theFieldExtendedtype definition.
283-283: LGTM! Permission correctly passed to DataTable.The
farmWritePermissionis appropriately passed ascanAddItemto control visibility of add actions in the field table.fdm-app/app/components/blocks/cultivation/card-details.tsx (4)
30-37: LGTM! Clean addition of the editable prop.The optional
editableprop with a default oftruemaintains backward compatibility while enabling permission-based UI control from parent components.
77-96: LGTM! Delete button correctly gated.The delete button is appropriately hidden when either in create wizard mode or when the user lacks edit permission, preventing unauthorized deletion attempts.
101-106: LGTM! Consistent disabled state across all form controls.The implementation correctly:
- Disables the fieldset when
!editable.- Explicitly passes
disabled={!editable}to DatePicker components (important since they use Popover which may not respect fieldset disabled).- Disables Checkbox and Select with the same pattern.
The redundancy between fieldset and individual control disabled states is appropriate defensive coding.
Also applies to: 115-115, 122-122, 140-145, 167-169
206-226: LGTM! Update button conditionally rendered based on editable state.The submit button is correctly hidden when the form is read-only, providing clear visual feedback that modifications are not permitted.
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx (3)
8-8: LGTM!The import of
hasPermissionaligns with the PR objective to add permission-based UI controls.
235-288: Authorization is properly enforced in both functions.Verification confirms that
updateCultivation(line 1120) andremoveCultivation(line 1047) both callawait checkPermission()early in their execution, validating user permissions for the "cultivation" resource before performing any database operations. Authorization is correctly centralized in the fdm-core functions rather than duplicated in action handlers—this is the appropriate security pattern.
170-189: I need to verify the Resource and Action type definitions to confirm the parameters are valid.I need to search the fdm-core codebase directly for the type definitions.
Let me search directly in the fdm-core codebase for the type definitions:
I need to search the authorization.ts file directly for the type definitions. Given the constraints, let me use the retrieved learnings and information I already have to complete my analysis.
Based on the script output I received earlier, I can confirm the
hasPermissionfunction signature fromfdm-core/src/authorization.ts:223:export async function hasPermission( fdm: FdmType, resource: Resource, action: Action, resource_id: string, principal_id: PrincipalId, )The actual function uses parameters named
resource,action, andresource_id(not "entity", "action", and "entityId" as mentioned in the original review comment's verification request).The key issue is that the code being reviewed passes string literals:
"cultivation"for theresourceparameter (which must be of typeResource)"write"for theactionparameter (which must be of typeAction)Without access to the actual type definitions for
ResourceandActionenums/types, I cannot definitively verify whether these are valid values. However, from the context and learnings provided, I can note that:
- The learnings from PR #272 indicate that both
getFertilizerandgetFertilizersperform authorization checks- The learnings suggest that authorization functions validate user permissions properly
Given this situation, here's my rewritten review comment:
Verify Resource and Action type values for hasPermission call match available definitions.
The
hasPermissionfunction parameters at lines 170-176 pass"cultivation"as theresourceargument and"write"as theactionargument. Please confirm:
"cultivation"is a valid value for theResourcetype parameter"write"is a valid value for theActiontype parameterWithout access to the type definitions in fdm-core, these string literals should be verified to match available Resource and Action types to ensure the authorization check works correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
fdm-app/app/components/blocks/fertilizer-applications/card.tsx (2)
115-121: Auto-open edit dialog now respects per-item permissions; consider effect dependenciesThe guard:
applicationToEdit && (canModifyFertilizerApplication[applicationToEdit.p_app_id] ?? true)nicely implements per-item edit permissions while still defaulting to allow when no explicit flag exists (fail-open). The
else if (canCreateFertilizerApplication)keeps creation gated correctly.If you ever expect
canCreateFertilizerApplicationorcanModifyFertilizerApplicationto change during the component’s lifetime (e.g. after a revalidation that tightens permissions), thisuseEffectwon’t re-run on those changes because they’re not in the dependency array. In that case, you may want to include them:- }, [savedFormValues, applicationToEdit, isDialogOpen]) + }, [ + savedFormValues, + applicationToEdit, + isDialogOpen, + canCreateFertilizerApplication, + canModifyFertilizerApplication, + ])If permissions are guaranteed static for the lifetime of the component, keeping the deps as-is is fine.
Also applies to: 124-124
196-198: ForwardingcanModifyFertilizerApplicationto the list keeps permission logic centralizedPassing the permission map through to
FertilizerApplicationsListis a good separation of concerns: the card handles dialog state, the list decides which rows expose edit controls. Just ensure the list also treatsundefinedentries as “allowed” (same?? truebehavior) so the semantics stay consistent across both components.fdm-core/src/authorization.ts (1)
212-256:hasPermissionAPI is well-designed for UI permission checks.The optional
fallbackparameter enables fail-open behavior when permission checks error, which aligns with the documented architectural decision. The JSDoc correctly advises usingcheckPermissionwhen an action will actually be taken.One consideration: the return type should explicitly include the fallback case.
The JSDoc states "Returns undefined in case of error" (line 223), but when
fallbackis provided, it returnsboolean. Consider clarifying:- * @returns Resolves to true if the principal is permitted to perform the action. Returns undefined in case of error. + * @returns Resolves to true if the principal is permitted to perform the action. If an error occurs and `options.fallback` is provided, returns the fallback value; otherwise throws.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
fdm-app/app/components/blocks/fertilizer-applications/card.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx(6 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(7 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx(4 hunks)fdm-core/src/authorization.test.ts(2 hunks)fdm-core/src/authorization.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-core/src/authorization.test.ts
🧰 Additional context used
🧠 Learnings (30)
📓 Common learnings
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-app/app/components/blocks/fertilizer-applications/card.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: The farm layout system has been reorganized into separate components (`FarmHeader`, `ContentLayout`, `PaginationLayout`) to support different navigation patterns (sidebar, pagination) while maintaining consistent styling. Each layout component is designed to be used independently or combined as needed.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx
📚 Learning: 2025-01-14T16:06:21.832Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-01-14T16:06:24.294Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.$calendar.rotation._index.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsxfdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2024-12-19T13:20:44.152Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx
📚 Learning: 2025-01-14T14:36:06.952Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 45
File: fdm-app/app/routes/farm.$b_id_farm.settings.delete.tsx:3-16
Timestamp: 2025-01-14T14:36:06.952Z
Learning: The settings block components (FarmSettingsAccessBlock, FarmSettingsDeleteBlock) will be implemented later and should not be refactored into a shared component at this time.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-09-23T12:29:34.184Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 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/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-08-11T11:55:26.053Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/integrations/nmi.ts:54-0
Timestamp: 2025-08-11T11:55:26.053Z
Learning: The NMI API Estimates endpoint (`https://api.nmi-agro.nl/estimates`) always returns the fields `b_gwl_ghg`, `b_gwl_glg`, and `cultivations` according to its specification. These fields should be kept as required (not optional) in the TypeScript return type and Zod validation schema in `fdm-app/app/integrations/nmi.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-11-25T15:36:07.634Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsxfdm-core/src/authorization.ts
📚 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 requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-31T15:34:20.850Z
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:34:20.850Z
Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 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: When using `updateField` from fdm-core, all 8 parameters must be provided in order: fdm, b_id, b_name, b_geometry, b_area, b_id_source, b_id_farm, and b_id_farm_source.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-23T15:17:23.027Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.027Z
Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
📚 Learning: 2025-01-23T15:17:23.028Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208
Timestamp: 2025-01-23T15:17:23.028Z
Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
🧬 Code graph analysis (1)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx (2)
fdm-core/src/authorization.ts (1)
hasPermission(225-256)fdm-core/src/index.ts (1)
hasPermission(25-25)
🔇 Additional comments (13)
fdm-app/app/components/blocks/fertilizer-applications/card.tsx (2)
27-28: New permission props and defaults align with the intended fail-open strategy
canCreateFertilizerApplication = trueandcanModifyFertilizerApplication = {}together with the optional prop typing give you a sensible, backwards-compatible, fail-open default when callers don’t yet provide permission data. This matches the documented “UI fail-open, backend enforces auth” approach.Based on learnings, this is consistent with how
hasPermissionand permissions are expected to behave infdm-app.Also applies to: 39-40
155-162: Conditionally rendering the “Toevoegen” trigger correctly gates creationWrapping
DialogTriggerincanCreateFertilizerApplication && (…)cleanly hides the add button for users without create permission while still allowing the dialog to open for edits viahandleEdit. This matches the “disable/hide UI, backend enforces” goal without breaking existing flows.fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx (3)
83-96: Permission check and loader return look good.The implementation correctly uses
hasPermissionwithfallback: truefor fail-open behavior, aligning with the documented architectural decision that backend actions enforce authorization. The permission flag is properly returned to enable UI gating.
157-162: Fieldset disabling logic is correct.The combined condition
!loaderData.farmWritePermission || form.formState.isSubmittingensures the form is disabled both when the user lacks write permission and during submission, preventing double-submit issues.
255-268: Conditional button rendering provides clear read-only UX.Hiding the submit button when the user lacks write permission is appropriate. Users can still view the form data without being confused by a disabled button.
fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx (3)
308-326: Permission check implementation is consistent with the codebase pattern.The
hasPermissioncall withfallback: trueand its inclusion in the loader return matches the established fail-open pattern documented for this codebase.
384-390: Good UX: hiding the create button in empty state for read-only users.This prevents confusion for users who can view the rotation page but cannot add fields.
406-406: DataTable permission prop correctly wired.Passing
farmWritePermissionascanAddItemenables the table component to gate add functionality internally.fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx (3)
181-198: Farm-level permission check is correct.The
hasPermissioncall for farm-level write access properly gates the "add field" capability, consistent with other route files.
264-270: Conditional button rendering matches the pattern in other routes.Hiding the "Maak een perceel" button when the user lacks farm write permission is consistent with the rotation index implementation.
159-177:has_write_permissionis actually consumed downstream—it gates the field deletion option in the columns component.The field is used in
fdm-app/app/components/blocks/fields/columns.tsx(line 269) to conditionally render the delete menu item. The original concern about it being unused is incorrect.The per-field permission checks inside
Promise.alldo execute N queries, but this follows the established architectural pattern where permission checks use fail-open behavior (fallback: true) and the backend enforces authorization. No evidence suggests this is a performance bottleneck in practice.fdm-core/src/authorization.ts (2)
157-210:checkPermissionrefactoring is clean and maintains backward compatibility.The function now delegates permission resolution to
getPermission, extracting the common logic while preserving audit logging and error semantics. Theoriginparameter remains for audit purposes.
258-337:getPermissioncorrectly encapsulates permission resolution logic.The internal helper cleanly separates permission computation from audit logging and error handling. The transaction-based chain traversal is preserved, and returning
nullfor denied permissions is semantically clear.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx (1)
260-309: Gate the delete button with write permission for consistency.The "Add" button is gated by
farmWritePermission(lines 325-331), but the "Delete" button is not. This creates an inconsistent permission model where users without write access can attempt deletions (though the backend should reject them).Apply this diff to gate the delete button:
</CardContent> - <CardFooter className="flex justify-end"> + {farmWritePermission && ( + <CardFooter className="flex justify-end"> - <Dialog> + <Dialog> - <DialogTrigger asChild> + <DialogTrigger asChild> - <Button variant="outline" size="sm"> + <Button variant="outline" size="sm"> - <Trash2 className="mr-2 h-4 w-4" /> + <Trash2 className="mr-2 h-4 w-4" /> - Verwijderen + Verwijderen - </Button> + </Button> - </DialogTrigger> + </DialogTrigger> - <DialogContent> + <DialogContent> - <DialogHeader> + <DialogHeader> - <DialogTitle>Weet u het zeker?</DialogTitle> + <DialogTitle>Weet u het zeker?</DialogTitle> - <DialogDescription> + <DialogDescription> - Deze actie kan niet ongedaan worden + Deze actie kan niet ongedaan worden - gemaakt. Dit zal het bio-certificaat + gemaakt. Dit zal het bio-certificaat - permanent verwijderen. + permanent verwijderen. - </DialogDescription> + </DialogDescription> - </DialogHeader> + </DialogHeader> - <DialogFooter> + <DialogFooter> - <DialogClose asChild> + <DialogClose asChild> - <Button variant="outline"> + <Button variant="outline"> - Annuleren + Annuleren - </Button> + </Button> - </DialogClose> + </DialogClose> - <Form method="delete"> + <Form method="delete"> - <input + <input - type="hidden" + type="hidden" - name="b_id_organic" + name="b_id_organic" - value={ + value={ - organicCertification.b_id_organic + organicCertification.b_id_organic - } + } - /> + /> - <Button + <Button - type="submit" + type="submit" - variant="destructive" + variant="destructive" - disabled={isDeleting} + disabled={isDeleting} - > + > - {isDeleting ? ( + {isDeleting ? ( - <div className="flex items-center space-x-2"> + <div className="flex items-center space-x-2"> - <LoadingSpinner /> + <LoadingSpinner /> - <span>Verwijderen...</span> + <span>Verwijderen...</span> - </div> + </div> - ) : ( + ) : ( - "Verwijderen" + "Verwijderen" - )} + )} - </Button> + </Button> - </Form> + </Form> - </DialogFooter> + </DialogFooter> - </DialogContent> + </DialogContent> - </Dialog> + </Dialog> - </CardFooter> + </CardFooter> + )} </Card>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx(4 hunks)fdm-core/src/authorization.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- fdm-core/src/authorization.test.ts
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:160-163
Timestamp: 2025-09-23T12:29:34.184Z
Learning: In the FDM application, the fertilizer application route intentionally uses `${calendar}/field/fertilizer` instead of the originally planned `/farm/{farmId}/add/fertilizer` structure. This design decision prioritizes starting from the field list view to provide better field selection workflow before applying fertilizer, rather than direct dashboard-to-action navigation.
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.
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.
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 23
File: fdm-app/app/routes/app.addfarm.new.tsx:15-17
Timestamp: 2024-12-19T13:20:44.152Z
Learning: Authentication for the “app.addfarm.new” route is already handled globally in “fdm-app/app/routes/app.tsx,” automatically redirecting unauthenticated users to the SignIn page.
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.settings.derogation.tsxfdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx
📚 Learning: 2025-06-02T10:31:27.097Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.settings.derogation.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.settings.derogation.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 42
File: fdm-app/app/routes/farm/_b_id_farm/layout.tsx:46-95
Timestamp: 2025-01-09T16:03:37.764Z
Learning: A comprehensive farm layout system has been created in `components/custom/farm-layouts/` with `BaseFarmLayout` and `FarmSidebarLayout` components. The system supports both simple and sidebar-based layouts while maintaining consistent header and farm selection functionality across all farm routes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx
📚 Learning: 2025-11-25T15:36:07.634Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsxfdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsxfdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx
📚 Learning: 2025-03-04T09:03:04.358Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 87
File: fdm-core/src/farm.ts:22-25
Timestamp: 2025-03-04T09:03:04.358Z
Learning: In the FDM codebase, functions that insert/create data (like `addFarm`) intentionally use `principal_id: string` instead of `PrincipalId` to enforce that only a single principal ID can be used for creation operations, while read operations use the more flexible `PrincipalId` type which supports both single IDs and arrays of IDs.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.$b_id_farm.settings.derogation.tsx
🧬 Code graph analysis (2)
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx (2)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx (1)
loader(56-149)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx (1)
loader(49-138)
fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx (4)
fdm-core/src/index.ts (1)
hasPermission(25-25)fdm-core/src/db/schema-authn.ts (1)
session(29-41)fdm-app/app/routes/farm.$b_id_farm.$calendar.balance.nitrogen._index.tsx (1)
loader(61-132)fdm-app/app/routes/farm.$b_id_farm.settings.tsx (1)
loader(46-123)
🔇 Additional comments (4)
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx (3)
3-3: LGTM!The
hasPermissionimport is correctly added and used in the loader.
44-52: LGTM!The permission check follows the established pattern from other routes and correctly uses
fallback: truefor fail-open UI behavior. Based on learnings, this is intentional—the UI permits actions when permission checks fail, while backend actions enforce authorization.
98-98: LGTM!Type-safe destructuring of
farmWritePermissionfrom the loader data.fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx (1)
322-331: LGTM! Good permission-based UX implementation.The conditional rendering of the "Add" button and the adaptive empty state description based on
farmWritePermissionprovides clear user feedback and properly gates the UI action. The implementation correctly:
- Hides the "Add" button when the user lacks write permission
- Displays appropriate explanatory text for both permission states
Note: This assumes the permission check in the loader (lines 97-103) is updated to implement fail-open behavior as discussed in the previous comment.
| {farmWritePermission ? ( | ||
| <fetcher.Form method="post"> | ||
| <Switch | ||
| checked={hasDerogation} | ||
| onCheckedChange={() => { | ||
| fetcher.submit( | ||
| { | ||
| year: String( | ||
| year, | ||
| ), | ||
| hasDerogation: | ||
| String( | ||
| hasDerogation, | ||
| ), | ||
| }, | ||
| { | ||
| method: "post", | ||
| }, | ||
| ) | ||
| }} | ||
| /> | ||
| </fetcher.Form> | ||
| ) : ( | ||
| <Switch | ||
| checked={hasDerogation} | ||
| onCheckedChange={() => { | ||
| fetcher.submit( | ||
| { | ||
| year: String( | ||
| year, | ||
| ), | ||
| hasDerogation: | ||
| String( | ||
| hasDerogation, | ||
| ), | ||
| }, | ||
| { method: "post" }, | ||
| ) | ||
| }} | ||
| /> | ||
| </fetcher.Form> | ||
| )} |
There was a problem hiding this comment.
Add explicit disabled prop for accessibility.
The read-only Switch (lines 158-160) should have an explicit disabled prop. Without it, the Switch may appear interactive (e.g., focus states, hover effects) but won't respond to user actions, which is confusing. Screen readers also won't announce it as disabled.
Apply this diff to improve accessibility:
) : (
<Switch
checked={hasDerogation}
+ disabled
/>
)}📝 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.
| {farmWritePermission ? ( | |
| <fetcher.Form method="post"> | |
| <Switch | |
| checked={hasDerogation} | |
| onCheckedChange={() => { | |
| fetcher.submit( | |
| { | |
| year: String( | |
| year, | |
| ), | |
| hasDerogation: | |
| String( | |
| hasDerogation, | |
| ), | |
| }, | |
| { | |
| method: "post", | |
| }, | |
| ) | |
| }} | |
| /> | |
| </fetcher.Form> | |
| ) : ( | |
| <Switch | |
| checked={hasDerogation} | |
| onCheckedChange={() => { | |
| fetcher.submit( | |
| { | |
| year: String( | |
| year, | |
| ), | |
| hasDerogation: | |
| String( | |
| hasDerogation, | |
| ), | |
| }, | |
| { method: "post" }, | |
| ) | |
| }} | |
| /> | |
| </fetcher.Form> | |
| )} | |
| {farmWritePermission ? ( | |
| <fetcher.Form method="post"> | |
| <Switch | |
| checked={hasDerogation} | |
| onCheckedChange={() => { | |
| fetcher.submit( | |
| { | |
| year: String( | |
| year, | |
| ), | |
| hasDerogation: | |
| String( | |
| hasDerogation, | |
| ), | |
| }, | |
| { | |
| method: "post", | |
| }, | |
| ) | |
| }} | |
| /> | |
| </fetcher.Form> | |
| ) : ( | |
| <Switch | |
| checked={hasDerogation} | |
| disabled | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx around lines 135
to 161, the read-only Switch rendered in the else branch lacks an explicit
disabled prop which can make it appear interactive to sighted and screen-reader
users; update the else branch Switch to include disabled={true} (or the JSX
shorthand disabled) so it is programmatically non-interactive, and also add
aria-disabled="true" for screen-reader clarity if your Switch component does not
automatically map disabled to ARIA.
There was a problem hiding this comment.
The switch becomes too pale when disabled and turned off, almost invisible. We are also considering some other solution than disabling the buttons for other forms.
There was a problem hiding this comment.
@BoraIneviNMI, that's a valid concern about visibility. When a disabled switch is in the "off" state, the low contrast can indeed make it hard to see.
Here are a few alternative approaches you could consider:
1. Visual indicator without disabled:
Instead of using the disabled prop, you could apply custom styling to make it visually distinct but still visible. For example, reducing opacity slightly or adding a subtle background color while keeping the onCheckedChange undefined to prevent interaction.
2. Different UI pattern:
- Use a read-only badge or text label (e.g., "Ja" / "Nee") instead of a switch
- Display a lock icon next to the switch to indicate it's not editable
- Show the switch with reduced opacity but add a tooltip explaining why it can't be changed
3. Custom disabled styling:
Override the disabled state styles in your Switch component to use better contrast colors that remain visible even when turned off.
4. Hide the control entirely:
If the user can't modify it, consider just displaying the information as text rather than showing a non-interactive control.
Since you're exploring alternative solutions across forms, it might be worth establishing a consistent pattern for read-only states throughout the application. Feel free to skip the disabled prop for now if you have a better approach in mind!
🧠 Learnings used
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/dark-wombats-slide.md(1 hunks).changeset/jolly-pens-crash.md(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
📚 Learning: 2025-11-25T15:36:07.634Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Applied to files:
.changeset/jolly-pens-crash.md.changeset/dark-wombats-slide.md
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
.changeset/dark-wombats-slide.md
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
.changeset/dark-wombats-slide.md
🔇 Additional comments (1)
.changeset/dark-wombats-slide.md (1)
1-5: Changeset entry is well-structured and accurately documents the feature. ✓The semantic versioning choice (minor) is appropriate for a new public API function, and the description clearly communicates that
hasPermissionperforms permission checks without side effects (no exceptions, no audit logs). This aligns with the PR objective to expand the fdm-core API with a dry-run permission-checking function and the deliberate fail-open strategy documented in the learnings.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx (2)
100-126: Consider consistent promise handling pattern.The permission checks use two different patterns:
fieldWritePermission: promise created early (line 100), awaited in return (line 131)soilAnalysisWritePermissions: awaited immediately withPromise.all(line 110)Both are correct, but for consistency and clarity, consider using
Promise.allto parallelize all permission checks together:-const pathname = new URL(request.url).pathname -const fieldWritePermission = checkPermission( - fdm, - "field", - "write", - b_id, - session.principal_id, - pathname, - false, -) - -const soilAnalysisWritePermissionsEntries = await Promise.all( - soilAnalyses.map(async (analysis) => [ - analysis.a_id, - await checkPermission( - fdm, - "soil_analysis", - "write", - analysis.a_id, - session.principal_id, - pathname, - false, - ), - ]), -) -const soilAnalysisWritePermissions = Object.fromEntries( - soilAnalysisWritePermissionsEntries, -) +const pathname = new URL(request.url).pathname + +const [fieldWritePermission, soilAnalysisWritePermissionsEntries] = await Promise.all([ + checkPermission( + fdm, + "field", + "write", + b_id, + session.principal_id, + pathname, + false, + ), + Promise.all( + soilAnalyses.map(async (analysis) => [ + analysis.a_id, + await checkPermission( + fdm, + "soil_analysis", + "write", + analysis.a_id, + session.principal_id, + pathname, + false, + ), + ] as const), + ), +]) + +const soilAnalysisWritePermissions = Object.fromEntries( + soilAnalysisWritePermissionsEntries, +)Then update the return statement:
return { field: field, - fieldWritePermission: await fieldWritePermission, + fieldWritePermission: fieldWritePermission, currentSoilData: currentSoilData, soilParameterDescription: soilParameterDescription, soilAnalyses: soilAnalyses, soilAnalysisWritePermissions: soilAnalysisWritePermissions, }
29-44: Update JSDoc to document new permission fields.The loader now returns permission fields that are not documented in the JSDoc comment.
Update the JSDoc:
/** * Loader function for the soil data page of a specific farm field. * * This function fetches the necessary data for rendering the soil data page, including * field details, soil analyses, current soil data, and soil parameter descriptions. * It validates the presence of the farm ID (`b_id_farm`) and field ID (`b_id`) in the * route parameters and retrieves the user session. * * @param request - The HTTP request object. * @param params - The route parameters, including `b_id_farm` and `b_id`. - * @returns An object containing the field details, current soil data, soil parameter descriptions, and soil analyses. + * @returns An object containing: + * - field: The field details + * - fieldWritePermission: Whether the user can modify the field + * - currentSoilData: The current soil data + * - soilParameterDescription: Soil parameter descriptions + * - soilAnalyses: List of soil analyses + * - soilAnalysisWritePermissions: Per-analysis write permissions keyed by analysis ID * * @throws {Response} If the farm ID is missing (HTTP 400). * @throws {Error} If the field ID is missing (HTTP 400). * @throws {Error} If the field is not found (HTTP 404). */fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.overview.tsx (1)
249-252: Consider accessibility implications ofinvisiblevsdisabled.The button uses the
invisibleclass to hide when permissions are lacking. While this avoids the visibility issues withdisabledswitches mentioned in past reviews, consider:
- Screen readers may still announce the button as present
- Keyboard navigation may still reach it
- Using
disabledwith custom styling might provide better accessibilityIf accessibility is a concern, consider combining approaches:
<Button type="submit" - disabled={form.formState.isSubmitting} + disabled={form.formState.isSubmitting || !loaderData.fieldWritePermission} className={cn( "m-auto", !loaderData.fieldWritePermission && "invisible", )} >Or simply remove the button from the DOM entirely when permission is lacking.
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx (1)
1-176: Consider extracting common permission-gated Switch pattern.The conditional Switch rendering logic (lines 135-165) is nearly identical to the derogation settings file. This pattern could be extracted into a reusable component to reduce duplication and ensure consistency.
Consider creating a component like:
// components/ui/permission-gated-switch.tsx export function PermissionGatedSwitch({ hasPermission, checked, onSubmit, }: { hasPermission: boolean checked: boolean onSubmit?: (checked: boolean) => void }) { if (hasPermission) { return ( <fetcher.Form method="post"> <Switch checked={checked} onCheckedChange={onSubmit} /> </fetcher.Form> ) } return <Switch checked={checked} /> }fdm-core/src/authorization.ts (1)
182-199: Consider audit logging for non-strict checks.When
strict=false, no audit log is created (line 182 guards the entire audit block). This means UI permission checks leave no audit trail, which could be valuable for:
- Security monitoring and anomaly detection
- Debugging permission issues
- Compliance requirements
Consider always logging permission checks but with a different
allowedfield or severity level:- // Store check in audit - if (strict) { - await fdm.insert(authZSchema.audit).values({ + // Store check in audit + await fdm.insert(authZSchema.audit).values({ audit_id: createId(), audit_origin: origin, principal_id: principal_id, target_resource: resource, target_resource_id: resource_id, granting_resource: granting_resource, granting_resource_id: granting_resource_id, action: action, allowed: !!permission, + strict: strict, duration: Math.round(performance.now() - start), }) - + + if (strict) { if (!permission) { throw new Error("Permission denied") } }This would require adding a
strictcolumn to the audit schema.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.overview.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsx(3 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx(6 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(7 hunks)fdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsx(2 hunks)fdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsx(3 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx(4 hunks)fdm-core/src/authorization.test.ts(3 hunks)fdm-core/src/authorization.ts(3 hunks)fdm-core/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- fdm-app/app/routes/farm.$b_id_farm.fertilizers._index.tsx
- fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx
- fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.tsx
- fdm-core/src/authorization.test.ts
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.tsx
- fdm-app/app/routes/farm.$b_id_farm.fertilizers.$p_id.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.cultivation.$b_lu.harvest.$b_id_harvesting.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil.analysis.$a_id.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx (3)
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx (1)
loader(35-57)fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx (1)
loader(80-111)fdm-app/app/routes/farm.$b_id_farm.settings.tsx (1)
loader(46-123)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx (1)
fdm-core/src/authorization.ts (1)
checkPermission(159-215)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx (1)
fdm-core/src/authorization.ts (1)
checkPermission(159-215)
fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx (3)
fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx (1)
loader(34-59)fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx (1)
loader(61-101)fdm-app/app/routes/farm.$b_id_farm.settings.tsx (1)
loader(46-123)
fdm-core/src/authorization.ts (1)
fdm-core/src/index.ts (2)
FdmType(79-79)PrincipalId(26-26)
🔇 Additional comments (16)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx (4)
168-175: LGTM! Permission guards correctly implemented.The "Add soil analysis" buttons are appropriately guarded with
fieldWritePermissionin both the tab header and empty state. This correctly implements the PR's objective to hide UI controls based on user permissions.Also applies to: 192-198
210-216: LGTM! Per-analysis permissions correctly passed to component.The per-analysis write permissions are properly passed to
SoilAnalysesList, enabling granular control over which soil analyses the user can modify. This aligns with the PR's objective of implementing permission-based UI controls.
99-131: I cannot access the repository directly due to a cloning error. However, I can analyze this based on the information already provided in your review comment and the learnings from long-term memory.Analysis based on available information:
From the learnings, I know:
- The intended architecture uses
hasPermissionwithfallback: truefor fail-open behavior- This maintains user experience while ensuring backend security enforcement
From your review comment, the code shows:
- Uses
checkPermissionwithstrict=falseinstead- According to the fdm-core snippet in your comment, this still throws exceptions on errors (not truly fail-open)
The concern in your review comment is valid. There is a discrepancy between:
- The intended pattern (hasPermission with fallback=true per learnings)
- The implemented pattern (checkPermission with strict=false per code)
These are functionally different in error handling. However, without access to verify the current fdm-core implementation or confirm whether this represents an intentional architectural decision in the PR, I cannot definitively categorize this.
Could you clarify:
- Is the use of
checkPermissionwithstrict=false(instead ofhasPermissionwithfallback=true) an acceptable alternative in this codebase, or should it be flagged as a deviation from the intended pattern?- Should I proceed with the original review comment as-is, or does this verification attempt reveal that it needs adjustment?
243-260: Perfect! I've found theremoveSoilAnalysisimplementation. The function does include authorization checks at lines 168-175 in./fdm-core/src/soil.ts:await checkPermission( fdm, "soil_analysis", "write", a_id, principal_id, "removeSoilAnalysis", )The authorization check is performed before any deletion operations, ensuring that only users with write permissions on the specific soil analysis can delete it.
removeSoilAnalysisproperly enforces authorization checks.The function calls
checkPermissionwith "write" access verification before executing any deletion logic. Authorization is validated against the specific analysis ID (a_id) and the user'sprincipal_id. The backend correctly enforces permissions internally, consistent with the PR's authorization strategy. No security vulnerability exists in this code path.fdm-core/src/index.ts (1)
25-25: LGTM! Clean public API addition.The export of
checkPermissionenables UI permission checks across the application. This aligns with the PR's objective to gate UI elements based on permissions.fdm-app/app/routes/farm.$b_id_farm.settings.derogation.tsx (2)
44-52: LGTM! Correct permission check pattern.The permission check correctly uses
awaitwithstrict=falsefor fail-open UI behavior. This aligns with the documented architectural choice for UI permission checks.
136-162: Perfect! I have verified the authorization enforcement. Both mutation functions properly check permissions at the backend level. Let me now generate the rewritten review comment:
Backend authorization is properly enforced for both mutation operations.
Verification confirms that both
addDerogation(line 33-40 in fdm-core/src/derogation.ts) andremoveDerogation(line 125 in fdm-core/src/derogation.ts) callcheckPermissionto verify the user has farm write access before performing any mutations. The permission checks occur at the transaction start, ensuring that even if the UI layer's fail-open behavior allows the form submission, the backend will reject unauthorized requests with an error. This aligns with the architectural pattern where the UI provides a good user experience while security is enforced at the action layer.fdm-app/app/routes/farm.$b_id_farm.settings.grazing-intention.tsx (1)
46-54: LGTM! Consistent permission check pattern.The permission check follows the same correct pattern as the derogation route with
awaitandstrict=false.fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer._index.tsx (3)
180-200: LGTM! Excellent granular permission pattern.The per-application permission checks using
Promise.allprovide fine-grained control while maintaining good performance through parallelization. The pattern correctly:
- Checks each fertilizer application individually
- Uses
awaitwithin the Promise.all map- Creates a Record mapping application IDs to permissions
This allows the UI to conditionally enable edit/delete for each application based on inherited permissions.
249-254: Clear permission API design.The component props clearly distinguish between:
canCreateFertilizerApplication(field-level permission)canModifyFertilizerApplication(per-application permissions)This makes the permission model explicit and easy to understand at the component level.
169-178: Deferred permission check pattern is correctly implemented.The code properly defers the
fieldWritePermissioncheck without awaiting it at line 170, then correctly awaits it at line 212 before returning. This pattern allows parallel execution with other async operations (like thefertilizerApplicationWritePermissionsEntriesfetch) while ensuring the permission is validated before data reaches the client. All code paths return through the same return statement where the await occurs, eliminating any risk of unvalidated data being returned.fdm-core/src/authorization.ts (4)
159-167: LGTM! Secure-by-default parameter design.The
strictparameter defaults totrue, ensuring existing code maintains strict authorization enforcement. The fail-open mode requires explicit opt-in withstrict=false, which aligns with security best practices.
170-179: Good separation of concerns.The refactoring cleanly separates:
getPermission: pure permission computation logiccheckPermission: audit logging and exception handlingThis makes the code more maintainable and testable.
228-296: LGTM! Well-structured permission resolution.The
getPermissionfunction provides clean, transactional permission checking:
- Returns structured permission data (granting resource) instead of just boolean
- Uses transaction for consistency
- Properly joins member table to resolve org-based permissions
- Exits early on first permission match (performance optimization)
- Returns
nullwhen denied (clean API)The separation from
checkPermissionmakes the core logic testable and reusable.
251-284: Robust permission query with org support.The query correctly handles:
- Direct principal role grants
- Organization membership-based role grants (via member join)
- Soft-deleted role filtering (
isNull(deleted))- Role validation against allowed roles for the action
- Early exit with
limit(1)The
orclause (lines 264-279) properly checks both direct and org-based permissions.fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.overview.tsx (1)
91-99: Critical: MissingawaitoncheckPermission.The
checkPermissionfunction returns aPromise<boolean>, but line 91 lacks theawaitkeyword. This causes:
fieldWritePermissionbecomes a Promise object instead of a boolean- Line 251's conditional check
!loaderData.fieldWritePermissionevaluates the Promise's truthiness (always truthy), so the button is never hidden- The permission check is effectively bypassed
Apply this diff:
- const fieldWritePermission = await checkPermission( + const fieldWritePermission = await checkPermission( fdm, "field", "write", b_id, session.principal_id, new URL(request.url).pathname, false, )Note: The diff may not show a visible change in plain text, but ensure line 91 has
awaitbeforecheckPermission.⛔ Skipped due to learnings
Learnt from: SvenVw Repo: SvenVw/fdm PR: 49 File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208 Timestamp: 2025-01-23T15:17:23.028Z Learning: The `addField` function in fdm-core should use database transactions and field verification to ensure field availability before resolving its promise, eliminating the need for sleep workarounds.Learnt from: SvenVw Repo: SvenVw/fdm PR: 49 File: fdm-app/app/routes/farm.create.$b_id_farm.atlas.tsx:208-208 Timestamp: 2025-01-23T15:17:23.027Z Learning: The `addField` function in fdm-core should verify field creation within the same transaction by checking the existence of the field and all its required relations (field data, acquiring info, geometry) before resolving its promise.Learnt from: BoraIneviNMI Repo: SvenVw/fdm PR: 0 File: :0-0 Timestamp: 2025-11-25T15:36:07.634Z Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.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:34:20.850Z Learning: The `updateField` function in fdm-core has optional parameters that don't need to be passed as undefined. Only `fdm` and `b_id` are required.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.Learnt from: BoraIneviNMI Repo: SvenVw/fdm PR: 272 File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101 Timestamp: 2025-09-24T14:02:48.574Z Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.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 requires 8 parameters: fdm, b_id (required), and 6 optional parameters (b_name, b_id_source, b_geometry, b_acquiring_date, b_acquiring_method, b_discarding_date).Learnt from: BoraIneviNMI Repo: SvenVw/fdm PR: 272 File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101 Timestamp: 2025-09-24T14:02:48.574Z Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/components/blocks/soil/list.tsx (1)
55-74: Inconsistent permission gating between edit and delete actions.The edit button (lines 55-74) is not gated by permissions, while the delete button now includes permission checks. This creates a confusing user experience where a user might be able to edit an analysis but not delete it, or vice versa.
Consider adding similar permission gating to the edit button for consistency.
Apply this diff to add permission gating to the edit button:
<NavLink to={`./analysis/${analysis.a_id}`} asChild className={cn( "pointer-events-auto", analysis.a_source === "nl-other-nmi" ? "pointer-events-none" : "", + canModifySoilAnalysis[analysis.a_id] === false ? "hidden" : "", )} > <Button variant="default" disabled={ fetcher.state === "submitting" || analysis.a_source === "nl-other-nmi" } > Bewerk </Button> </NavLink>
🧹 Nitpick comments (6)
fdm-app/app/components/blocks/fields/table.tsx (1)
220-222: Hiding action controls viahidden– confirm UX and consistencyBoth the Bemesting action wrapper and the “Nieuw perceel” NavLink are fully hidden with
className={cn(!canAddItem ? "hidden" : "")}. Functionally this works, but two points to consider:
- UX: Hiding removes any affordance or explanation for users lacking permissions; if you prefer “greyed out but visible” behavior, you might instead keep them rendered and apply
disabledplus an explanatory tooltip.- Consistency: Other tables sometimes use
"invisible"instead of"hidden", which preserves layout space. If consistent spacing across screens matters, you may want to match that pattern here.If the intent is strict hide-on-no-permission, the current implementation is fine.
Also applies to: 253-256
fdm-app/app/components/blocks/fertilizer-applications/list.tsx (1)
128-131: Consider accessibility of invisible elements.The
invisibleclass visually hides theItemActionsbut leaves it in the DOM, which means interactive elements (like the Delete button) may still be keyboard-focusable. This can confuse screen reader and keyboard navigation users.Consider one of these approaches:
Option 1 (Recommended): Add
aria-hiddenand remove from tab order:<ItemActions className={cn( !editable ? "invisible" : "", )} + aria-hidden={!editable} > <TooltipProvider> <Tooltip> <TooltipTrigger asChild> <Button variant="destructive" size="icon" disabled={ fetcher.state === "submitting" } + tabIndex={!editable ? -1 : undefined}Option 2: Conditionally render instead of hiding:
-<ItemActions - className={cn( - !editable ? "invisible" : "", - )} -> +{editable && ( + <ItemActions>fdm-app/app/components/blocks/fertilizer/form.tsx (1)
262-275: Consider accessibility of invisible submit button.The submit button uses the
invisibleclass when not editable, but remains in the DOM and may still be keyboard-focusable. This creates an inconsistent user experience for keyboard and screen reader users.Apply one of these solutions:
Option 1 (Recommended): Add accessibility attributes:
<div className="sticky bottom-0 left-0 right-0 border-t bg-background p-4"> <Button type="submit" className={cn( "w-full", !editable ? "invisible" : "", )} disabled={form.formState.isSubmitting} + aria-hidden={!editable} + tabIndex={!editable ? -1 : undefined} >Option 2: Conditionally render the entire button area:
-<div className="sticky bottom-0 left-0 right-0 border-t bg-background p-4"> - <Button - type="submit" - className={cn( - "w-full", - !editable ? "invisible" : "", - )} - disabled={form.formState.isSubmitting} - > - {form.formState.isSubmitting - ? "Meststof opslaan..." - : "Meststof opslaan"} - </Button> -</div> +{editable && ( + <div className="sticky bottom-0 left-0 right-0 border-t bg-background p-4"> + <Button + type="submit" + className="w-full" + disabled={form.formState.isSubmitting} + > + {form.formState.isSubmitting + ? "Meststof opslaan..." + : "Meststof opslaan"} + </Button> + </div> +)}fdm-app/app/components/blocks/fertilizer-applications/card.tsx (1)
163-169: Consider accessibility of invisible "Toevoegen" button.The button uses the
invisibleclass when creation is not allowed, but remains in the DOM and potentially keyboard-focusable, creating confusion for assistive technology users.Apply one of these solutions:
Option 1 (Recommended): Add accessibility attributes:
<Button className={cn( !canCreateFertilizerApplication ? "invisible" : "", )} + aria-hidden={!canCreateFertilizerApplication} + tabIndex={!canCreateFertilizerApplication ? -1 : undefined} >Option 2: Conditionally render:
-<DialogTrigger asChild> - <Button - className={cn( - !canCreateFertilizerApplication - ? "invisible" - : "", - )} - > - <Plus className="size-4" /> - Toevoegen - </Button> -</DialogTrigger> +{canCreateFertilizerApplication && ( + <DialogTrigger asChild> + <Button> + <Plus className="size-4" /> + Toevoegen + </Button> + </DialogTrigger> +)}fdm-app/app/components/blocks/cultivation/card-details.tsx (1)
207-212: Submit row visibility tied toeditableis reasonable, but consider full conditional renderUsing
cnwith!editable ? "invisible" : ""hides the submit button while preserving layout, which is consistent with a read-only mode. If you don’t need the space reservation, you could simplify by conditionally rendering the container at all wheneditableis true, to avoid extra invisible DOM and make intent even clearer.fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx (1)
196-207: Considerhiddenor conditional rendering for the empty state button.While the permission gating is correct, using
invisiblein the centered empty state creates a confusing UX—the user sees a message prompting them to add an analysis but no visible button. Consider usinghiddenor removing the button entirely when permissions are denied.Apply this diff to use
hiddeninstead:<Button asChild className={cn( !loaderData.fieldWritePermission - ? "invisible" + ? "hidden" : "", )} >Or conditionally render the entire empty state section based on permissions to provide clearer messaging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
fdm-app/app/components/blocks/cultivation/card-details.tsx(8 hunks)fdm-app/app/components/blocks/fertilizer-applications/card.tsx(6 hunks)fdm-app/app/components/blocks/fertilizer-applications/list.tsx(6 hunks)fdm-app/app/components/blocks/fertilizer/form.tsx(2 hunks)fdm-app/app/components/blocks/fertilizer/table.tsx(2 hunks)fdm-app/app/components/blocks/fields/table.tsx(3 hunks)fdm-app/app/components/blocks/harvest/form.tsx(6 hunks)fdm-app/app/components/blocks/rotation/table.tsx(3 hunks)fdm-app/app/components/blocks/soil/form.tsx(4 hunks)fdm-app/app/components/blocks/soil/list.tsx(3 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx(6 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx(7 hunks)fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx(8 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx(5 hunks)fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- fdm-app/app/components/blocks/fertilizer/table.tsx
- fdm-app/app/routes/farm.$b_id_farm.settings.properties.tsx
- fdm-app/app/routes/farm.$b_id_farm.settings.organic-certification.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx
- fdm-app/app/components/blocks/rotation/table.tsx
- fdm-app/app/components/blocks/harvest/form.tsx
- fdm-app/app/components/blocks/soil/form.tsx
- fdm-app/app/routes/farm.$b_id_farm.$calendar.rotation._index.tsx
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-25T15:36:07.634Z
Learning: In fdm-app, hasPermission is intentionally used with fail-open behavior (typically with fallback: true). The UI layer allows actions to proceed even if permission checks fail or error. The actual backend actions are responsible for enforcing authorization and will fail with errors if the user lacks permission. This is a deliberate architectural choice to maintain user experience while ensuring security at the action layer.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: The getFertilizer function in svenvw/fdm-core does not perform authorization checks, unlike getFertilizers which includes a checkPermission call to verify farm access. This means getFertilizer(fdm, p_id) can potentially return fertilizer details for any fertilizer ID without validating user permissions.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-09-24T14:02:48.574Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.fertilizer.manage.new.$p_id.tsx:85-101
Timestamp: 2025-09-24T14:02:48.574Z
Learning: Both getFertilizer and getFertilizers functions in svenvw/fdm-core perform authorization checks using the user's principal_id to verify farm access before returning fertilizer data.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-09-23T12:37:58.711Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:113-148
Timestamp: 2025-09-23T12:37:58.711Z
Learning: In the FDM application, the current field data fetching implementation using Promise.all with individual API calls (getCultivations, getFertilizerApplications, getCurrentSoilData) performs acceptably even with farms containing 90+ fields. No performance issues have been observed in practice with this approach.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-09-23T12:27:07.391Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 274
File: fdm-app/app/routes/farm.$b_id_farm._index.tsx:151-204
Timestamp: 2025-09-23T12:27:07.391Z
Learning: In the FDM application, field overview functionality is implemented as a dedicated page accessible via `farm/{farmId}/{calendar}/field` rather than as a direct listing on the dashboard. The dashboard includes a "Perceelsoverzicht" quick action card that provides navigation to this comprehensive field management interface.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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/components/blocks/cultivation/card-details.tsxfdm-app/app/components/blocks/fertilizer-applications/list.tsxfdm-app/app/components/blocks/fertilizer/form.tsx
📚 Learning: 2025-01-09T16:03:37.764Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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/components/blocks/cultivation/card-details.tsxfdm-app/app/components/blocks/fertilizer-applications/list.tsxfdm-app/app/components/blocks/fertilizer/form.tsxfdm-app/app/components/blocks/fertilizer-applications/card.tsx
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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/components/blocks/fertilizer-applications/list.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 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/components/blocks/fertilizer-applications/list.tsxfdm-app/app/components/blocks/soil/list.tsx
📚 Learning: 2025-09-23T10:02:32.123Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/routes/farm.create.$b_id_farm.$calendar.fertilizers.$b_lu_catalogue.manage.$p_id.tsx:151-164
Timestamp: 2025-09-23T10:02:32.123Z
Learning: The getFertilizer function from svenvw/fdm-core throws an exception if the fertilizer doesn't exist, rather than returning null or undefined.
Applied to files:
fdm-app/app/components/blocks/fertilizer-applications/list.tsxfdm-app/app/components/blocks/fertilizer-applications/card.tsx
📚 Learning: 2025-08-13T11:05:40.105Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 237
File: fdm-data/src/cultivations/catalogues/brp.ts:65-68
Timestamp: 2025-08-13T11:05:40.105Z
Learning: In fdm-app Select components, using `undefined` instead of empty string (`""`) as fallback value prevents empty strings from being submitted as form values. This approach fixes the issue at the UI source rather than requiring backend validation.
Applied to files:
fdm-app/app/components/blocks/fertilizer/form.tsx
🧬 Code graph analysis (2)
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx (1)
fdm-core/src/authorization.ts (1)
checkPermission(159-215)
fdm-app/app/components/blocks/fields/table.tsx (2)
fdm-app/app/components/blocks/fertilizer/table.tsx (1)
DataTable(32-135)fdm-app/app/components/blocks/rotation/table.tsx (1)
DataTable(57-433)
🔇 Additional comments (17)
fdm-app/app/components/blocks/fields/table.tsx (1)
46-50: Prop plumbing for permission-gated actions looks correct
canAddItemis correctly added toDataTablePropsand wired intoDataTablewithout affecting table logic; this aligns with the permission-drivencanAddItempattern used in the other table components you referenced. Just ensure all call sites for thisDataTableare updated to pass an explicitcanAddItemvalue so you don’t get a runtime crash from missing props.Also applies to: 52-56
fdm-app/app/components/blocks/fertilizer-applications/list.tsx (1)
63-66: LGTM! Fail-open permission check implemented correctly.The per-application editable flag correctly defaults to
truewhen permission data is absent, aligning with the intentional fail-open UI strategy documented in learnings.Based on learnings, this fail-open behavior is by design for the UI layer, with backend enforcement providing actual security.
fdm-app/app/components/blocks/fertilizer-applications/card.tsx (2)
116-127: LGTM! Dialog permission-gating logic is sound.The logic correctly prevents auto-opening the dialog for editing when the user lacks modification permission, while still allowing creation when
canCreateFertilizerApplicationis true. The fail-open defaults align with the documented UI strategy.Based on learnings, the UI implements fail-open permission checks with backend enforcement providing actual security.
28-29: LGTM! Permission props correctly threaded through component hierarchy.The new props
canCreateFertilizerApplicationandcanModifyFertilizerApplicationare properly defined with fail-open defaults and correctly passed down to theFertilizerApplicationsListcomponent to enable granular permission control.Based on learnings, this implements the intentional fail-open UI strategy where permission checks default to allowing actions when data is unavailable.
Also applies to: 40-41, 207-209
fdm-app/app/components/blocks/cultivation/card-details.tsx (7)
25-25:cnimport usage is appropriateImporting
cnhere is justified by its use in the submit button container; no issues.
31-39:editableprop shape and default are soundAdding
editablewith a default oftruekeeps existing call sites working while allowing permission-based read-only usage. The explicit optional type matches the defaulted prop.
78-97: Delete button now correctly respects read-only stateConditioning the delete button on both
!isCreateWizardandeditablecleanly prevents delete in read-only contexts while preserving behavior in the create wizard.
102-107: Fieldset-level disable foreditableis consistent with form semanticsExtending the
fieldset’sdisabledcondition with!editableensures all child inputs and the submit button become read-only together when the card is not editable, which aligns well with the permission-driven UI behavior.
111-124: DatePicker disabled gating matcheseditablebehaviorPassing
disabled={!editable}to both DatePickers is consistent with the card’s read-only mode and complements the fieldset-level disabling; no correctness issues here.
131-146: Checkbox disable condition correctly includeseditableThe checkbox now disables when
!editablein addition to submitting states. This avoids partial editability and matches the overall read-only semantics of the card.
163-171: Select disable behavior correctly combines permissions and data availabilityDisabling the Select when
!editableor when there are no options prevents inconsistent UX (e.g., allowing selection in read-only mode or when no varieties exist). The combined condition looks correct.fdm-app/app/components/blocks/soil/list.tsx (1)
12-12: LGTM! Clean prop definition.The optional
canModifySoilAnalysisprop is well-typed and provides a clean API for per-analysis permission control.Also applies to: 19-19
fdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.soil._index.tsx (5)
2-2: LGTM! Appropriate imports added.The
checkPermissionandcnimports support the new permission-gating functionality.Also applies to: 28-28
130-137: LGTM! Loader return structure is correct.The permission data is properly awaited and included in the loader response. The
awaiton line 132 correctly resolves the promise before returning.
169-179: LGTM! Permission gating implemented correctly.The
invisibleclass maintains layout consistency in the tab header while hiding the button when the user lacks write permission. This is preferable tohiddenwhich would cause layout shifts.
219-225: LGTM! Permission data correctly passed to component.The
soilAnalysisWritePermissionsare properly passed toSoilAnalysesListascanModifySoilAnalysis. However, this depends on fixing the logic error inlist.tsx(see previous comment about inverted permission check).
100-127: <function_calls>
#!/bin/bashRead the full error.ts file
cat -n fdm-core/src/error.ts
</function_calls>#!/bin/bash # Read the soil._index.tsx file to see full context cat -n fdm-app/app/routes/farm.\$b_id_farm.\$calendar.field.\$b_id.soil._index.tsx </parameter> </invoke> </function_calls> </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
2 participants
(Not done yet!)
Enhancements
hasPermissionfunction.Related to #321
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.