Hey @mstenta but also anyone who’s used these modules maybe @miriam also. I’ve have claud 3.8 evaluate the use of the intake flow process (intake form → intake log → continue/abandon + assign → create farm → create plan) and it did a deep dive. It identified some questions I wanted to post here and get feedback on. The rest of this is from AI directly FYI, so apologies for the language if it’s verbose or uses words like a developer from SF
What we’re reusing (your pattern, closely): staged intake → pending log → status-gated review-by-injection (the triage form on the log’s own view page) → assign owner → spin off a plan → notify. We’re copying the details too — the abandon revision message (+reason), owner stamped on both the intake log and the plan, the three mail keys, the form-protection trait, and an immutability constraint on the plan↔farm↔intake refs (your RcdImmutable — we read it as tenancy-escape protection and want to generalize it).
Where we deviate, and why:
Multitenant access coupling. RCD is single-tenant — you scoped it that way on purpose (per the README) — so owner is a marker and access is one broad rcd_staff role. We’re on farm_multitenant, where access = group membership, so assigning the owner also adds them as an org member.
Config-driven spawn sequence. You start one plan type (rcd_rcp). We spin off a configured sequence (Fields→Plantings→Activities) via a typed parent→children plan reference (modeled on your practice_implementation_plan) + a config key — so we can reuse it for conservation/benchmarking/research later.
Substrate. We build on quickform_plans + farm_intake + multitenant (your README says farm_rcd isn’t meant to be installed as a dependency, so we reuse the pattern, not the module).
The finding — a real multitenant access gap (your call whether to fix upstream): farm_multitenant’s group-access handler + query-tag filter cover asset and log only. plan, plan_record, and organization have no membership-based access control — they’re governed solely by site-role permissions. Verified empirically on a 2-org test instance: a TAP who is a member of only Org B, holding view any plan, can view Org A’s plans and plan_records (which hold submission data), while correctly being denied Org A’s land asset. So spun-off plans + their submissions leak across orgs to any broad-role user.
We’d close it by adding a plan/plan_record access handler + hook_query_TAG_alter keyed on plan.farm → membership, mirroring your AccessControlHandlerLog. Is plan-access-by-role intentional, or would you want this upstreamed into farm_multitenant? (This is the big one.)
Two smaller questions:
A farm-less pending intake log is forbidden to non-admins under the multitenant log handler (it denies logs with no asset ref). On RCD that’s fine (single-tenant; view any rcd_intake works). On multitenant, did you intend intake to stay a broad-role-visible log — and would you exempt the intake bundle from the handler, or model triage differently?
Would you value a generic intake/spawn pattern (config spawn sequence + typed parent→children plan ref) as a reusable thing, vs. keeping it per-project?
Full design doc + the deviation rationale (see attached design doc).
@gbathree Can you summarize what you’re trying to achieve? Who is creating records of each type and who should/shouldn’t have access to them?
Who should have access to plan, plan_record, and organization entities in your use-case? Is there a reason it needs to be related to Organization membership?
Yes - the goal here is that we can allow for a more long-term multi-tenant structure. This isn’t a specific need now that I know of (for Serena or Nano), but it’s been expressed enough in the past that I’d like to account for it sooner than later.
A simple case example: a TAP is assigned by the Program Manager to support 30 of 300 farms in a multi-farm farmOS instance. They should be able to see and modify all data associated with those 30 farms, but not any data associated with the other 300. This data should include logs and assets (as it does today) but also plans and plan records, and potentially other (new) entity types that shouldn’t be shared (? can’t think of any, but maybe in the future… not a concern for now?). I know we have a gap of logs which are not associated with an asset, but at this point it’s not a primary concern.
That said - some things are ok to share, like taxonomy terms, across all farms. Sharing taxonomy terms has some downside to the producer (sharing a special sauce fertilizer product or something), but it has a lot of upside to the data maintainer in supporting comparability and reducing duplicates.
Miriam’s code already relates access to Org membership, so this is just extending that to plans and plan records. This works nicely because we’re trying to provide a clean way, via assignment to an organization (which is consistent with the desired user journey), or a person being able to access everything associated with that organization.
Can you just add a farm reference field to your custom plan types, and use that to grant access to them, in the same way that the farm reference on asset entities does?
Or maybe that’s what you’re proposing.
The challenge is that farm is not a standard field on plan entities, which is what the farm_multitenant module depends on.
The farm_rcd module adds its own farm reference field to the plan type it provides. It uses this for filtering plans by farm, but not for any multitenant access control.
Two ideas:
farmOS core adds a farm reference field to all plan entities, like it does to asset entities. Then the farm_multitenant module can depend on its existence and provide access logic for it.
The farm_multitenant could provide a hook that modules could implement to say “use this relationship that I defined”. Then it would be the plan-type-providing module’s responsibility to add the relationship to the farm, and tell farm_multitenant about it.
In both cases, access to plan_record entities should follow the same pattern as log entities - access to them should depend on the plan they are related to.
Option 1 raises some questions… can we assume that a farm reference field makes sense on all plan types? And of course its a core data model change, so it would require wider community consideration.
Option 2 is very flexible, but I’m not sure what the complexity of implementing that in farm_multitenant would be.
What do you think? Would simply adding a farm reference field and tying that into the Multitenant access logic meet your needs?
I think, especially since we’ve both experienced the same issue, it’s worth considering a push to core. I’m happy to include this in my work and make a PR if you’re game to review.
In both cases, access to plan_record entities should follow the same pattern as log entities - access to them should depend on the plan they are related to.
100% agreed, makes sense.
Option 1 raises some questions… can we assume that a farm reference field makes sense on all plan types? And of course its a core data model change, so it would require wider community consideration.
IMO Plans should be first class entities in FarmOS (like assets and logs), even though they are underused right now. From many farmers’ perspectives, the plans are where most of the action is!
We’re talking about two specific downstream modules that need a farm reference field on plan entities… but they need it for different reasons.
In the RCD context, the plan needs to be assigned to a farm so that the plan can pull high-level information about that farm and plug it into the Resource Conservation Plan template.
In the Multitenant context, plans need to be associated with a farm in order to restrict access to them based on farm membership.
Core could support both by providing a common data model that they both leverage for their specific needs, but are there cases where adding a farm reference field to a plan would be undesirable?
None of the other modules that provide plan types (that I’m aware of) would be making use of the new field, but it would be available to them if they wanted to. And even without the Multitenant or RCD modules, it would allow plans to be filtered by farm, which is nice.
So generally speaking I’m leaning towards agreeing that it makes sense to add in core.
But… if we did add it to core we also need to consider whether core is also responsible for maintaining a separation between the “graphs” of farm records. It does that already with assets and logs to make sure that there isn’t any crossover of records between farms. That’s harder to do with plan and plan_record entities because we don’t know what references they will have. It might be possible to dynamically add constraints to them, but this starts to quickly move from a “trivial” task of adding a base field to something much more complicated (and requiring automated tests).
If you feel like tackling all of that, feel free! But don’t be surprised if it turns into a rabbit hole. That happened to me with the asset/log constraints.
Alternatively, if you just need something that works now for Multitenant, you could take a shortcut and implement what you need temporarily in that module. it could add a multitenant_farm reference field, and use that to control access to plan and plan_record entities. By namespacing it with multitenant_* you avoid any potential conflict if core adds a plan field later, and if that happens the Multitenant module can include an update hook to migrate to the core field and delete the custom one.
mmmm… You mean because plan records are such an empty slate they could create anything in ways that aren’t definable basically? I think… well… I feel like that is kind of on the developer to pay attention to that. If you are using a plan_record to create new assets / logs / other entities, it’s kind of on you to also ensure you are assigning those assets to the correct organization (which, if plan has an attached org, then it’s easy). In short - whoever takes it on it’s their rabbit hole
That same issue you already addressed with assets is way less because it’s not expected that you’re programatically creating other stuff when adding a log or asset… they mostly just stand alone.
I think if I did it now without the change to core, I would just add the farm relationship like you did in a custom plan (since all these are custom plans anyway).
But yeah, curious what others thing also, or if there are cases where this would be more annoying than it’s value.
@gbathree You can include this as a patch to your deployments if you want.
Notably, this does not add any constraint validation. That needs to be discussed in more detail, and may ultimately determine how quickly this gets merged (or not).
