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...

Comments

yched’s picture

Status: Active » Needs review
StatusFileSize
new151.71 KB

1st patch, let's see what testbot says.

yched’s picture

Unrelated, but found out #2344843: hook_field_storage_config_update_forbid() sample code still uses ArrayAccess on FieldStorageConfig while tracking our field_storage_config uses.

Status: Needs review » Needs work

The last submitted patch, 1: 2344821-field_name-1.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
StatusFileSize
new172.83 KB
new24.12 KB

Migrate fails beat me for now.

swentel’s picture

@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.

Status: Needs review » Needs work

The last submitted patch, 4: 2344821-field_name-4.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
StatusFileSize
new177.24 KB

Should fix the migrations.

yched’s picture

StatusFileSize
new4.42 KB

Forgot the interdiff

yched’s picture

Also, pushed to http://cgit.drupalcode.org/sandbox-yched-1736366, branch 2344821-field_name

Status: Needs review » Needs work

The last submitted patch, 7: 2344821-field_name-7.patch, failed testing.

andypost’s picture

I.e rename 'name' to 'field_name' in FSC

+1

yched’s picture

Status: Needs work » Needs review
StatusFileSize
new177.6 KB
new867 bytes

Fixes the last migration fail
(+ reroll after #2344979: Rename field type list_text to list_string)

swentel’s picture

Status: Needs review » Reviewed & tested by the community

Discussed in entity meeting, approved by fago and effulgentsia, so we can do this.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 4202482 and pushed to 8.0.x. Thanks!

After the beta we should look to protect all the properties on the field config entities.

  • alexpott committed 4202482 on 8.0.x
    Issue #2344821 by yched: Unify 'name' / 'field_name' in FieldConfig /...
xano’s picture

Assigned: Unassigned » xano
Status: Fixed » Active

This broke HEAD. Working on it.

swentel’s picture

  • Gábor Hojtsy committed ca513fb on 8.0.x
    Issue #2345607 followup by webflo: The combination of #2345607 and #...
swentel’s picture

larowlan’s picture

Can we have a change record?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.