Problem/Motivation

Child of #3624396: Plan: Harden InstanceInterface, IslandInterface and DisplayBuildableInterface as public API before RC1. An open builder learns about a change made elsewhere through the event stream, which relays what SseState records. Only the builder's API controllers record a change, after the events that need it, and display_builder_ai does it by hand. Any other writer, a drush command, a migration, a module calling InstanceInterface, records nothing, so open builders keep showing the old state.

Proposed resolution

  • Record the change when an instance is saved as a new revision, every edit, undo and redo included. In-place saves are skipped: adding a translation on GET, and the resave after a mutator. Publishing records too, although its save is in place: open builders show the publish state.
  • Keep the session of the request that made it, so the stream still skips the author, who already has the change on screen. A save outside a request records none, and reaches everyone.
  • Drop saveSseData() and the SSE_EVENTS list from the controllers, and the manual record from display_builder_ai.
  • The builder UI events and their island fan-out stay as they are: they build the response of the current request.
  • Kernel tests: an edit through InstanceInterface and a publish are recorded, adding a translation is not.

Remaining tasks

None.

User interface changes

An open builder now shows a change made outside the builder.

API changes

None.

Data model changes

Issue fork display_builder-3627279

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

mogtofu33 created an issue. See original summary.

mogtofu33’s picture

mogtofu33 commented
Issue tags: -display_builder-1.0.0-beta9 +display_builder-1.0.0-beta
pdureau’s picture

pdureau commented
Assigned: Unassigned » pdureau

Drop saveSseData() and the SSE_EVENTS list from the controllers, and the manual record from display_builder_ai.

That's tempting 😉

Let's see if this proposal can be done without adding too much complexity. It will help us deciding if we keep in 1.0.0-beta scope or not.

Th goal would be to move logic from Controller(s) to Instance entity, without touching anything else, and without altering any tests.

Let's also check publishing actions (publish, revert, restore) actions in a SSE context. It was already not working great (discussed in #3621930: Island cannot decline, nor ask for the event stream) and we can hope it will work better now.

Disclaimer: I have little knowledge about display_builder_ai, so I hope

pdureau opened merge request !522

pdureau’s picture

pdureau commented
Status: Active » Needs review

I pushed the most naive and straightforward change I could think of: " move logic from Controller(s) to Instance entity, without touching anything else, and without altering any tests."

Is the SSE working properly? Is it enough related to the initial ambition of the ticket? Let's check before sending to proper review.