Closed (fixed)
Project:
farmOS
Version:
2.x-dev
Component:
Miscellaneous
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
30 Nov 2020 at 16:46 UTC
Updated:
20 Jan 2021 at 19:14 UTC
Jump to comment: Most recent
Problem/Motivation
In 1.x there is a farm_crops (Crop/Varieties) taxonomy vocabulary. Each term has a farm_crop_family field that references a term from the farm_crop_families (Crop family) taxonomy vocabulary. Plant assets have a farm_crop field that can reference a term in the farm_crops taxonomy.
It's worth noting that farm_crop_families taxonomy had little to no integration in the farmOS UI. It was mostly behind the scenes and used as "metadata" on crop/variety terms. But having the ability to manage crop/varieties terms as a part of a crop family is quite useful. I use this feature in a custom farmOS module to display Plant assets filtered by crop family.
Proposed resolution
Using the same logic in 2.x would make for a straightforward migration. But is there anything we want to change?
- The taxonomy term hierarchy *could* be used to structure varieties within specific families. There is a nice convenience factor to this, but also becomes less structured. Incorporating a better UI to manage crop families seems like a better solution.
- Any name changes? I'm assuming we want to keep the
farm_prefix on the taxonomies, but maybe drop the prefix from the field names? EDIT: I see we're dropping thefarm_prefix from taxonomies too, and only keeping it in the core module names. That was my confusion!
Comments
Comment #2
paul121 commentedComment #3
paul121 commentedComment #4
paul121 commentedTo get this moving I went ahead and created the plant_type and crop_family vocabularies, along with the crop_family, transplant_days, maturity_days and companions fields on the plant_type term. Just wanted to get it into code, and follow up to change names after if needed. In my 2.x-farm_plant_type branch.
In chat we discussed changing the "farm_crops" machine name to "plant_type" to be consistent with the "animal_type" vocabulary for Species/breeds. This seems sensible so I went ahead with the change, but easy to change! This also changes the plant asset.crop field name to be asset.plant_type.
We also discussed changing "farm_crop_families" to "plant_family", but this doesn't make as much sense because the plant asset doesn't directly reference the crop family term. "plant_type_family" might be a more reasonable name. For now, I've just changed the existing "farm_crop_families" to "crop_family" per our standard naming convention. Again, easy to change.
I've created a migration for plant_type and crop_family terms, but need to create some demo data before I can test migrating the other plant_type specific fields. Also need feedback for the final vocabulary + field names to use!
Comment #5
paul121 commentedFinished testing migrations for the other plant_type specific fields (transplant days, maturity days, crop family, companions and images). Barring any further name changes as described above, this should be good to go.
Note that I defined all these plant_type specific fields using config rather than the bundle plugin system. I'm not sure if we want to (or even can) introduce the bundle plugin system to taxonomy terms? Seems like we will face similar challenges by not doing so, although modules adding base fields on taxonomy terms doesn't seem as critical as adding base fields to logs, assets and plans.
Comment #6
m.stentaThanks @paul121!
I see what you mean about "plant_family" - but that wouldn't bother me too much.
I think more broadly I'm still not completely satisfied with the two taxonomies - but this isn't something we'll be able to change drastically during the 2.x upgrade I don't think - so I'm in favor of keeping things as-is (but changing the names).
It makes me wonder if we may want a more general "Family" taxonomy in the future - which can represent other things beyond "plants". I could see the "Animal Type" terms referencing a "Family" in the same way - as well as "Mushroom Type" (in https://github.com/farmOS/farm_mushroom).
With that in mind... what if we change "Crop Family" to "Plant Family" for now (accepting that it is still only referenced by "Plant Type" terms), and open a new issue for considering migrating this to a new "Family" taxonomy in the future?
If you agree, and have a chance to make that change today, please do! Otherwise I will do it before this gets merged. I'll continue reviewing the rest of this branch in the meantime...
Comment #7
m.stentaI think we should also merge the
farm_plant_typemodule into thefarm_plantmodule, similar tofarm_animal_type(see comment: https://www.drupal.org/project/farm/issues/3186106#comment-13954552).Comment #8
m.stentaYea this is a tricky one... I agree with the approach you've taken for now. I may open a separate issue for broader considerations moving forward...
Comment #9
m.stentaI spun off a new issue for the taxonomy term bundle plugin discussion: #3190851: Enable bundle plugins for taxonomy terms
Comment #10
m.stentaThe more I think about this, the more I wonder: should we just do this now? It would probably be less work to do it now than in the future. And we could add a family reference field to animal type terms as a nice little bonus.
@paul121 - Do you see any issues with this idea? Curious how it might affect your custom module.
Comment #11
m.stentaI went ahead and did this in my branch (rebased onto
2.xand pushed to my fork: https://github.com/mstenta/farmOS/tree/2.x-farm_plant_type).I also made the "Plant type" field capable of referencing multiple terms, and made it a required field, to match 1.x.
Comment #12
m.stentaI took a quick pass at this in my branch, to see what it might look like. Haven't tested all the changes yet...
Comment #13
m.stenta@paul121 - I noticed you made a separate migration for the "companions" field - in the
farm_migrate_referencegroup. I think we can actually do this as part of the "plant type" migration itself, because it only references other terms of the same vocabulary.The reason we separate the "asset parents" field migrations out to the
farm_migrate_referencegroup is because we migrate each asset type separately. The Migrate API offers the ability to automatically create "stub" terms during a migration, which it fills in with more details once it gets to them. We intentionally turn this off for the "parents" migrations, because it is *possible* that someone may NOT migrate all of their asset types. So creating stubs *could* result in blank assets being created in that case.But in the case of "Plant type", since the "Companions" field references terms in the same bundle, we know that all of the terms will be created, so we can allow stubs to be created.
I went ahead and merged that into the plant type migration in my branch. But I haven't tested it yet... so it will be good to double check that it still works as expected. If you have some testing data set up already and can give that a quick try please feel free! Otherwise I will try to later this week.
Comment #14
m.stentaI added a quick commit to my branch that adds this field to "Animal type" terms. Still untested - and I'll probably merge it separately, but I'll keep it in this branch for now.
Comment #15
m.stentaSpoke with @paul121 about the "Crop family" (specific) vs "Family" (general) idea, and decided to split that out to a separate issue/discussion: #3191115: [META] Generalize crop_family to family vocabulary?
I've rebased and split my branch into two:
Testing
2.x-farm_plant_typebranch now on a fresh install... but I think it's ready to go otherwise!Comment #16
m.stentaComment #18
m.stentaTested the migrations and it's working great! Thanks again @paul121! Merged!
Comment #19
paul121 commentedAha! That makes sense. I knew about the stub functionality but was curious why we weren't using it elsewhere. Great!