DDEV is the official local development tool of Drupal. And like Drupal, DDEV depends on the support of the open source community.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 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: 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!