#3562989: Implements RevisionLogInterface for Instance entity show some issues in our current management of "saved" data:

  • we are storing data identical to, so duplicated of, the ones already stored by displays in config and content storage. This is useless, take storage space and error prone. What is happening if the data are diverging? how to reconciliation?
  • this saved field has no equivalence in Revisions API

So, lets fix it before adopting Revisions API.

Proposed resolution

Instance entity must be aware of the buildable plugin

Add a buildable string base field to InstanceEntity. If an instance entity is created by a DisplayBuidable plugin, the plugin ID is stored in the base field.

Add PublishableInterface::getBuildablePlugin(): the plugin ID with the already stored contexts must be enough to load any DisplayBuidable plugin from any Instance entity.

Use the new mechanism in event subscribers

Thanks to the new ::getBuildablePlugin(), the logic of each DisplayBuilderEvents::ON_SAVE subscribers (1 per sub-module) become strictly identical, so we can do 3 simplifications:

  • Merge them into a single subscriber service in main module.
  • DisplayBuildable::getContextRequirement() can become protected, rely on plugins attributes, and be removed from the interface.
  • We can pass the Instance entity coming from the controller in DisplayBuilderEvents instead of recretaing it from Instance ID.

We can do even more: remove the event all together ! We already have the instance, so we already know the buildable plugin, so we can publish.

Move the publication logic to display buildable plugins

In:

  • Instance::saveIsCurrent() check the hash of the buildable sources and compare
  • Instance::hasSave() becomes ::getPublishedHash()

Then:

  • Remove save base field
  • Remove Instance::setSave()

Remaining tasks

Leveraging the contexts may also be the opportunity to less rely on the "black magic" of parsing instance ID string. See: #3573905: Simplify DisplayBuildableInterface

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:

Comments

pdureau created an issue. See original summary.

pdureau’s picture

Status: Active » Needs work

Work in progress

Remaining:

  • make DisplayBuildableInterface::createFromInstanceEntity() implementations work only with contexts instead of parsing instance entityt ID
  • get timestamp of published data in LogsPanel
  • finish InstancePublishingTest
  • update doc?
pdureau’s picture

Issue summary: View changes
pdureau’s picture

Works about context has been moved to its own issue #3579299: Tidy context management

pdureau’s picture

Do we store the instance ID in a field or do we rely on ContextAwarePluginManagerTrait::getDefinitionsForContexts() ?

pdureau’s picture

Chained issues: #3562989: Implements RevisionLogInterface for Instance entity is rebasing the current issue which is rebasing #3579299: Tidy context management which is rebasing 1.0.x

pdureau’s picture

We may also need to get rid of profile content field in Instance entities.

And what about ApiController::revert() ? There is some logic belonging to entity_view_override buildable plugin here. Do we introduce a "revertable" interface for buildable plugins? Maybe in an other issue.

pdureau’s picture

Issue summary: View changes
pdureau’s picture

Issue summary: View changes
pdureau’s picture

Issue summary: View changes
pdureau’s picture

Issue summary: View changes
pdureau’s picture

We are getting close ;)

TODO:

  • Fix the failing tests: InstancePublishingTest, ApiPublishingControllerTest, playwright...
  • Remove InstanceInterface::getBuildablePlugin() ?
  • update doc?

Follow-up:

pdureau’s picture

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

Ready to review.

This MR is embedding #3579299: Tidy context management and embedded in #3562989: Implements RevisionLogInterface for Instance entity, which can be reviewed together or separately.

pdureau’s picture

Assigned: mogtofu33 » Unassigned
Status: Needs review » Closed (outdated)

Time goes by and Jean (@mogtofu33) clearly expressed the will to review all changes in a single MR, the one from #3562989: Implements RevisionLogInterface for Instance entity.

It is not necessary to keep this intermediary ticket in review.

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.