Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add images to asset-related taxonomies #536

Closed
wants to merge 2 commits into from

Conversation

c4ndel4
Copy link

@c4ndel4 c4ndel4 commented May 20, 2022

With the addition of entity views on taxonomy terms (#458) this pages are more used and having a image could be helpful.

Copy link
Member

@paul121 paul121 left a comment

Choose a reason for hiding this comment

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

This is a great start @c4ndel4 ! Some thoughts:

The image field definitions field.field.taxonomy_term.*.image look good to me. As you mentioned on the forum I think you are correct that this won't require any update hooks for the field storage since we're reusing the existing field.storage.taxonomy_term.image config and thus the taxonomy_term__image table. Although I do have a few questions about this:

  • Right now field.storage.taxonomy_term.image is provided by the farm_plant_type module. So this means that farm_animal_type and farm_material_type would need to depend on farm_plant_type. Should we (can we?) move this field storage to a more common core module?
  • I think we would still need an update hook to create this field.field.taxonomy_term.*.image field config on existing installs.

The entity view and form displays are another beast.. I think we might need an update hook to create them as well. Ideally we could use bundle fields on taxonomy terms like we do for our assets/logs/plans. That would effectively solve all the above problems. But iirc it isn't this easy to turn on bundle fields for taxonomy terms... hmm.

@paul121
Copy link
Member

paul121 commented May 20, 2022

Linking to a related issue: https://www.drupal.org/project/farm/issues/3244880

Perhaps we could add a file field to these taxonomies at the same time.

@mstenta
Copy link
Member

mstenta commented May 20, 2022

Thanks @c4ndel4!

I agree w/ @paul121 - we should add file field at the same time.

And we need update hook(s) to update config on existing sites.

@c4ndel4
Copy link
Author

c4ndel4 commented May 21, 2022

Thanks for the replies, but you have named the beast that terrifies me the most... The update_hook

  • Right now field.storage.taxonomy_term.image is provided by the farm_plant_type module. So this means that farm_animal_type and farm_material_type would need to depend on farm_plant_type. Should we (can we?) move this field storage to a more common core module?

Yes, probably move the crop_family to its own module and define all common fields in there. I did something similar with a new taxonomy "species" that would replace "crop_family". But maybe is better to keep the existing "crop_family" and if a nerd wants to call it species he (I) can change the label of that field.

The entity view and form displays are another beast.. I think we might need an update hook to create them as well.

I already hate them. I made a module to add the species taxonomy without deleting the crop_family (https://github.com/c4ndel4/farm_species) and this is where all problems come from. The configuration items already exists and I must delete them before installing. And when I uninstall the module, the core views are lost. I thought I could read the yml files from the filesystem but I don't really know where the module it's going to be installed...

Ideally we could use bundle fields on taxonomy terms like we do for our assets/logs/plans. That would effectively solve all the above problems. But iirc it isn't this easy to turn on bundle fields for taxonomy terms... hmm.

Could you point me to an example? Adding fields to an asset or a log was really easy and would be great to provide the same simplicity to other entities.

@mstenta
Copy link
Member

mstenta commented May 23, 2022

Yes, probably move the crop_family to its own module and define all common fields in there.

No, crop_family should stay where it is. And it wouldn't make sense to package these common fields in the same module as it anyway. I'm not sure where the best place would be for them, honestly. The basic need would be a module for term field storage config, since that's the only place we need it (all other entity types are covered by bundle plugins provided by the farm_entity module).

Ideally we could use bundle fields on taxonomy terms like we do for our assets/logs/plans. That would effectively solve all the above problems. But iirc it isn't this easy to turn on bundle fields for taxonomy terms... hmm.

I agree this would be ideal but I am very skeptical that it is worth the effort and potential maintenance cost moving forward.

@mstenta
Copy link
Member

mstenta commented Mar 20, 2024

I've opened a new PR that adds file and image fields to all taxonomy terms via hook_entity_base_field_info() (so we don't need config entities for every bundle):

I'm going to close this one. @c4ndel4 if you are using these changes in your own instance right now then my PR may cause problems for you. Happy to help you transition to the new PR's approach if you need!

@mstenta mstenta closed this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants