Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
DER field makes your field dependent on every module that provides an entity type.
Proposed resolution
Only add dependencies to referenceable entity types.
Remaining tasks
- Tests.
- Upgrade path.
- Upgrade path test.
- Review.
- Commit.
User interface changes
None
API changes
None
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | fix_incorrect_der_field-2839178-19-test-only.patch | 8.03 KB | jibran |
| #18 | der-field-dependencies-tests.patch | 6.04 KB | dpi |
| #5 | only_add_dependencies-2839178-5.patch | 1.99 KB | jibran |
Comments
Comment #2
jibranIt's an easy fix but we have to update existing DER field storage config.
Comment #3
jibranWe are not changing the existing config schema. We only need to update the existing config object so added a post-update hook.
Comment #5
jibranComment #6
jibranComment #7
larowlanNice work
Comment #8
dpiI see that it DER was already attempting to "add dependencies to referencable entity types", its just that its implementation was broken. Updated issue title to reflect this.
Why not create a hook_update_N to run these updates? instead of a post_update. You should only need to set dependencies once manually. Subsequent updates to field storage will be made with
DynamicEntityReferenceItem::calculateStorageDependencies(Note: First hook_update_N should be be 8101)
Needs tests for all variants of
DynamicEntityReferenceItem::getTargetTypesComment #9
jibranUpdate hooks are used when we change the config schema which we are not in this case because we are allowed to change the schema during update hook there is a chance the schema can be invalid and therefore we can't use entity API in update hooks. To avoid the invalid schema situation during the site update process hook_post_update is introduced so the use of post_update hook is perfectly valid in this case.
Well it'd be great to have the tests for this but upgrade path test to update field config are not easy to write you need to update the whole config table manually to add fields on top of that you have to enable the DER during the DB import which is also a pain. I don't want to invest so much energy in re-saving of a config entity. If someone is interested in writing the test please feel free to report a follow-up task. I'll be more than happy to help them out.
Anyway, thanks for the review. Your input is always useful. And thank you for reporting #2844921: Rename dynamic_entity_reference_update_8001.
While writing the reply is reviewed the patch again and I think it is ready. Let's get this in and if someone will hit a bug we can always write tests for it then. :P
Comment #11
jibranCommitted and pushed to 8.x-1.x. Because 8.x-2.x in alpha so I didn't commit the post_update hook. Thanks!
Comment #13
dpiModify the config directly via configFactory. Example of dep update in
field_update_8002.I didn't mean update path tests, although that would be great.
I mean to test that dependencies were created depending on target types:
target_typemanually (test each variant)Comment #14
jibranWell, that should be done in post_update hook. Let me confirm that with @alexpott and @amateescu.
Oh! for this yeah we can add some asserts to an existing KTB. We are creating tons of fields in tests.
Comment #15
dpiI found the entity_reference dependency test:
\Drupal\KernelTests\Core\Entity\EntityReferenceFieldTest::testEntityReferenceFieldDependenciesObviously DER would need to add more tests than this.
Comment #16
dpiChanging to active, at least for the tests.
Comment #17
dpiTests for settings variants modifying dependencies.
---
A new test class was created instead of using
DynamicEntityReferenceFieldTest, rationale:The reason there is another test entity is so that a new provider is made available for diffing.
I used the
$canonicalizeparam inassertEquals. This ignores order of array values, and therefore keeps the $expected variables clean.For my reference: https://github.com/dpi/dynamic_entity_reference/tree/field-dependencies-...
Comment #18
dpiYou might need this
Comment #19
jibranAwesome! work. Here is a test only patch.
Comment #21
dpi0/4 new tests pass. Looks good to go.
Comment #24
jibranIndeed. Thanks! Committed and pushed to 8.x-1.x and 8.x-2.x