Problem/Motivation

When updating on last dev, error occur:

Error: Call to a member function setNewPresent() on null in Drupa l\display_builder_entity_view\Entity\EntityViewDisplay->postSave() (line 18 2 of /checkout/web/modules/contrib/display_builder/modules/display_builder_entity_view/src/Entity/EntityViewDisplayTrait.php)

Steps to reproduce

Proposed resolution

Here is a patch proposition by @yannickoo

public function postSave(EntityStorageInterface $storage, $update = TRUE): void {
    // Reset the state once you import a configuration.
-    if ($this->isSyncing()) {
-      $instance = $this->getInstance();
+   if ($this->isSyncing() && ($instance = $this->getInstance())) {

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork display_builder-3603094

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
Assigned: Unassigned » pdureau

mogtofu33 opened merge request !294

pdureau’s picture

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

OK for me.

pdureau’s picture

pdureau commented
Title: Update error » Skip config import if no instance entity
pdureau’s picture

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

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Reviewed & tested by the community » 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.

pdureau’s picture

pdureau commented
Assigned: Unassigned » pdureau
Status: Fixed » Needs work

I see it was not merged yet. Maybe because of fails in phpunit. So back to "Needs work".

This fail was expected, and is related to changes in UI Patterns 2.0.16 released yesterday:

[
     0 => Array &1 [
+        'node_id' => '1',
         'source_id' => 'component',
         'source' => Array &2 [],
         'third_party_settings' => Array &3 [],
-        'node_id' => '1',
     ],
 ]

We just need to update the order of properties in the tests.

There are also 4 "Failed asserting that an array is empty." fails, because an empty array inside an empty array is not considered empty by PHP:

(
    [0] => Array (
            [node_id] => 
            [source_id] => 
            [source] => Array()
            [third_party_settings] => Array ()
        )
)

Investigating.

mogtofu33’s picture

mogtofu33 commented

I checked to fix the tests, but so we have now this root array with 'node_id' but source_id = NULL.

pdureau’s picture

pdureau commented
Assigned: pdureau » mogtofu33

3 commits pushed on the MR, I guess this ticket is on your side.

Are you sure those changes in EntityViewDisplayTrait, API controllers, Instance entity and SourceTree are needed just because we have updated UI Patterns from 2.0.15 to 2.0.16?

mogtofu33’s picture

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

EntityViewDisplayTrait is the original bug fix.
There is some inconsistency changes unrelated, like save() usage. But mostly it's tests fix and workaround this source_id => NULL from ui_patterns.

But would be great to know if it's a problem from ui_patterns.

Seems like ui_patterns SourceValueItem inherit FieldItemBase inherit Map for function isEmpty() wich check null when we are empty array. Then FieldItemList::filterEmptyItems() could remove the phantom stuff as currently it didn't.

mogtofu33’s picture

mogtofu33 commented
Related issues: +#3604317: SourceValueItem missing isEmpty() causes phantom null items to survive filterEmptyItems() [AI assisted]

I removed the workaround and created an issue on ui_patterns https://www.drupal.org/project/ui_patterns/issues/3604317

pdureau’s picture

pdureau commented
Status: Needs review » Needs work
But would be great to know if it's a problem from ui_patterns.

I was investigating that this afternoon. Argument #1 ($source_id) must be of type string, null given, called in SourceTree.php is caused by this (relevant) change in UI Patterns:

-class SourceValueItem extends MapItem {
+class SourceValueItem extends FieldItemBase {

Because we are losing the (maybe too) generous MapItem::isEmpty()

  public function isEmpty() {
    return empty($this->values);
  }

and relying on the more strict and convoluted Map::isEmpty().

So, with those 3 additional lines in UI Patterns + reordering the properties in InstancePublishingTest::testRestore() (2 lines) must be enough to fix, without the risk of changing our tests, the delicate SourceTree or the publishing logic.

I take the ticket back to finish the investigation and maybe to propose another MR (or two 😉).

pdureau’s picture

pdureau commented

I don't understand when I am supposed to work on this ticket or not 😕 it is the second time today commits are pushed while the ticket is assigned to me.

pdureau opened merge request !296

pdureau’s picture

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

Here is my proposal: https://git.drupalcode.org/project/display_builder/-/merge_requests/296/...

UI Patterns 2.0.16 compatibility OK with only 2 lines changed in InstancePublishingTest::testRestore() & 3 lines added in #3604317: SourceValueItem missing isEmpty() causes phantom null items to survive filterEmptyItems() [AI assisted] (see my comment).

Without the risk of doing changes in the tests, the SourceTree, the controllers... parts of the codebase which are delicate and tricky.

What do you think?

Edit: I forgot to update composer.json

pdureau’s picture

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

I will add the composer.json and also DisplayBuilderKernelTestBase::testGetParentId() from your MR because it add coverage without expecting a change of the existing logic 😉

pdureau’s picture

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

Done.