Content templates, part 3b: store exposed slot subtrees on individual entities

FYI this is inspired by what I proposed to @effulgentsia while discussing #3511366: [META] Introduce a `ContentTypeTemplate` config entity + related infrastructure, and which he encouraged to explore:

#3517845: Content templates, part 3a: allow content templates to define exposed slots is in!

There's still unaddressed feedback by @larowlan. I don't see nearly enough test coverage in here yet — there's many more possible edge cases.

Mostly applauding, but also some questions. A key part here should be reviewed by multiple people.

Couple of comments
- one about a constraint I think we're missing
- one about an edge case of a slot named '0'

Assigning to Wim for a final review, on balance I don't think splitting the field type is worth it unless we foresee future uses where it will make things easier. We could also always add additional testing, but I think what we've got here is enough to move forward with this feature.

Actually, NW for test failures first!

Just seen this, there some things that as I was typing seemed fine, however there are some that don't and I don't see discussion of these in this issue yet. I tried to answer my own questions as much as possible.

1. How will this handle re-ordering of slots?

Storing the slot as a column means that if a content template re-orders slots, this should be OK - the slot determines where things fit in, not the field delta. However it does mean that content, and not just content, individual revisions, will have different deltas corresponding to different slots depending on when it's created.

e.g.:

entity 1 | revision 1 | delta 0 | slot 1
entity1 | revision 1 | delta 1 | slot 2

entity 1 | revision 2 | delta 0 | slot 2
entity 1 | revision 2 | delta 1 | slot 1

If we always request data based on the slot and never the delta, then that's probably fine, even for revision restores and diffs, so it might just be an extra artefact in the database. However this goes onto the next questions:

2. If a content template removes a slot, what should happen?

Should there be validation that informs the site builder that content is going to be hidden/orphaned?

It's possible to add validation or messaging based on querying the field column but I'm not sure what that implies for either the UX or the storage.

3. If I want to delete all field data associated with a slot programmatically is there a way to do this?

I don't think there is because it's field data in a specific entity, it would require bulk updating entities via VBO or a custom update.

4. If a content template re-adds a slot with the same name after previously deleting it, what should happen?

e.g. if content was previously hidden, then should it re-appear? Or... should adding a slot have validation that no field data with the new slot name already exists and prevent it, or add a confirmation step?

--

I think all of these questions would be easier to answer if instead of a multiple cardinality field, this used multiple instances of the field.

e.g. answering the questions above with multiple field instances:

1. this is fine - can still be a column on a single-cardinality field, but no delta.

2. The field data would persist if nothing explicit is done, messaging would be the same except:

3. You could delete the field in this case to remove the data from the database, separately from step #2. This could use standard field deletion mechanisms, no need for custom bulk updates.

4. This problem does not really go way conceptually, but because it would be possible to delete the field, or not, there would be more options for handling it.

The downside to multiple field instances is that the UI for template building would need to create the new fields under the hood when slots are added, but it does feel like that's one problem to solve in one place, rather than multiple problems to solve across the lifecycle of an entity type.

I also tried to think how this would interact with #3477428: [PP-1] Refactor (or decide not to) the XB field type to be multi-valued, to de-jsonify the tree, and to reference the field_union type of the prop values and at first thought it was incompatible, but actually think it's about the same either way, just with more or less rows in the database - all the above questions would be equally valid/invalid too.

All of these questions were discussed as part of the design process but in fairness haven't been documented in the issue queue so I'm glad you're asking these questions. I don't know how close we're having all of these capabilities available so someone else will have to respond to this from that perspective.

1. How will this handle re-ordering of slots?

The slots should have an unique id. User should be able to change their placement in the template without it impacting the contents of that slot or other slots.

2. If a content template removes a slot, what should happen?

User should receive a warning that content inside the slot will be deleted. However, the content would not be deleted immediately; it would actually get deleted when the content entities with the slot contents get re-saved.

3. If I want to delete all field data associated with a slot programmatically is there a way to do this?

We discussed implementing garbage collection process running on cron. That would require having APIs for this.

4. If a content template re-adds a slot with the same name after previously deleting it, what should happen?

In this scenario, in case that the garbage collection had not happened yet, it would be fine to bring back the old slot content. But this would require the slot having the same unique id.

