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.

Files: 
CommentFileSizeAuthor
#5 2397807-formatter_3rd_party_schema-5.patch3.52 KByched
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,552 pass(es). View

Comments

yched’s picture

Issue summary: View changes
Gábor Hojtsy’s picture

I meant to open an issue for this two weeks ago but it somehow fallen through the cracks. So true. Any proposed naming scheme?

yched’s picture

Issue summary: View changes

I'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 ?

yched’s picture

Also - 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)

yched’s picture

Status: Active » Needs review
FileSize
3.52 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,552 pass(es). View

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

Gábor Hojtsy’s picture

Status: Needs review » Needs work

We 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?

yched’s picture

Status: Needs work » Needs review

Yes, 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 :-)

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the clarification on the testing bit. Looks good to me :)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Nice 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!

  • alexpott committed c791cc2 on 8.0.x
    Issue #2397807 by yched: EntityDisplay schema for third_party_settings...

Status: Fixed » Closed (fixed)

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