Problem/Motivation

When changing entity view displays new steps get added to the state of a builder. Even when trying to import the config via drush cim it seems like the state config has more priority than the newly imported configuration.

Steps to reproduce

  1. Make changes to your entity view display
  2. Import your view display from config

In order to see my changes I had to execute the following SQL commands:

DELETE FROM key_value_expire WHERE collection = "tempstore.shared.display_builder_sse";
DELETE FROM key_value WHERE collection = 'state' AND name LIKE 'display_builder%';

Proposed resolution

It would be great to reset the state once you import a configuration or to add a new step to the history.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork display_builder-3572328

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

yannickoo created an issue. See original summary.

pdureau’s picture

pdureau commented
Issue tags: +display_builder-1.0.0-beta4
pdureau’s picture

pdureau commented

add a new step to the history.

This is what we already do for imports from Layout Builder (we add a new step in history each time a Layout Builder display is saved)

pdureau’s picture

pdureau commented
Issue tags: +Novice

Maybe we can play with SynchronizableInterface already expected by ConfigEntityInterface implemented in ConfigEntityBase through SynchronizableEntityTrait:

  • View config entity for ViewDisplay buildable
  • PageLayout config entity for PageLayout buildable
  • EntityViewDisplay config entity for EntityView buildable

So, somewhere in ::postSave(), we can check SynchronizableInterface::isSyncing() and trigger Instance::setNewPresent() with a log message saying "Synced from configuration import" or something like that.

gurnoor kaur made their first commit to this issue’s fork.

pdureau’s picture

pdureau commented

Hi gurnoor kaur,

If you want to work on this issue, don't forget to assign it to you.

gurnoor kaur’s picture

gurnoor kaur commented
Assigned: Unassigned » gurnoor kaur

gurnoor kaur opened merge request !252

pdureau’s picture

pdureau commented
Issue tags: -display_builder-1.0.0-beta4 +display_builder-1.0.0-beta5
pdureau’s picture

pdureau commented
Status: Active » Needs work

Also, it must be done for Views config entities. We have no direct control on the entity here, but there is a hook related to ::postSave() I believe

And some related tests would be welcomed.

gurnoor kaur’s picture

gurnoor kaur commented

There was no such hook for the postSave implementation so i added a hook entity_update for config import .

gurnoor kaur’s picture

gurnoor kaur commented
Status: Needs work » Needs review
pdureau’s picture

pdureau commented
Status: Needs review » Needs work

Thanks a lot for your work

I see you are using \Drupal::logger('display_builder');. You may have been misled by this sentence of mine:

with a log message saying "Synced from configuration import" or something like that.

The "log message" here is the second parameter of InstanceInterface->setNewPresent(). You are already correctly using it. No need for Logger service.

Also, is it possible to add some phpunit tests? We will mess a lot with InstanceInterface next weeks, by adopting the Core's Entity Revision API, and we need to be careful not breaking your work.

gurnoor kaur’s picture

gurnoor kaur commented
Status: Needs work » Needs review
yannickoo’s picture

yannickoo
Location Berlin
commented
Status: Needs review » Needs work

Hey there is a merge conflict, therefore setting to Needs work 😇

gurnoor kaur’s picture

gurnoor kaur commented
Status: Needs work » Needs review
pdureau’s picture

pdureau commented
Assigned: gurnoor kaur » pdureau

Ok thanks, I am having a look.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33
Status: Needs review » Reviewed & tested by the community

Looks ok to me.

Manually tested on local env:

  • Page Layout: Config import with change: ✅ OK, imported as a "step"
  • Page Layout: Config import without relevant change: ✅ OK, no "step" added
  • Entity View: Config import with change: ✅OK, imported as a "step"
  • Entity View: Config import without relevant change: ✅ OK, no "step" added
  • Views: Config import with change: ✅ OK, imported as a "step"
  • Views: Config import without relevant change: ✅ OK, no "step" added

I got some weird behaviours with Views (some panels not properly swapped) but there is nothing in the MR which seem to be the cause of that, it may be because of my messy local dev environment.