We discussed implementing garbage collection process running on cron. That would require having APIs for this.

This seems like a lot of potentially complex and expensive logic that would have to go through both entities and their revisions, loading and saving them, vs. being able to use the field deletion API which already exists if this was doing field-per-slot.

All of the other design considerations seem like they'd be either neutral or easier with field-per-slot to me.

All of these questions were discussed as part of the design process but in fairness haven't been documented in the issue queue so I'm glad you're asking these questions.

Thanks for the answers. However this issue has quite serious data integrity and (edit, see comment below) security implications, so - where did that discussion take place? And is it possible for someone who's not an Acquia employee or contractor to read it?

If not, how are people who are not Acquia employees or contractors expected to contribute to Experience Builder, both before it's stable, but also, more importantly, afterwards, when they have real sites with data integrity or security issues, and want to look through git history and find the reasons that things were implemented how they were in order to submit bug reports or work on fixes.

Further questions on top of #28.

User should receive a warning that content inside the slot will be deleted. However, the content would not be deleted immediately; it would actually get deleted when the content entities with the slot contents get re-saved.

I think there are several information disclosure risks with this approach:

Let's say a site deletes a slot from a template, and they are told that content will be deleted.

1. Can field data in the slot still be search indexed? #3483301: Create a configurable search api processor for XB data is planning to add an XB-specific search API integration, is that going to have to implement hiding of the data from search API separately?

2. What will prevent 'deleted' slots from being available in the field data to JSON:API / REST? Or conversely, it might be a problem if they aren't available in certain cases? I'm not sure at all what the behaviour should be there.

Additionally, and related to #2, having this as field deltas rather than fields, will make it impossible to implement field access for slots, and hence incompatible with modules like field permissions. Maybe Experience Builder is going to have a completely independent 'slot access' system but given it's using a field for storage, it feels like that decision should be explicit and documented, not an accidental byproduct of the implementation here.

It seems that we could benefit from an ADR for this because we are trying to weigh tradeoffs between two options. Should we do that here or should we open a separate issue for that?

+1 for ADR

There is currently no way for individual entities to provide the subtrees that should fill in those slots, to be assembled for final rendering. Let's add that in this issue.

I think we also have to allow for multiple view modes.

What happens if Full exposes 3 slots and Teaser exposes an entirely different slot.

Is delta + slot enough to distinguish - do we also need a view mode column?

What does the UI look like when editing a piece of content with multiple view mode templates? is there a view mode switcher? Do we prevent saving if a piece of content is not valid for a given view mode?

Adding #3477428: [PP-1] Refactor (or decide not to) the XB field type to be multi-valued, to de-jsonify the tree, and to reference the field_union type of the prop values as related

@larowlan in #21:

one about an edge case of a slot named '0'

Another reason to do #3519891: Constrain slot names allowed by XB in Components (and in its component tree)! 😄

I'm glad to see @lauriii already said as much in #29 😊

