Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
Once #2050843: Users 0 and 1 are created without a UUID is in, clean up the @todo added in #2090541: Avoid storing numeric target_id's for default values in field instance CMI files related to this issue in ConfigurableEntityReferenceFieldItemList
Proposed resolution
Remove the code related to the old exception with users 0 and 1. Maybe reuse code in this class and remove TaxonomyTermReferenceFieldItemList
Remaining tasks
Remove it.
User interface changes
None
API changes
None
Comment | File | Size | Author |
---|---|---|---|
#16 | entity_uuids-notices-2162005-16.patch | 898 bytes | Berdir |
#12 | interdiff.txt | 785 bytes | plopesc |
#12 | clean_user_uuid_exception-2162005-12.patch | 11.37 KB | plopesc |
Comments
Comment #1
plopescWhile removing code exceptions for users in
ConfigurableEntityReferenceFieldItemList
I realized that resultant code is almost the same asTaxonomyTermReferenceFieldItemList
, so the second class could be removed, usingConfigurableEntityReferenceFieldItemList
as list_class for bothTaxonomyTermReferenceItem
andConfigurableEntityReferenceItem
.This approach reuses the code, but forces to make enitty_reference module as a dependency for taxonomy module. Maybe, if you consider this approach, we could move this class where both modules can access it. Otherwise, we can just remove only some lines from
ConfigurableEntityReferenceFieldItemList
, leavingTaxonomyTermReferenceFieldItemList
as is now.Regards.
Comment #2
plopescComment #4
yched CreditAttribution: yched commentedThanks for following up on this, @plopesc.
The extra dependency is a bit unfortunate IMO.
But, in the same spirit than #2156337: merge ConfigEntityReferenceItemBase up into EntityReferenceItem, and fix inconsistencies, I think the methods could be moved to an EntityReferenceItem*List* class at the level of the \Core EntityRef field type (Core\Field\Plugin\Field\FieldType\EntityReferenceItem), that all other "reference" field types would use too.
(as a side note, I bumped #2073661: Add a EntityReferenceField::referencedEntities() method, which also has a case for introducing an EntityReferenceItemList class)
Comment #5
plopescThanks for your review, @yched
It looks nicer now :)
Comment #7
yched CreditAttribution: yched commented5: clean_user_uuid_exception-2162005-5.patch queued for re-testing.
Comment #8
plopescI believe we can postpone this on #2073661: Add a EntityReferenceField::referencedEntities() method where some we are taking some decisions that could affect this patch.
Pick it up again after fix this one will avoid some unnecessary re-rolls.
Regards
Comment #9
BerdirI think this can be unpostponed.
Comment #10
plopescHello
Here is a re-roll of the previous patch in #5.
I made some local tests and worked. Let's see what testbot says...
Comment #12
plopescAdded check to avoid notices when fields return default values which does not include target_uuid
Comment #13
yched CreditAttribution: yched commentedAwesome !
+ removal of two specialized list classes : awesome++
Comment #14
alexpottCommitted f2015b7 and pushed to 8.x. Thanks!
Comment #16
Berdir$entity_uuids is not defined and is not doing an isset() check, so if targets actually don't exist, this is throwing php notices.
Comment #17
BerdirComment #18
swentel CreditAttribution: swentel commentedGood to go when green.
Comment #19
alexpottCommitted a5e7c59 and pushed to 8.0.x. Thanks!