Improve UX and Functionality for Advisor and Researcher Roles#333
Merged
Conversation
🦋 Changeset detectedLatest commit: 4aa2deb 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 |
Contributor
|
Warning Rate limit exceeded@BoraIneviNMI has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughFetchers in farm routes now retrieve farm data and include it in loader responses; SidebarFarm accepts a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Loader as "farm.tsx / farm.$b_id_farm.tsx Loader"
participant Auth as "fdm.server"
participant DB as "getFarm()"
participant Sidebar as "SidebarFarm"
participant UI
User->>Loader: Request /farm or /farm/:b_id_farm
Loader->>Auth: validate session
Auth-->>Loader: session (principal_id)
Loader->>DB: getFarm(fdm, principal_id, b_id_farm)
alt farm found
DB-->>Loader: farm object (includes roles)
Loader->>Sidebar: pass farm
Sidebar->>Sidebar: getSuperiorRole(farm.roles)
Sidebar->>UI: render farm name + role Badge
else unauthorized / 401
Loader->>User: redirect to /signin?redirectTo=...
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #333 +/- ##
============================================
Coverage 87.59% 87.59%
============================================
Files 79 79
Lines 3951 3951
Branches 1141 1141
============================================
Hits 3461 3461
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:
|
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
fdm-core/src/authorization.ts (3)
1-1: Membership-aware permission check is implemented correctly; consider sharing the predicateThe new
leftJoinonauthNSchema.memberplus theor(inArray(authZSchema.role.principal_id, principal_ids), inArray(authNSchema.member.userId, principal_ids))condition gives you the desired behavior: users inherit permissions from organizations they belong to while still supporting direct principal-based roles. To avoid repeating this somewhat complex predicate across multiple queries, you could extract it into a small helper (e.g.,principalOrMemberFilter({ principalIds })) and reuse it incheckPermission,getRolesOfPrincipalForResource, andlistResources.Also applies to: 13-13, 179-210
271-288: Update JSDoc for getRolesOfPrincipalForResource to reflect inherited roles supportThe implementation now joins
authNSchema.memberand applies the same OR condition ascheckPermission, so this function does effectively return roles inherited via organization membership. The JSDoc still says “CAUTION: This function does not return inherited roles yet.” which is now misleading. Please update the comment to describe the new behavior (e.g., that it returns the union of direct and membership-derived roles for the given principal ID(s)).Also applies to: 294-320
538-552: listResources correctly includes org-derived access; consider deduping resource IDsThe added
leftJoinand OR condition inlistResourcescorrectly ensure that users see resources granted either directly to their principal_id or via membership in an organization. One small improvement: because the same resource can now be reachable through multiple principals/roles, the returnedresource_idarray can contain duplicates. If you want a clean set of accessible IDs, you could wrap the final map with aSet:- return result.map( - (resource: { resource_id: string }) => resource.resource_id, - ) + return [ + ...new Set( + result.map( + (row: { resource_id: string }) => row.resource_id, + ), + ), + ]This keeps the API easier to consume without changing semantics.
Also applies to: 568-592
fdm-app/app/routes/farm.$b_id_farm.tsx (1)
1-1: Loader farm retrieval aligns with auth model; JSDoc could mention the newfarmfieldFetching the farm via
getFarm(fdm, session.principal_id, b_id_farm)right aftergetSessionis consistent with how other core calls use the principal identifier for authorization, and extending the loader return to includefarmwill make it straightforward for downstream components to show farm name and roles. The only follow-up I’d suggest is updating the loader’s documentation block so it also states thatfarmis returned alongsidefarmIdandsession.Also applies to: 6-6, 30-51
fdm-app/app/components/blocks/sidebar/farm.tsx (1)
1-1: Role badge behavior is good; avoid mutatingfarm.rolesin getSuperiorRoleThe new
farmprop andfarmRolebadge nicely surface the user’s strongest role on the current farm, with an explicit priority of owner → advisor → researcher, which matches the permissions table.Implementation-wise, two small refinements would make this more robust:
getSuperiorRolemutates its input viaallRoles.sort(...), which in turn mutatesfarm.roles. That’s harmless today but can be surprising if the same array is reused elsewhere. Safer variant:function getSuperiorRole(allRoles: ("owner" | "advisor" | "researcher")[]) { if (allRoles.length === 0) return null const ordering: ("owner" | "advisor" | "researcher")[] = [ "owner", "advisor", "researcher", ] const [best] = [...allRoles].sort( (a, b) => ordering.indexOf(a) - ordering.indexOf(b), ) return best }Instead of
const ordering: unknown[], typingorderingto the same role union (or reusing a sharedRoletype from core, if exported) will give you better type checking and self-documentation.Functionally this all looks correct; these are just polish suggestions.
Also applies to: 34-47, 57-62, 69-71, 119-129
fdm-app/app/routes/farm.tsx (1)
1-1: Loader now preloads farm context; consider updating the docstringUsing
params.b_id_farm ? await getFarm(fdm, session.principal_id, params.b_id_farm) : undefinedin the root farm loader cleanly wires farm context into the layout when a farm ID is part of the URL, while still short-circuiting for/farmwithout a specific farm. The explicit 401 handling with a redirect to/signin?redirectTo=…also fits well with the existing auth flow. I’d just update the loader’s comment to mention that it may also return afarmobject in addition to the user-related fields.Also applies to: 23-23, 47-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
fdm-app/app/components/blocks/sidebar/farm.tsx(4 hunks)fdm-app/app/routes/farm.$b_id_farm.tsx(2 hunks)fdm-app/app/routes/farm.tsx(5 hunks)fdm-core/src/authorization.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (30)
📓 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.
📚 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/sidebar/farm.tsxfdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.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/components/blocks/sidebar/farm.tsxfdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.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/components/blocks/sidebar/farm.tsxfdm-app/app/routes/farm.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/components/blocks/sidebar/farm.tsxfdm-app/app/routes/farm.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/components/blocks/sidebar/farm.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/components/blocks/sidebar/farm.tsxfdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.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/components/blocks/sidebar/farm.tsxfdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.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.tsxfdm-app/app/routes/farm.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.tsxfdm-app/app/routes/farm.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.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.tsxfdm-app/app/routes/farm.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.tsxfdm-app/app/routes/farm.tsx
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.tsxfdm-core/src/authorization.ts
📚 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.tsxfdm-app/app/routes/farm.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.tsxfdm-app/app/routes/farm.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.tsxfdm-app/app/routes/farm.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/routes/farm.$b_id_farm.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks, as evidenced by the changelog store using createJSONStorage(() => localStorage) directly.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.tsx
📚 Learning: 2024-11-25T12:42:32.783Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.tsx
📚 Learning: 2025-02-24T10:49:54.523Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 84
File: fdm-app/app/root.tsx:89-145
Timestamp: 2025-02-24T10:49:54.523Z
Learning: In the ErrorBoundary component of fdm-app/app/root.tsx, all client errors (400, 401, 403, 404) are intentionally displayed with a 404 status code for security purposes.
Applied to files:
fdm-app/app/routes/farm.$b_id_farm.tsxfdm-app/app/routes/farm.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/authorization.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/authorization.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-core/src/authorization.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts currently returns only firstname, surname, image, and role, but needs to include id and email fields to support downstream operations like role updates and user removal. This will be fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.ts
📚 Learning: 2025-04-18T14:18:44.350Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:69-76
Timestamp: 2025-04-18T14:18:44.350Z
Learning: The authentication schema (fdm-authn) is based on better-auth's schema structure, which does not include `updatedAt` fields for tables like `organization`.
Applied to files:
fdm-core/src/authorization.ts
📚 Learning: 2025-01-24T11:56:40.906Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/rollup.config.js:17-30
Timestamp: 2025-01-24T11:56:40.906Z
Learning: For the FDM project, keep error handling simple when basic error notification (console.error) is sufficient, avoiding unnecessary complexity in error handling mechanisms.
Applied to files:
fdm-app/app/routes/farm.tsx
📚 Learning: 2025-08-11T11:57:42.225Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields.$centroid.tsx:0-0
Timestamp: 2025-08-11T11:57:42.225Z
Learning: In Remix loaders, the `getSession(request)` call should be retained even when the returned session value isn't used elsewhere in the function, as it serves as an authentication guard that validates the user has a valid session and likely throws an error or redirects if the session is invalid. This is a common security pattern to protect routes.
Applied to files:
fdm-app/app/routes/farm.tsx
🧬 Code graph analysis (1)
fdm-app/app/routes/farm.$b_id_farm.tsx (1)
fdm-core/src/db/schema-authn.ts (1)
session(29-41)
🔇 Additional comments (1)
fdm-app/app/routes/farm.tsx (1)
121-128: Passing loaderData.farm into SidebarFarm completes the role-visibility flowWiring
farm={loaderData.farm}into<SidebarFarm />ensures the sidebar can show the current farm name and the user’s most powerful role based on the loader’s farm data. This ties together the new core authorization behavior and the UI-side role badge cleanly, without introducing extra data-fetching in the component tree.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fdm-core/src/authorization.test.ts (1)
709-752: Organization-derived role tests also reuseorganization_member_id.These tests may work currently but are susceptible to the same test isolation issues if earlier tests leave residual state. For robustness, consider creating fresh users within each test, similar to the fix suggested for
listResources.The test at lines 728-752 nicely verifies that both direct roles ("researcher") and organization-derived roles ("owner") are returned together.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-core/src/authorization.test.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
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.
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: 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.
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.
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: 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.
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts currently returns only firstname, surname, image, and role, but needs to include id and email fields to support downstream operations like role updates and user removal. This will be fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.test.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 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/authorization.test.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/authorization.test.ts
📚 Learning: 2025-04-18T14:18:44.350Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:69-76
Timestamp: 2025-04-18T14:18:44.350Z
Learning: The authentication schema (fdm-authn) is based on better-auth's schema structure, which does not include `updatedAt` fields for tables like `organization`.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-01-29T14:33:41.822Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 65
File: fdm-core/src/global-setup.ts:7-9
Timestamp: 2025-01-29T14:33:41.822Z
Learning: In the fdm project, tests should be organized per file to maintain separation of concerns, rather than consolidating them into a centralized location.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T15:23:48.352Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:58:48.603Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-04T11:09:08.169Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-core/src/cultivation.ts:246-246
Timestamp: 2025-03-04T11:09:08.169Z
Learning: In the FDM codebase, the `fdm` parameter should be documented as "The FDM instance providing the connection to the database. The instance can be created with {link createFdmServer}." in JSDoc comments.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:38:52.315Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:38:52.315Z
Learning: When writing unit tests for the fdm project, avoid using vi.mock() as it has caused implementation problems in the past. Use direct mock functions with vi.fn() instead.
Applied to files:
fdm-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
🪛 GitHub Actions: Tests
fdm-core/src/authorization.test.ts
[error] 574-574: AssertionError: expected 4 to be 2 // Object.is equality
🪛 GitHub Check: core (24)
fdm-core/src/authorization.test.ts
[failure] 574-574: src/authorization.test.ts > Authorization Functions > listResources > should list resources that the user's organization has access to
AssertionError: expected 4 to be 2 // Object.is equality
- Expected
- Received
- 2
- 4
❯ src/authorization.test.ts:574:48
🔇 Additional comments (3)
fdm-core/src/authorization.test.ts (3)
21-25: LGTM!The organization-related imports are correctly added to support the new organization-based permission tests.
99-106: Good use of unique slugs for test organizations.Using
test-${createId()}ensures each test gets a unique organization. However, the accumulated memberships of the sharedorganization_member_iduser still cause cross-test contamination.
136-154: Test invitesorganization_memberbut checksprincipal_idaccess.This test verifies that the organization creator (
principal_id) can access the farm through the organization. The test logic is valid sinceprincipal_idcreated the organization. However, note that the invitation/acceptance at lines 138-145 adds a membership fororganization_member_idthat persists to subsequent tests.
Contributor
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 (1)
fdm-core/src/authorization.test.ts(7 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 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: 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.
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.
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.
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.
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.
📚 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/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
fdm-core/src/authorization.test.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/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts currently returns only firstname, surname, image, and role, but needs to include id and email fields to support downstream operations like role updates and user removal. This will be fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:18:44.350Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:69-76
Timestamp: 2025-04-18T14:18:44.350Z
Learning: The authentication schema (fdm-authn) is based on better-auth's schema structure, which does not include `updatedAt` fields for tables like `organization`.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T15:23:48.352Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Applied to files:
fdm-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:58:48.603Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-04T11:09:08.169Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-core/src/cultivation.ts:246-246
Timestamp: 2025-03-04T11:09:08.169Z
Learning: In the FDM codebase, the `fdm` parameter should be documented as "The FDM instance providing the connection to the database. The instance can be created with {link createFdmServer}." in JSDoc comments.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:38:52.315Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:38:52.315Z
Learning: When writing unit tests for the fdm project, avoid using vi.mock() as it has caused implementation problems in the past. Use direct mock functions with vi.fn() instead.
Applied to files:
fdm-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
🔇 Additional comments (3)
fdm-core/src/authorization.test.ts (3)
88-108: LGTM! Test isolation issue resolved.Moving organization and organization member creation to
beforeEachensures each test gets fresh entities with unique IDs, addressing the test isolation concerns raised in previous reviews where shared membership accumulated across tests.
558-580: LGTM! Organization-based resource listing works correctly.With per-test organization and member creation in
beforeEach, this test properly verifies that organization members can list all farms their organization has access to without interference from other tests.
712-755: LGTM! Comprehensive role derivation tests.These tests properly verify that:
- Members inherit roles from their organization's farm access
- Members can have both direct roles and organization-derived roles
- Role priority is correctly maintained (owner before researcher)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
fdm-core/src/authorization.test.ts (1)
174-196: Test description does not match implementation.The test description states "should not grant permissions higher than the organization permissions," implying it verifies that organization members cannot exceed the organization's role. However, line 190 checks
organization_id(the organization entity) rather thanorganization_member_id(the invited member).To match the description, check the member's permissions:
await expect( checkPermission( fdm, "farm", "write", farm_id, - organization_id, + organization_member_id, "test", ), ).rejects.toThrow(
🧹 Nitpick comments (1)
.changeset/hungry-shirts-scream.md (1)
5-5: Consider rewording for clarity.The phrasing is slightly awkward. Consider:
-Fixes that members of an organization that has a role on a farm inherit that role +Fix: Organization members now correctly inherit the organization's role on a farm
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/hungry-shirts-scream.md(1 hunks).changeset/new-places-shop.md(1 hunks)fdm-core/src/authorization.test.ts(7 hunks)fdm-core/src/authorization.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 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: 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.
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: 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.
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.
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-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:
.changeset/new-places-shop.md
📚 Learning: 2024-11-25T12:42:32.783Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Applied to files:
.changeset/new-places-shop.md
📚 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:
.changeset/new-places-shop.md
📚 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:
.changeset/new-places-shop.md
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
.changeset/hungry-shirts-scream.mdfdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
.changeset/hungry-shirts-scream.mdfdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts currently returns only firstname, surname, image, and role, but needs to include id and email fields to support downstream operations like role updates and user removal. This will be fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
.changeset/hungry-shirts-scream.mdfdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
.changeset/hungry-shirts-scream.mdfdm-core/src/authorization.tsfdm-core/src/authorization.test.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 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/authorization.tsfdm-core/src/authorization.test.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/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:18:44.350Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:69-76
Timestamp: 2025-04-18T14:18:44.350Z
Learning: The authentication schema (fdm-authn) is based on better-auth's schema structure, which does not include `updatedAt` fields for tables like `organization`.
Applied to files:
fdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 Learning: 2025-03-06T15:23:48.352Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:58:48.603Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-04T11:09:08.169Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-core/src/cultivation.ts:246-246
Timestamp: 2025-03-04T11:09:08.169Z
Learning: In the FDM codebase, the `fdm` parameter should be documented as "The FDM instance providing the connection to the database. The instance can be created with {link createFdmServer}." in JSDoc comments.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:38:52.315Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:38:52.315Z
Learning: When writing unit tests for the fdm project, avoid using vi.mock() as it has caused implementation problems in the past. Use direct mock functions with vi.fn() instead.
Applied to files:
fdm-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
🔇 Additional comments (6)
fdm-core/src/authorization.ts (3)
186-210: Well-implemented organization-based permission inheritance.The left join with the member table combined with the
or()condition correctly enables users to inherit permissions from their organizations. The existing role and resource checks remain intact, ensuring proper authorization boundaries.
294-319: LGTM!The OR-based matching correctly retrieves roles from both direct grants and organization memberships, and the existing
Setdeduplication at line 319 prevents duplicate role entries in the result.
563-588: Good use ofselectDistinctto handle duplicate resources.The switch to
selectDistinctcorrectly prevents duplicate resource IDs when a user has access through multiple paths (e.g., both direct role and organization membership).fdm-core/src/authorization.test.ts (3)
88-109: Test isolation properly addressed.Creating the organization and organization member in
beforeEachwith unique identifiers viacreateId()ensures each test starts with a clean slate, avoiding the membership accumulation issue flagged in previous reviews.
558-606: Good test coverage for organization-based resource listing.These tests properly validate:
- Resources are listed when the user's organization has access
- Duplicates are prevented when a user has both direct and organization-based access to the same resource
738-781: Comprehensive tests for organization-derived roles.These tests properly validate that:
- Roles granted to an organization are correctly derived for its members
- Both direct and organization-derived roles are returned together
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
fdm-core/src/authorization.ts (1)
186-211: Left join with OR filter may include unintended NULL matches.When
authNSchema.member.userIdis NULL (no matching organization membership), theinArray(authNSchema.member.userId, principal_ids)condition could behave unexpectedly depending on the database. While PostgreSQL typically treatsNULL IN (...)as NULL (falsy), this relies on implicit behavior.Consider adding an explicit NULL check for clarity and safety:
or( inArray( authZSchema.role.principal_id, principal_ids, ), - inArray( - authNSchema.member.userId, - principal_ids, - ), + and( + isNotNull(authNSchema.member.userId), + inArray( + authNSchema.member.userId, + principal_ids, + ), + ), ),This would require importing
isNotNullfrom drizzle-orm.fdm-app/app/components/blocks/sidebar/farm.tsx (1)
39-48:getSuperiorRolehelper can be simplified and better typed.The
orderingarray is typed asunknown[], which loses type safety. Also, the function can be simplified since the ordering array already defines the priority:- function getSuperiorRole(allRoles: ("owner" | "advisor" | "researcher")[]) { - if (allRoles.length > 0) { - const ordering: unknown[] = ["owner", "advisor", "researcher"] - const sorted = [...allRoles].sort( - (a, b) => ordering.indexOf(a) - ordering.indexOf(b), - ) - return sorted[0] - } - return null - } + function getSuperiorRole(allRoles: ("owner" | "advisor" | "researcher")[]) { + const ordering = ["owner", "advisor", "researcher"] as const + for (const role of ordering) { + if (allRoles.includes(role)) return role + } + return null + }This avoids sorting and is more readable—it returns the first (highest priority) role found.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
fdm-app/app/components/blocks/sidebar/farm.tsx(4 hunks)fdm-app/app/routes/farm.tsx(5 hunks)fdm-core/src/authorization.test.ts(7 hunks)fdm-core/src/authorization.ts(6 hunks)
🧰 Additional context used
🧠 Learnings (36)
📓 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: 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.
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: 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.
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.
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.
📚 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/authorization.tsfdm-core/src/authorization.test.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 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/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts will be updated in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5 to include id and email fields, which are necessary for subsequent role updates and user removal operations.
Applied to files:
fdm-core/src/authorization.tsfdm-core/src/authorization.test.tsfdm-app/app/components/blocks/sidebar/farm.tsx
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needs to include user ID and email fields to support role updates and user removal operations, which will be fixed in a future commit (b17fac16c9e5a0de56d0346e712b2ce966d305d5).
Applied to files:
fdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts needed to include id and email fields for users, which was fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:51:48.033Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/organization.ts:300-339
Timestamp: 2025-04-18T14:51:48.033Z
Learning: The `getUsersInOrganization` function in fdm-core/src/organization.ts currently returns only firstname, surname, image, and role, but needs to include id and email fields to support downstream operations like role updates and user removal. This will be fixed in commit b17fac16c9e5a0de56d0346e712b2ce966d305d5.
Applied to files:
fdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 Learning: 2025-04-18T14:18:44.350Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-core/src/db/schema-authn.ts:69-76
Timestamp: 2025-04-18T14:18:44.350Z
Learning: The authentication schema (fdm-authn) is based on better-auth's schema structure, which does not include `updatedAt` fields for tables like `organization`.
Applied to files:
fdm-core/src/authorization.tsfdm-core/src/authorization.test.ts
📚 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-core/src/authorization.tsfdm-core/src/authorization.test.tsfdm-app/app/routes/farm.tsx
📚 Learning: 2024-11-27T12:15:36.425Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 9
File: fdm-data/src/cultivations/index.test.ts:57-59
Timestamp: 2024-11-27T12:15:36.425Z
Learning: In `fdm-data/src/cultivations/index.test.ts`, the `fdm` object created by `drizzle` does not have an `.end()` method. Cleanup code should not attempt to call `fdm.end();`.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T15:23:48.352Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/catalogues.ts:134-170
Timestamp: 2025-03-06T15:23:48.352Z
Learning: When writing tests for fdm-core, avoid using Vitest's `vi` mocking utilities and prefer manual JavaScript mocks.
Applied to files:
fdm-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:58:48.603Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:58:48.603Z
Learning: When writing unit tests for the fdm project, avoid using Vitest's mocking utilities (vi) as it has caused problems in the past not related to the actual code. Instead, use simple object literals with methods that throw errors to test error handling.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-04T11:09:08.169Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 88
File: fdm-core/src/cultivation.ts:246-246
Timestamp: 2025-03-04T11:09:08.169Z
Learning: In the FDM codebase, the `fdm` parameter should be documented as "The FDM instance providing the connection to the database. The instance can be created with {link createFdmServer}." in JSDoc comments.
Applied to files:
fdm-core/src/authorization.test.ts
📚 Learning: 2025-03-06T14:38:52.315Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 95
File: fdm-core/src/cultivation.ts:67-73
Timestamp: 2025-03-06T14:38:52.315Z
Learning: When writing unit tests for the fdm project, avoid using vi.mock() as it has caused implementation problems in the past. Use direct mock functions with vi.fn() instead.
Applied to files:
fdm-core/src/authorization.test.ts
📚 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-core/src/authorization.test.ts
📚 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.tsxfdm-app/app/components/blocks/sidebar/farm.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.tsxfdm-app/app/components/blocks/sidebar/farm.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.tsxfdm-app/app/components/blocks/sidebar/farm.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.tsxfdm-app/app/components/blocks/sidebar/farm.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.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.tsxfdm-app/app/components/blocks/sidebar/farm.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.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.tsxfdm-app/app/components/blocks/sidebar/farm.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.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.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.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.tsxfdm-app/app/components/blocks/sidebar/farm.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks, as evidenced by the changelog store using createJSONStorage(() => localStorage) directly.
Applied to files:
fdm-app/app/routes/farm.tsx
📚 Learning: 2025-09-25T15:10:59.708Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 272
File: fdm-app/app/store/field-fertilizer-form.tsx:45-47
Timestamp: 2025-09-25T15:10:59.708Z
Learning: In the FDM application, Zustand stores with persist middleware using sessionStorage/localStorage don't require SSR hardening guards. The existing store patterns in fdm-app work without typeof window checks or memory storage fallbacks.
Applied to files:
fdm-app/app/routes/farm.tsx
📚 Learning: 2025-02-24T10:49:54.523Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 84
File: fdm-app/app/root.tsx:89-145
Timestamp: 2025-02-24T10:49:54.523Z
Learning: In the ErrorBoundary component of fdm-app/app/root.tsx, all client errors (400, 401, 403, 404) are intentionally displayed with a 404 status code for security purposes.
Applied to files:
fdm-app/app/routes/farm.tsx
📚 Learning: 2025-01-24T11:56:40.906Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 49
File: fdm-core/rollup.config.js:17-30
Timestamp: 2025-01-24T11:56:40.906Z
Learning: For the FDM project, keep error handling simple when basic error notification (console.error) is sufficient, avoiding unnecessary complexity in error handling mechanisms.
Applied to files:
fdm-app/app/routes/farm.tsx
📚 Learning: 2025-08-11T11:57:42.225Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.atlas.fields.$centroid.tsx:0-0
Timestamp: 2025-08-11T11:57:42.225Z
Learning: In Remix loaders, the `getSession(request)` call should be retained even when the returned session value isn't used elsewhere in the function, as it serves as an authentication guard that validates the user has a valid session and likely throws an error or redirects if the session is invalid. This is a common security pattern to protect routes.
Applied to files:
fdm-app/app/routes/farm.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/components/blocks/sidebar/farm.tsx
📚 Learning: 2024-11-25T12:42:32.783Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Applied to files:
fdm-app/app/components/blocks/sidebar/farm.tsx
🧬 Code graph analysis (1)
fdm-app/app/routes/farm.tsx (2)
fdm-app/app/routes/farm.$b_id_farm.tsx (1)
loader(30-55)fdm-app/app/components/blocks/sidebar/farm.tsx (1)
SidebarFarm(34-312)
🔇 Additional comments (10)
fdm-core/src/authorization.ts (2)
261-270: Documentation update accurately reflects the new behavior.The updated JSDoc correctly documents that user principals can now derive roles through their organization memberships, and the caution about inherited roles from related resources is preserved.
568-592: Good use ofselectDistinctto prevent duplicate resource IDs.The switch from
selecttoselectDistinctcorrectly handles the case where a user has both direct access and organization-derived access to the same resource, preventing duplicate entries in the result.fdm-core/src/authorization.test.ts (4)
88-109: Test isolation improved by creating organization and member inbeforeEach.Moving the organization and organization member creation from
beforeAlltobeforeEachcorrectly addresses the test isolation issue from past reviews. Each test now starts with a fresh organization and member, preventing membership accumulation across tests.
139-157: Test correctly verifies organization member's inherited access.The test now properly checks that
organization_member_id(the invitee) gains write access to the farm through their organization membership, which aligns with the test description.
174-196: Test correctly validates that organization role limits member permissions.The test grants the organization a "researcher" role (read-only) and verifies that an organization member (even with "owner" role within the organization) cannot perform "write" actions on the farm. This correctly tests the permission ceiling behavior.
757-781: Test verifies combined direct and organization-derived roles.This test correctly validates that when a user has both a direct role ("researcher") and an organization-derived role ("owner"), both roles are returned by
getRolesOfPrincipalForResource. The expected result["owner", "researcher"]confirms the function returns all applicable roles without duplication.fdm-app/app/components/blocks/sidebar/farm.tsx (1)
121-135: Role badge display meets the PR objective of showing user's role.The badge correctly displays the computed superior role with appropriate Dutch translations. The
key={farmRole}prop is unnecessary on a non-list element but harmless. The "Onbekend" fallback is defensive coding sincefarmRolecan only be one of the three known roles ornull(in which case the badge isn't rendered).fdm-app/app/routes/farm.tsx (3)
69-79: Error handling correctly redirects 401 errors to signin.The catch block properly handles authentication failures by redirecting to
/signinwith the current path preserved, allowing users to return after re-authenticating. Other errors are re-thrown for the ErrorBoundary to handle.
127-127: Farm data correctly passed to SidebarFarm component.The
farmprop is properly passed toSidebarFarm, which will beundefinedwhen no farm is selected (e.g., on/farmindex) and populated when viewing a specific farm. This aligns with the component's prop type.
58-60: I'll verify the duplicategetFarmcalls concern by examining the route structure and data loading patterns.
<function_calls>
shell
#!/bin/bashFind all route files that might call getFarm
rg -n "getFarm(" --type=ts fdm-app/app/routes/ -A 2 -B 2
</function_calls>Now let me examine the child route file to understand the data flow:
<function_calls>
shell
#!/bin/bashFind the farm.$b_id_farm route file
fd "farm.$b_id_farm" fdm-app/app/routes/ --type f
</function_calls>Let me also check the structure of the farm.tsx file to see its complete context:
<function_calls>
shell
#!/bin/bashGet the full farm.tsx file to understand context
wc -l fdm-app/app/routes/farm.tsx
</function_calls>
This was referenced Jan 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There is a suspicion that users sometimes don't get their permissions derived from their organization being partnered with a farm. This pull request makes the user's most significant role clearer to them, and resolves any bugs found in the authorization system.
Enhancements
Closes #321
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.