EntityViewDisplay and EntityFormDisplay contain "third party settings" at two levels :
- at the top level of the Display (3rd party settings for the display, like DS layout)
- at the level of each field displayed in the 'content' entry (3rd party settings for the formatter)
Both locations currently use the same schema entry:
entity_view_display.third_party.[module]
entity_form_display.third_party.[module]
They shouldn't, those are completely different, and we'll find completely different stuff here.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 2397807-formatter_3rd_party_schema-5.patch | 3.52 KB | yched |
Comments
Comment #1
yched commentedComment #2
gábor hojtsyI meant to open an issue for this two weeks ago but it somehow fallen through the cracks. So true. Any proposed naming scheme?
Comment #3
yched commentedI'd say:
- entity_(view|form)_display.third_party.[module] makes sense for the top-level one.
- we have since then unified field.(widget|formatter).settings for widget/formatter settings,
so maybe field.(widget|formatter).third_party.[module] for the third party settings entry in individual fields ?
Comment #4
yched commentedAlso - it would IMO make sense to extract
core.entity_view_display.*.*.*[content][sequence]to a separate type entry, since it's going to be reused by anything that uses field formatters & widgets outside of an EntityDisplay (e.g Views)
Comment #5
yched commentedPatch introduces separate schema entries, as per #3 :
field.widget.third_party.[module]
field.formatter.third_party.[module]
Not doing #4 for now, still mulling on that.
Comment #6
gábor hojtsyWe should also keep tests for the top level third party settings, no? Now renamed this applies to the formatter/widget level, not the display level. Was this always testing the formatter/display level?
Comment #7
yched commentedYes, the ones in field_third_party_test.schema.yml were about widget/fomatter - see field_third_party_test.module
HEAD has existing tests for the "display-level" 3rd party settings, that's
entity_view_display.third_party.entity_test in entity_test/config/schema/entity_test.schema.yml - see entity_test_entity_presave()
Patch doesn't touch those, they are still here.
So I think we're good in terms of testing.
Also, after some more thought, I'm not sure anymore #4 is a good idea. Views only uses a subset of the entries present in EVD (doesn't use 'weight' & 'label'. It can always duplicate the third_party entry if it actually uses them (not even sure it does atm).
So yeah, patch #5 is my candidate patch for review & commit :-)
Comment #8
gábor hojtsyThanks for the clarification on the testing bit. Looks good to me :)
Comment #9
alexpottNice catch. This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed c791cc2 and pushed to 8.0.x. Thanks!