Problem/Motivation
The biggest part of the issues this module has are caused by its custom field storage implementation. This also adds a lot of unnecessary complexity to the module. It's reinventing the wheel, since Drupal has a whole field storage system built in that automatically deals with things like translations and revisions.
Proposed resolution
Rewrite the module to stop using the custom storage service. Instead, make the caption a standard field property whose storage is handled like any other field property in Drupal.
To address these issues, the module should be refactored to eliminate the custom storage service and instead leverage Drupal's default field storage system. This involves redefining the caption as a standard field property, allowing Drupal to handle its storage, translations, and revisions natively. By doing so, the module will:
- Simplify the codebase: Remove the custom storage implementation and rely on Drupal's proven field storage mechanisms.
- Improve maintainability: Reduce the risk of bugs and inconsistencies by using core functionality.
- Enhance compatibility: Ensure seamless integration with other Drupal features, such as translations and revisions, without additional custom logic.
- Follow best practices: Align with Drupal's architecture by utilizing its built-in field storage system.
Implementation Steps
- Remove Custom Storage Service:
- Delete the custom storage service and related classes.
- Ensure all dependencies on the custom storage are removed.
- Define Caption as a Field Property:
- Modify the field type plugin to define the caption as a standard field property.
- Update the schema and property definitions to reflect this change.
- Update Widget and Formatter:
- Adjust the field widget to handle the caption property as part of the default field storage.
- Update the field formatter to display the caption property correctly.
- Migrate Existing Data:
- Provide an update hook or migration path to transfer existing caption data from the custom storage to the default field storage.
- Test Thoroughly:
- Ensure the changes work seamlessly with translations, revisions, and other Drupal features.
- Verify that existing data is preserved and displayed correctly after the migration.
Remaining tasks
Provide an upgrade path.
Issue fork image_field_caption-3472997
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
dieterholvoet commentedThe minimum Drupal version is bumped to 9.2 because of the usage of the usage of the new field widget hook. I also added back the Image module dependency since it was incorrectly removed in #3355337: Support Drupal 10. Setting to Needs work for the upgrade path implementation.
Comment #4
bachbach commentedexactly what i though after hours trying to make caption in nested parragraph translation to work :(
then i found your issue and your patch work like a charm!
had to :
pmu the module (obviously delete all the caption contents)
composer require dev version of module
apply your patch
enable the module again
thanks for your work !
Comment #5
bachbach commentedComment #6
foxy-vikvik commentedComment #7
foxy-vikvik commentedComment #8
foxy-vikvik commentedComment #9
dieterholvoet commented@foxy-vik why did you make the caption a plain text field instead of a formatted text field in #e2e9925? In the current version of the module it's also a formatted text field, so I don't see why you would change that.
Comment #10
foxy-vikvik commented@dieterholvoet It was a mistake. I found an issue and changed it. This merge request is still a work in progress. So it will be formatted text. I will change it when I have time.
Comment #12
foxy-vikvik commentedComment #13
kksandr commentedComment #14
kksandr commentedComment #15
foxy-vikvik commentedComment #17
foxy-vikvik commentedCommitted to 4.0.x. Thanks!
Comment #18
dieterholvoet commentedMaybe it could be an idea to not drop the old tables just yet, just in case there are still issues with the migration? The dropping of the tables could be added once a stable release of the module is made.
Comment #20
kksandr commentedThat's a good idea, let's do it that way.