Problem/Motivation
The property that holds the field name is different in FieldStorageConfig ('name') and FieldConfig ('field_name').
That somewhat made sense when we had FieldConfig / FieldInstanceConfig, since the FieldConfig was the "primary" entity (thus it had a 'name'), and FieldInstanceConfigs were secondary entities, referring to a FieldConfig by [entity type + "name of the FieldConfig", i.e 'field_name'].
Now that FieldConfig / FieldStorageConfig are aligned with the more generic concepts in the Entity Field API, the question of "who holds the 'name', who refers to it" shifted a little.
- If we keep the current logic, then the current 'field_name' in FC is wrong, it should be 'field_storage_name', but that feels off because:
- In the Entity Field API, the FieldDefinition is the primary concept. Thus doesn't really make sense that the FSC is the one that holds the "name". We won't change everyone's language habits to say "field storage name" instead of "field name".
Proposed resolution
In the FieldConfig / FieldStorageConfig world, I think it's clearer to view things as "the FC and FSC are related by referring to a common 'field_name'". In other words, neither holds the name, they both refer to a 'field_name'.
--> Use 'field_name' in both FC and FSC
I.e rename 'name' to 'field_name' in FSC
Remaining tasks
Agree, code, review...
User interface changes
None
API changes
- A config entry changes in field.storage.*.yml (thus, according to the beta guidelines, is a schema change that should happen before beta...)
- The corresponding runtime property changes in FieldStorageConfig objects. This should only affect code that creates and interacts with configurable fields specifically, not using FieldStorageDefinitionInterface - that is, probably very little actual runtime code, but a fair amount of tests...
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | interdiff.txt | 867 bytes | yched |
| #12 | 2344821-field_name-12.patch | 177.6 KB | yched |
| #8 | interdiff.txt | 4.42 KB | yched |
| #7 | 2344821-field_name-7.patch | 177.24 KB | yched |
| #4 | interdiff.txt | 24.12 KB | yched |
Comments
Comment #1
yched commented1st patch, let's see what testbot says.
Comment #2
yched commentedUnrelated, but found out #2344843: hook_field_storage_config_update_forbid() sample code still uses ArrayAccess on FieldStorageConfig while tracking our field_storage_config uses.
Comment #4
yched commentedMigrate fails beat me for now.
Comment #5
swentel commented@yched is this in a branch ? I can work on it further then if it's not green yet on saturdaymorning and talk it through with alex and catch already.
Comment #7
yched commentedShould fix the migrations.
Comment #8
yched commentedForgot the interdiff
Comment #9
yched commentedAlso, pushed to http://cgit.drupalcode.org/sandbox-yched-1736366, branch 2344821-field_name
Comment #11
andypost+1
Comment #12
yched commentedFixes the last migration fail
(+ reroll after #2344979: Rename field type list_text to list_string)
Comment #13
swentel commentedDiscussed in entity meeting, approved by fago and effulgentsia, so we can do this.
Comment #14
alexpottCommitted 4202482 and pushed to 8.0.x. Thanks!
After the beta we should look to protect all the properties on the field config entities.
Comment #16
xanoThis broke HEAD. Working on it.
Comment #17
swentel commentedThere's a patch at #2336743: When more than one language is added in the profile, the installer ignores those already.
Comment #19
swentel commentedGot fixed in #2345607: Translated taxonomy terms not rendered in the entity display language already.
Comment #20
larowlanCan we have a change record?