Problem/Motivation

Inspired from both:

We are observing the emergence of 2 distinct behaviours related to Instance & Events:

State alterations

Those methods and events never reach the DisplayBuildable plugins and alter only the Instance entity and its "volatile" storage (through SourceTree): attachToRoot, attachToSlot, moveToSlot, remove...
state

Publishing

Those methods and events pass through the DisplayBuildable plugins and alter the Instance entity and the permanent storage: publish (actually save in some part of the code, let's tidy that), restore & revert
state

Proposed resolution

It is important to distinct those 2 mechanisms to secure the upcoming code changes, so we know we have 2 clear paths, one where DisplayBuildable must never be called, and one where they are the leading logic.

On controllers

Introduce ApiPublishingController

From To
ApiController::save() ApiPublishingController::publish() (and rename the related display_builder.api_save route to display_builder.api_publish, with its path)
ApiController::restore() ApiPublishingController::restore()
ApiController::revert() ApiPublishingController::revert()

And split related tests.

On Instance entity

Introduce PublishableInterface

In Instance entity methods:

From To
InstanceInterface::canSaveContextsRequirement() PublishableInterface::isPublishable() (we can also remove the parameter)
InstanceInterface::hasSaveContextsRequirement() (whatever, it will not last anyway)
InstanceInterface::saveIsCurrent() PublishableInterface::isPresentPublished() or ::isPublishedPresent()
InstanceInterface::setSave() (whatever, it will not last anyway)
InstanceInterface::hasSave() PublishableInterface::isPublished()

There is also EntityPublishedInterface, but it doesn't seem to match our model.requirements?

So, the newly introduced PublishableInterface looks like something like that:

interface PublishableInterface {
  public function restore(): void;
  public function isPublished(): ?int;
  public function isPresentPublished(): bool;
  public function isPublishable(): bool;
}

Surprisingly, publish() and revert() are not needed yet in this new interface, but it will happen soon

And we can extract InstancePublishingTest from InstanceHistoryTest & InstanceTest

On events

  • Rename ON_SAVE event to ON_PUBLISH
  • Rename onSave() subscriber method to onPublish()
  • More?

Other

There are some strings and comments here and there referencing "save" instead of "publish".

CommentFileSizeAuthor
Capture d’écran du 2026-03-30 14-00-52.png21.03 KBpdureau
Capture d’écran du 2026-03-30 14-00-47.png12.07 KBpdureau

Issue fork display_builder-3582234

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’s picture

pdureau commented
Issue summary: View changes
pdureau’s picture

pdureau commented
Issue summary: View changes

pdureau opened merge request !263

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » pdureau
Status: Active » Needs review
mogtofu33’s picture

mogtofu33 commented
Status: Needs review » Needs work
pdureau’s picture

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

Hello @mogtofu33,

What do you think about the MR https://git.drupalcode.org/project/display_builder/-/merge_requests/263/... ?

The 2 main changes, which are important to make next developments easier are:

  • renaming from "save" to "publish" here a there, so now we use the same terminology everywhere
  • the new PublishableInterface with better named methods

I also did some splits following PublishableInterface scope, less immediately useful but which make sense to me:

  • split of controllers
  • split of controller tests
  • split of instance tests

Are you OK? Would it be relevant to also split IslandEventSubscriberInterface and the events classes? Do we need more documentation about this distinction?

mogtofu33’s picture

mogtofu33 commented
Status: Needs review » Needs work

Thanks for that.

You right we could split events as well, and ON_REVERT should probably be moved in display_builder_entity_view module.

In the meantime I would like to move interface used by a single or very few classes to related folder, will propose a commit. So we keep only cross based interfaces in src.

And create a src/Island layer for Island related classes. Will add a commit for that too.

pdureau’s picture

pdureau commented

and restore should probably be moved in display_builder_entity module.

Could restore stay something generic but only used by entity view display for now?

Maybe upcoming display buildable plugins from contrib or custom projects will need such mechanism.

mogtofu33’s picture

mogtofu33 commented

Right, I revert a change and move back to split event and subscribers with interfaces.

mogtofu33’s picture

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

Looking good, let merge, keep open for feedback.

  • mogtofu33 committed 9b6e1081 on 1.0.x authored by pdureau 
    task: #3582234 Split publishing logic from state logic

by: pdureau
by:...
pdureau’s picture

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

The goal of this issue was to make the distinction between:

  • State alterations: the methods and events never which reach the DisplayBuildable plugins and alter only the Instance entity and its "volatile" storage (through SourceTree): attachToRoot, attachToSlot, moveToSlot, remove...
  • Publishing: Those methods and events pass through the DisplayBuildable plugins and alter the Instance entity and the permanent storage: publish, restore & revert

Something clear, simple and welcoming for contributors.

However, we landed with:

  • 4 new events classes:
    • DisplayBuilderEvent: with ON_HISTORY_CHANGE, ON_RESTORE, ON_REVERT
    • DisplayBuilderNodeEvent with ON_ATTACH_TO_ROOT, ON_MOVE, ON_UPDATE
    • DisplayBuilderSlotEvent with ON_ATTACH_TO_SLOT
    • DisplayBuilderDeleteEvent with ON_DELETE
    • DisplayBuilderDataEvent: with ON_ACTIVE, ON_PUBLISH
  • 4 new interfaces:
    • IslandActiveEventInterface: onActive
    • IslandLifecycleEventsInterface: onHistoryChange, onRestore, onRevert
    • IslandSaveEventsInterface: onPublish, onPresetSave
    • IslandStructureEventsInterface: onAttachToRoot, onAttachToSlot, onMove, onUpdate, onDelete

So, we have some issues here:

  • ⚠️ This looks overly complicated for a MR dedicated to simplification: all those new classes, interfaces, traits, the METHOD_INTERFACE_MAP in IslandFanOutTrait, the match in createEventWithEnabledIsland, 1200 lines added in total, and a 120 lines architecture.md needed to have some idea of what is happening here
  • ⚠️ There is no consistency between events and island interfaces (example: ON_PUBLISH is with ON_ACTIVE but onPublish is with onPresetSave)
  • ⚠️ Some events has been made too limited (example: ON_DELETE lost the Node ID, I guess because we are not using it right now in available islands but we need to be future-proof, and keep sending the useful information)

So I am proposing to merge back the events and the interfaces:

  • as single files like it was before
  • or with 2 files each following ApiController/ApiPublishingController and InstanceInterface/PublishableInterface splits introduced in this MR

pdureau changed the visibility of the branch 3582234-simplify to hidden.

pdureau changed the visibility of the branch 3582234-simplify to hidden.

pdureau changed the visibility of the branch 3582234-simplify to hidden.

pdureau changed the visibility of the branch 3582234-simplify to active.

pdureau opened merge request !265

pdureau’s picture

pdureau commented

I am opening a new MR to prepare the first proposal ("merge back the events and the interfaces as single files like it was before") : https://git.drupalcode.org/project/display_builder/-/merge_requests/265
There are still some mention of removed interfaces in docs & comments, but most of the work is done. I hope it will help.

If you believe the second proposal ("... with 2 files each following ApiController/ApiPublishingController and InstanceInterface/PublishableInterface splits") is better, go ahead.

mogtofu33 opened merge request !266

mogtofu33’s picture

mogtofu33 commented

My goal was to prepare something to have events more flexible and isolated to let a custom island create and use it's own events. Like for
ON_PRESET_SAVE, ON_REVERT and ON_HISTORY_CHANGE which are specific to an island feature and not 'core'. Best being a custom module to be able to simply add a feature (api + event + island).

Revert here and will do on other MR for that to do it properly.

I am not sure about your 'future proof' point, we have data in the event but the subscriber force fixed parameters, so we already assume and force the event behavior, ON_DELETE node id passed but not used is an example of this 'de-sync'.

mogtofu33 closed merge request !266

mogtofu33 changed the visibility of the branch 3582234-publish-simplify to hidden.

  • mogtofu33 committed b1733342 on 1.0.x authored by pdureau 
    task: #3582234 Split publishing logic from state logic - simplify

by:...
mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Needs work » 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.