WRT deleting component subtrees (for this issue: of unused slots' subtrees): #3524406: [later phase] [Needs design] [PP-1] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up — but also of defunct components, which is necessary because of #3519168: Handle components provided by ComponentSources EXPLICITLY disappearing — enables deleting JS components that are in use.

@phenaproxima in #32:

the frontend has absolutely no concept of "this tree will fit into a slot".

1. That's intentional: docs/adr/0005-Keep-the-front-end-simple.md
2. That's for later: #3513563: [later phase] [META] Component slot restrictions ("which?") + limits ("how many?")
3. Yes, the client side will need a new "node type" for its client-side data model — see 3.4 UI Data Model: communicating a `component tree` to the front end in docs/data-model.md — currently only 3 node types exist: component, slot and region. I do agree a new one will be needed for this. That's what #3521002: [META] Maintainable client-side data model + internal HTTP API exists for.

P.S.: I'm very tempted to make "bonsai trees" an official term in XB, so can you elaborate on why you used that term and your rationale? 😄

@catch in #33:

RE: information disclosure risks Yep, I raised exactly that concern. 💯

The reason we have no choice but to allow this is that we know from the XB team members who've worked on https://github.com/acquia/cohesion (aka "Acquia Site Studio") for years that site builders will accidentally delete a slot, confirm through all the warnings, and then later realize that actually they do want to undo it after all.

RE: having this as field deltas rather than fields, will make it impossible to implement field access for slots Wow — glad you caught that! Had not crossed my mind yet!

@larowlan in #37: see the validation+config schema added in #3517845: Content templates, part 3a: allow content templates to define exposed slots — exposed slots are intentionally only allowed for the full/canonical view mode. Because otherwise you get different component trees per view mode, which would be an authoring/usability nightmare. I first identified this in #3511366-3: [META] Introduce a `ContentTypeTemplate` config entity + related infrastructure, @lauriii responded at #3511366-7: [META] Introduce a `ContentTypeTemplate` config entity + related infrastructure.2.

Apologies, @phenaproxima, for having sent you down this path 😬 Turns out that this "incremental step" was fraught with more challenges than I anticipated. I expected it to need refactoring later on for sure, but I tried to unblock you on next steps for content templates. Turns out that was not quite realistic.

Per discussion with @effulgentsia, we think this is best tabled until after #3468272: Store the ComponentTreeStructure field property one row per component instance. @larowlan is actively working on spikes to help us make the best choice possible.

Agreed that an ADR is needed and appropriate, but during this read-through/catch-up, I did not end up with the impression there's only 2 possible choices as @lauriii indicates in #35 — I think there's many possible choices.

#39: thanks for that lovely imagery :) I think we should make that an official term! 😄 Issue created: #3525480: Coin 2 new terms that are both precise and fun!.

As described at #3468272-34: Store the ComponentTreeStructure field property one row per component instance, I believe that MR will allow us to close this MR. Still, in case that's not true, I want to explicitly acknowledge that this should block #3515932: Milestone 1.0.0-beta1: Enable creation of non-throwaway sites, because without this supported, the data storage/model may need to change significantly later, which would be very disruptive.

Reviewing #3468272: Store the ComponentTreeStructure field property one row per component instance right now, it 100% certainly addresses this! 🥳

All that this issue will need to do is refine the ComponentTreeStructureConstraintValidator validation constraint, for which @larowlan even left a very nice @todo in the right place:

    // @todo For 'bonsai' trees, the parent may not be in the corresponding
    //   parent list but in a ContentTemplate for the parent entity. Add UUIDs
    //   from any relevant ContentTemplates for this entity -
    //   https://drupal.org/i/3519352

Hence removing the beta blocker tag, because this won't require data storage changes anymore; it will only require refining validation logic (essentially to allow more: allow a component instance to refer to a parent component instance that lives outside its own component tree). 🥳

All that this issue will need to do is refine the ComponentTreeStructureConstraintValidator validation constraint, for which @larowlan even left a very nice @todo in the right place:

Is that definitely all this issue needs to do if there needs to be data cleanup and access/rendering changes when exposed slot subtrees are added or removed?

#42: for that, we have #3524406: [later phase] [Needs design] [PP-1] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up.

@wim leers that issue wouldn't be necessary if the exposed subtrees were stored as individual fields as opposed to within one field, which I assumed was something to sort out here?

that issue wouldn't be necessary if the exposed subtrees were stored as individual fields as opposed to within one field, which I assumed was something to sort out here?

The downside of this approach would be that we'd have to read from multiple table to construct the merged tree. It would also add overhead for every exposed slot in the template (in a linear fashion).

I'm not saying its a bad idea, I think it warrants a spike so we have points of comparison for both approaches. I'll open an issue for that.

One benefit it would provide is that it might solve #37 (note that this isn't in the product requirements, which limit exposed slots to the full view mode).

When I was talking about cleanup with Alex we wondered whether we could make use of the deleted column in the default dedicated field storage schema for flagging slots that no longer existed.

This could probably use a consolidated comparison in an updated issue summary.

After reading through this I'm strongly in favor of one field per tree.

The benefits seem to be it matches closer to expected drupal norms, let's you order them, organize them by display and leverage other apps making it better for outside devs trying to interact. Nevermind the security and access issues mentioned.

The only downside of per field seems to be more complex queries (which can be cached right?) And a concern that users will delete them unexpectedly.

We have the trash module, I wonder if we could probably leverage a place holder that delays that deletion temporarily or something to solve that issue rather than an architectural decision that compromises security and makes other standard use cases far harder.

Opened #3526189: Spike: explore merits of one Canvas field per exposed slot in content templates for #44

Reanswering the questions from comment #28 now that #3468272: Store the ComponentTreeStructure field property one row per component instance is in

1. How will this handle re-ordering of slots?

It doesn't matter anymore - yes we have a delta in the field values (because of unlimited cardinality) but it is only important for elements that are siblings in the tree - ie. have the same value for parent_uuid and slot. So re-ordering a whole slot is transparent. Its only ordering within a slot that is important.

2. If a content template removes a slot, what should happen?

Now that slot is a column, we can use an entity query to detect any content in this slot and warn. In theory we could also set the 'deleted' column to 1 on those rows to avoid the overhead of loading and manipulating them in memory/queries if they do nothing. Or failing that we could have something similar to field_purge (but driven by queues) that cleaned up these rows during cron runs.

3. If I want to delete all field data associated with a slot programmatically is there a way to do this?

Yeah still would need to use an entity query to find items with a matching slot now that slot is a column of its own. Then load those items and call $entity->get('component_tree')->setValue(\array_filter($entity->get('component_tree')->getValue(), static fn (array $item) => $item['slot'] === $dead_slot))->save()

So that's back to bulk updating entities - so having a queue purge process to do this automatically on cron would probably be best, but yeah VBO is another option.

4. If a content template re-adds a slot with the same name after previously deleting it, what should happen?

This is another argument for trying to reuse the deleted column, because we could just toggle that back to 0 for any remaining data and the old values would re-appear. But yeah, needs input from @lauriii about the intended behaviour in this case

To further validate #3468272: Store the ComponentTreeStructure field property one row per component instance I merged this with 0.x and updated the `::mergeSubTrees` logic

Before

  public function mergeSubTrees(array $exposed_slot_info, iterable $subtrees): self {
    $subtrees = array_intersect_key(
      self::keySubtreesBySlot($subtrees),
      $exposed_slot_info,
    );

    $our_tree = Json::decode($this->get('tree')->getValue());
    $our_inputs = Json::decode($this->get('inputs')->getValue());

    foreach ($subtrees as $slot_name => $subtree) {
      assert($subtree instanceof self);
      $component_uuid = $exposed_slot_info[$slot_name]['component_uuid'] ?? NULL;
      $component_slot_name = $exposed_slot_info[$slot_name]['slot_name'] ?? NULL;

      if (empty($component_uuid)) {
        throw new SubtreeMergeException("Cannot merge subtree because we don't know the UUID of the component instance to target.");
      }
      if (is_null($component_slot_name)) {
        throw new SubtreeMergeException("Cannot merge subtree because we don't know the name of the component slot to target.");
      }

      $subtree_tree = Json::decode($subtree->get('tree')->getValue());

      if (!array_key_exists(ComponentTreeStructure::ROOT_UUID, $subtree_tree)) {
        throw new SubtreeMergeException("Cannot merge subtree because the subtree has no root UUID.");
      }

      // The target slot needs to be empty.
      if (!empty($our_tree[$component_uuid][$component_slot_name])) {
        throw new SubtreeMergeException("Cannot merge subtree because the targeted slot is not empty.");
      }

      // The stuff at the root of the subtree goes directly into the exposed
      // slot in our tree. Config schema validation guarantees that the slot is
      // empty, so we're not going to be overwriting anything.
      $our_tree[$component_uuid][$component_slot_name] = $subtree_tree[ComponentTreeStructure::ROOT_UUID];

      // Everything else in the subtree is just added to our tree as-is.
      unset($subtree_tree[ComponentTreeStructure::ROOT_UUID]);
      if (array_intersect_key($subtree_tree, $our_tree)) {
        throw new SubtreeMergeException("Cannot merge subtree because some of its components are already in the final tree.");
      }
      $our_tree += $subtree_tree;

      // Add the subtree's inputs to the final set of inputs.
      $subtree_inputs = Json::decode($subtree->get('inputs')->getValue());
      // If the final tree already has any of these inputs, that's a problem
      // because we can't be sure which ones we should actually use.
      if (array_intersect_key($subtree_inputs, $our_inputs)) {
        throw new SubtreeMergeException("Cannot merge subtree because some of its inputs are already in the final tree.");
      }
      $our_inputs += $subtree_inputs;
    }

    $our_tree = Json::encode($our_tree);
    $our_inputs = Json::encode($our_inputs);

    $this->get('tree')->setValue($our_tree);
    $this->get('inputs')->setValue($our_inputs);

    return $this;
  }

  private static function keySubtreesBySlot(iterable $subtrees): array {
    $keyed = [];
    foreach ($subtrees as $subtree) {
      assert($subtree instanceof self);
      $slot = $subtree->get('slot')->getValue();
      $keyed[$slot] = $subtree;
    }
    return $keyed;
  }

After

public function mergeSubTreeItemList(array $exposed_slot_info, ComponentTreeItemList $subTreeItemList): self {
    foreach ($exposed_slot_info as $slot_detail) {
      $parent_uuid = $slot_detail['component_uuid'];
      $slot = $slot_detail['slot_name'];
      if ($parent_uuid === NULL) {
        throw new SubtreeMergeException("Cannot merge subtree because we don't know the UUID of the component instance to target.");
      }
      if ($slot === NULL) {
        throw new SubtreeMergeException("Cannot merge subtree because we don't know the name of the component slot to target.");
      }
      $existing = \count(\iterator_to_array($this->componentTreeItemsIterator(self::isChildOfComponentTreeItemSlot($parent_uuid, $slot))));
      // The target slot needs to be empty.
      if ($existing !== 0) {
        throw new SubtreeMergeException("Cannot merge subtree because the targeted slot is not empty.");
      }
      foreach ($subTreeItemList->componentTreeItemsIterator(self::isChildOfComponentTreeItemSlot($parent_uuid, $slot)) as $item) {
        \assert($item instanceof ComponentTreeItem);
        if ($this->getComponentTreeItemByUuid($item->getUuid()) !== NULL) {
          throw new SubtreeMergeException("Cannot merge subtree because some of its components are already in the final tree.");
        }
        $this->appendItem($item->getValue());
      }
    }
    return $this;
  }

That feels like a pretty big endorsement to me. 80% of the new code is validation, the rest is ->appenditem

This is green on top of the new data storage model

It doesn't matter anymore - yes we have a delta in the field values (because of unlimited cardinality) but it is only important for elements that are siblings in the tree - ie. have the same value for parent_uuid and slot. So re-ordering a whole slot is transparent. Its only ordering within a slot that is important.

In one of the early conversations with @lauriii he was very concerned that the data model should reflect how things appear on the page (e.g. ordering not getting out of sync). I get the principle that only ordering within a slot would actually affect the rendering though.

Now that slot is a column, we can use an entity query to detect any content in this slot and warn. In theory we could also set the 'deleted' column to 1 on those rows to avoid the overhead of loading and manipulating them in memory/queries if they do nothing. Or failing that we could have something similar to field_purge (but driven by queues) that cleaned up these rows during cron runs.

Using the deleted column within a field is an interesting idea. I personally don't think loading and saving every revision of every entity with a deleted slot should be considered for data cleanup because it could be a huge undertaking with lots of content, there can be other problems loading and saving very old revisions with different data integrity problems etc.

So either actually using field purge (because field-per-slot), or expanding 'field delta purge by component' would avoid that. Although field delta purge by component obviously doesn't exist as a concept so it might not be a small undertaking to implement.

So that's back to bulk updating entities - so having a queue purge process to do this automatically on cron would probably be best

Per above I think that's something to avoid, it could be millions of entity/revision saves and crash due to other issues.

In one of the early conversations with @lauriii he was very concerned that the data model should reflect how things appear on the page (e.g. ordering not getting out of sync). I get the principle that only ordering within a slot would actually affect the rendering though.

This is probably one of the conversations at dev days but I have no recollection whatsoever on what the specific concern was. What's important is that components within a slot are ordered correctly. I don't have strong opinions on whether that's done with field delta or not.

What's also important is that there's a simple way to construct the tree of components because we may have decoupled clients like Lupus building alternate rendering engines for the data model.

If I'm understanding correctly, a content template could have 12 exposed slots. Which means 12 fields will be programmatically attached to that bundle, and all the implications of dedicated field storage. Right?

@mglaman yes exactly, if there are a lot of exposed slots, then field-per-slot means a lot of fields - but also 12 fields on an entity is not that many in the scheme of things, I regularly see bundles with 20-30 configurable fields. It's also nothing like paragraphs where you could have three different tables or more per-paragraph (equivalent to component instance/field delta).

So if it does simplify data clean-up when slots are unexposed (no loading and saving unbounded numbers of entities), and potentially other things like access/hiding of slots (because field access becomes available), then I think it would be a good trade off. If it's not actually a simplification, then it wouldn't be worth doing for its own sake, but at the moment we're talking about having to do #3524406: [later phase] [Needs design] [PP-1] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up vs probably not having to do it at all.

But also 12 different exposed slots would imply non-exposed slots in between, I can't see why there would be adjacent exposed slots, that could just be one slot. So we're talking about well over 20 distinct slots at that point? To me this seems like it would be an extreme upper bound, and 1-3 is more likely unless I'm missing something with the use case.

Bumping this to beta blocker because of the security concerns raised by @catch. It's important that we land on how to handle the storage needs of content templates before #3515932: Milestone 1.0.0-beta1: Enable creation of non-throwaway sites, even if the end-user facing functionality does not exist yet.

Discussion

@catch in #44: no, #3524406: [later phase] [Needs design] [PP-1] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up will be necessary even if we do that. Check it out — there's other ways to end up with dangling subtrees 😬

@larowlan in #45: use of the deleted column in the default dedicated field storage schema for flagging slots that no longer existed. 👏 Yes!

@larowlan in #48:

3. This is the part that this issue could make simple 👍 That would keep the scope of #3524406: [later phase] [Needs design] [PP-1] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up smaller, but #3524406 would still be necessary.
4. 💯 — that's why I was excited about that insight by you and @effulgentsia! 😄

@catch in #51:
Where you wrote

So either actually using field purge (because field-per-slot), or expanding 'field delta purge by component' would avoid that. Although field delta purge by component obviously doesn't exist as a concept so it might not be a small undertaking to implement.

I do understand field purge (because field-per-slot).

But I don't understand the other. We'd have to purge all component instances within an exposed slot that was deleted. So it'd be basically:

1. UPDATE {field_table} SET deleted=1 WHERE parent_uuid IS NULL AND slot LIKE 'my_deleted_exposed_slot'; for the content entity type+bundle whose ContentTemplate had an exposed slot deleted
2. the same for the {field_revision_table} revision table
3. … then gradually purging just those rows, not all fields. The challenge there is AFAICT the need to call \Drupal\file\Plugin\Field\FieldType\FileFieldItemList::deleteRevision() and friends.

@catch in #54:

But also 12 different exposed slots would imply non-exposed slots in between, I can't see why there would be adjacent exposed slots, that could just be one slot. So we're talking about well over 20 distinct slots at that point? To me this seems like it would be an extreme upper bound, and 1-3 is more likely unless I'm missing something with the use case.

How do you go from "12 exposed slots" to "20 distinct slots"? 🤔

MR review + issue/MR scope

👏 @larowlan for #49 and #50 🤩

The issue summary is now wildly out-of-date, given the new #3468272: Store the ComponentTreeStructure field property one row per component instance-powered reality, so let's fix that.

The current data model (specifically the allowed values for the parent_uuid and slot field properties in \Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::propertyDefinitions()) clearly are sufficient to make this functionality work — because the post-3468272-updated MR is passing tests.

The current MR chooses to not store one field per exposed slot. If we were to do one-field-per-subtree-for-an-exposed-slot, we'd need to do the following on top of this MR:

1. Add a exposed_slot storage setting to ComponentTreeItem.
2. Add a validation constraint that enforces that storage setting: it must disallow both parent_uuid and slot being NULL, because those would be component instances targeting the root of a component tree. (Which can only live in the ContentTemplate config entity for this content entity type + bundle!)
3. Update \Drupal\experience_builder\Entity\ContentTemplate::build() and the ComponentTreeLoader service not load the sole XB field, but all of the XB fields on the given content entity type + bundle, and call ::mergeSubTreeItemList() on all of them.
4. Tests

I don't see any reason why we cannot do that in a follow-up, nor do I think that that introduces any additional risk at this time.

That would allow @phenaproxima to get started on next steps (such as #3518248: [PP-1] Content templates, part 4 (boss battle): create a UI for editing templates).

Thoughts? 😊

Update issue summary, much less to talk about now because this is dramatically simpler since #3468272: Store the ComponentTreeStructure field property one row per component instance

I think we should move the multiple fields idea to a follow up spike. The main benefit is to allow identification and removal of dangling sub-trees but I think the cost (in terms of complexity) is too high. Implementing a purge technique (either by reusing deleted column from core or something custom) feels like a more long term approach in terms of managing complexity. We could do a spike of both (multiple fields vs purge approaches) to decided which is the better approach.

Agree with getting this in and unblocking the UI elements for the FE team.

@catch in #44: no, #3524406: [later phase] [Needs design] [PP-1] Provide API for finding and UI for surfacing dangling/dead component subtrees — aka garbage clean-up will be necessary even if we do that. Check it out — there's other ways to end up with dangling subtrees 😬

hmm, that issue seems like it's dealing with a data-integrity bug as a result of code going AWOL (similar to Views invalid plugin handling). For me that is very different to being forced to deal with a data integrity issue because of the way that fully valid and allowed configuration changes are reflected in the schema.

But I don't understand the other. We'd have to purge all component instances within an exposed slot that was deleted. So it'd be basically:

UPDATE {field_table} SET deleted=1 WHERE parent_uuid IS NULL AND slot LIKE 'my_deleted_exposed_slot'; for the content entity type+bundle whose ContentTemplate had an exposed slot deleted
the same for the {field_revision_table} revision table
… then gradually purging just those rows, not all fields. The challenge there is AFAICT the need to call \Drupal\file\Plugin\Field\FieldType\FileFieldItemList::deleteRevision() and friends.

This is about as far as I got trying to think what purging only components would look like, or well, it's a lot further than I got.

How do you go from "12 exposed slots" to "20 distinct slots"?

Hmm maybe I am missing something.

My understanding of #3517845: Content templates, part 3a: allow content templates to define exposed slots was that it allows for the following use case:

1. Parts of the template where the content author can add different components of their choosing.
2. Parts of the template which are 'fixed' and will be filled in via field content, or hard-coded to a specific block or something.

and that there could be multiple of #1 and #2.

If that is correct, then I would assume that when you have #2, you also have #1, hence going from 12 to 20, or at least, that you would have 12 exposed slots and 0 non-exposed slots. But do I have the wrong end of the stick?

But either way, the point is that I can't see people adding more than 12 of them, it seems like the sort of thing you'd add 1 or 2 of at most? And if so, then field-per-slot is a couple of fields, not dozens.

I commented in #3526189-3: Spike: explore merits of one Canvas field per exposed slot in content templates with a concern about the "different field for each exposed slot" idea.

@larowlan Thanks for #56 Thanks for the issue summary update!

@catch in #57: RE 12 vs 20: There could easily be 100 slots in a given component tree. Most of them would indeed be "fixed" in the ContentTemplate: the site builder populates them as they see fit. The ones that are exposed by the site builder to content creators for them to populate are required to be empty (see ValidExposedSlotConstraintValidator). So, yes, 12 exposed slots already seems pretty much the upper bound in real-world terms. (In fact, we could even limit it — for either or both content author UX and performance/scalability reasons!)

So: AFAICT you were just talking about how many actual component instance slots exist in total in a content template ("distinct", 20) vs which ones are exposed (12). 👍

I see that both @larowlan in #56 and @effulgentsia in #58 basically agreed with what I wrote in #55, so tightening scope, and landing this first.

So: AFAICT you were just talking about how many actual component instance slots exist in total in a content template ("distinct", 20) vs which ones are exposed (12).

Yes or more specifically, I couldn't think of a use case for consecutive exposed slots, so am assuming there'd always be at least one 'fixed' slot in-between. But the main point is that 12 would be at the top end, not an average.

👍Agreed on both counts!

Although, actually … I can think of a use case for consecutive exposed slots. Imagine you always want 2 columns. But you want content authors to populate them freely. That’s 2 slots in the same component instance that you’re both exposing.

I lost track of something when reviewing+merging this 😬 Created stable-blocking follow-up: #3528458: Component instances in exposed `ContentTemplate` slots must use the exposed slot's machine name.