Problem/Motivation

While working on #3594228: Remove explicit references to Layout Builder & #3542276: Embedding a display in an other with a formatter, I have noticed some complexity to orchestrate in a safe and explicit manner the relationship between EntityView and EntityViewOverride.

For example:

  • we have a display_builder_entity_view specific logic in StateButtons
  • Instance::revert() is currently emptying the sources. It is never supposed to happen from UI, but the logic exists and is dangerous
  • it is hard to implement those changes we may want sooner or later: do we disable and/or empty and override when the parent is disable? Do we fallback to the overridden when the overridable when some conditions are not met?

We need an official, "first class", overriding "contract", instead of relying on loosely specified snippets from the early days of the project.

Proposed resolution

Add a DisplayBuildableOverrideInterface extending DisplayBuildableInterface with:

  • public const OVERRIDE_FIELD_PROPERTY = 'override_field'; from DisplayBuildableInterface
  • public const OVERRIDE_PROFILE_PROPERTY = 'override_profile'; from DisplayBuildableInterface
  • public function revertSources(): array; from DisplayBuildableInterface
  • a new public function getOverriden(): DisplayBuildableInterface;

Implement it in EntityViewOverride and adapt the logic in Instance entity, StateButtons and elsewhere.

Issue fork display_builder-3618424

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pdureau created an issue. See original summary.

pdureau opened merge request !352

pdureau’s picture

pdureau commented
Title: Add an DisplayBuildableOverrideInterface » Add DisplayBuildableOverrideInterface
Assigned: pdureau » mogtofu33
Issue summary: View changes
Status: Active » Needs review

Done. Pipeline green.

The main benefit of such a change can be seen in the simplification of StateButtons::isOverridden() in the MR. I expect similar simplification when we will work on #3594228: Remove explicit references to Layout Builder & #3542276: Embedding a display in an other with a formatter.

Other nice stuff:

  • reverting a non-revertable instance is now impossible instead of emptying data
  • no more explicit references to display_builder_entity_view in the main display_builder module
  • it will be straightforward to add more overriding buildable implementation in the future and/or the ecosystem

However, I am not sure about terminology:

  • DisplayBuildableOverrideInterface VS OverridingDisplayBuildableInterface
  • ::getOverriden() vs ::getOverridenBuildable()

What do you think?

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Needs review » Needs work

Current semantic is good for me. Just some cleanup to do.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs work » Needs review

Thanks you for the feedback. I did the changes.

  • mogtofu33 committed a2175198 on 1.0.x authored by pdureau 
    task: #3618424 Add DisplayBuildableOverrideInterface

By: pdureau
mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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