Empty slot placeholder not rendering for content that has been published

It seems that something in the act of publishing is losing data from the JSON

Before publishing, the layout JSON for the One Column component (returned from http://xb.test/xb/api/layout/xb_page/2:GET) looks like:

[
  {
    "nodeType": "component",
    "type": "sdc.experience_builder.one_column",
    "uuid": "177122af-1679-4ee4-b700-dcf5ab376c4a"
    "slots": [
      {
        "id": "177122af-1679-4ee4-b700-dcf5ab376c4a/content",
        "name": "content",
        "nodeType": "slot",
        "components": []
      }
    ],
  }
]

after publishing, the slots[] array is empty:

[
  {
    "nodeType": "component",
    "uuid": "177122af-1679-4ee4-b700-dcf5ab376c4a",
    "type": "sdc.experience_builder.one_column",
    "slots": []
  }
]

Hm, there is likely something different between
\Drupal\experience_builder\Controller\ApiLayoutController::buildLayout()'s

      if (isset($full_tree[$component_instance_uuid])) {
        foreach ($full_tree[$component_instance_uuid] as $slot_name => $slot_children) {
          $component_instance['slots'][] = [
            'nodeType' => 'slot',
            'id' => $component_instance_uuid . '/' . $slot_name,
            'name' => $slot_name,
            'components' => $this->buildLayout($model, $item, $slot_children),
          ];
        }
      }

and \Drupal\experience_builder\Controller\ApiConfigControllers::buildLayoutAndModel()'s

      if (isset($full_tree[$component_instance_uuid])) {
        foreach ($full_tree[$component_instance_uuid] as $slot_name => $slot_children) {
          $component_instance_slot = [
            'id' => $component_instance_uuid . '/' . $slot_name,
            'name' => $slot_name,
            'nodeType' => 'slot',
            'components' => [],
          ];
          self::buildLayoutAndModel($component_instance_slot['components'], $model, $item, $slot_children);
          $component_instance['slots'][] = $component_instance_slot;
        }
      }

I bet the first of those two is wrong.

It's not clear to me at this time why such similar pieces of code exist in two places. Except … demo-driven development 😅 That's why there's this:

* @todo Follow up issue to extract this logic into a trait: https://www.drupal.org/project/experience_builder/issues/3499632

Seems like this is the time to make that consistent.

Debugging revealed that the "before publishing" JSON structure from #2 is what the client sends to the server.

But … the server side implementation literally forbids this:

new Count(['min' => 1], minMessage: 'Empty component subtree. A component subtree must contain >=1 populated slot (with >=1 component instance). Empty component subtrees must be omitted.'),

— \Drupal\experience_builder\Plugin\Validation\Constraint\ComponentTreeStructureConstraintValidator

Because it's unnecessary information to store.

I checked docs/data-model.md#3.4.1 `component node`s to see what the client claims to expect, and found:

`component node`s have the following keys
…
- `slots`: an object of `slot node`s if any of the `component slot`s of this `component instance` are populated

So isn't this is what we agreed upon? 😅

IOW, I'd expect that slot node to be generated by the client side based on the metadata in /xb/api/config/component:

  "sdc.experience_builder.one_column": {
    "source": "Module component",
    "metadata": {
      "slots": {
        "content": {
          "title": "Content",
          "description": "The contents of the column."
        }
      }
    },

… if not, the client-side data model docs should be changed to something like:

--- a/docs/data-model.md
+++ b/docs/data-model.md
-- `slots`: an object of `slot node`s if any of the `component slot`s of this `component instance` are populated
+- `slots`: an object of `slot node`s if any of the `component slot`s of this `component instance` have any slots, *even*
+  if those slots are not populated by any `component instance`s

It does look like the docs are slightly incorrect in this case.

I think it should read

- `slots`: an object of `slot node`s representing each `component slot` of this `component instance`

If the slots are not returned in the JSON layout data then it would be pretty arduous on the front end to have to recurse through every component instance in the layout and reference the component from the main component list to check if it should have slots and recreate them in the JSON.

WFM! Thanks!

Fix up. Looking into:

1. test coverage
2. * @todo Follow up issue to extract this logic into a trait: https://www.drupal.org/project/experience_builder/issues/3499632

If the slots are not returned in the JSON layout data then it would be pretty arduous on the front end to have to recurse through every component instance in the layout and reference the component from the main component list to check if it should have slots and recreate them in the JSON.

On second thought, I don't think this statement is actually as true as it sounded at first reading 😅

This is a simple tree traversal, that must happen either on the client or on the server. It could happen on the client … which would mean less data on the wire.

But, for the sake of convenience, and avoiding premature optimization: just running with this for now 👍

Let's not start doing 📌 ServerClientConversionTrait Active here and keep the issue tightly scoped — but IMHO we should do 📌 ServerClientConversionTrait Active sooner rather than later!

code looks good. I also manually tested with and without the fix. I saw the problem and the it is fixed!

the phpunit tests for some memory issue, so rerunning

Ah, some stupid number incrementing in test expectations needed 😄 Can you handle that?

Yep. will look into the test fail

Looks good!

Thanks! Final confirmation by @jessebaker. 🙏

Thanks for the confirmation, @jessebaker!

given we duplicate this logic between here and the config controller - does this belong on the tree item instead? thin controllers etc

See #13. I agree we need to refactor this duplication away. But doing it in this issue expands scope too much. Issue for fixing that: 📌 ServerClientConversionTrait Active .

Automatically closed - issue fixed for 2 weeks with no activity.