- Issue created by @larowlan
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
have each source plugin implement it
I'm confused by this. Block components never reference other entities? An SDC can never reference content entities?
It's not the component itself that can reference other (content) entities, but only the inputs for the component.
So then what you need is to call
\Drupal\experience_builder\Plugin\DataType\ComponentInputs::getPropSources()
and add some logic around that to filter down to onlycontent
dependencies?IOW: a sibling to
\Drupal\experience_builder\Plugin\DataType\ComponentInputs::getPropSourcesWithDependency()
(which was added in π Component trees with dynamic prop sources that introspect field values don't add the relevant fields to their dependencies Active ), to get just the content dependencies?IOW β¦
\Drupal\experience_builder\Plugin\DataType\ComponentInputs::calculateDependencies()
, but filtered down tocontent
dependencies? Which is already tested by\Drupal\Tests\experience_builder\Kernel\DataType\ComponentInputsDependenciesTest::testCalculateDependencies()
:// Verify content dependencies if we have a valid entity. $file_entity = $hero_reference->get('field_media_image')->entity; assert($file_entity instanceof File); $file_uuid = $file_entity->get('uuid')->value; self::assertSame([ 'module' => [ βββ ], 'config' => [ βββ ], 'content' => [ 'file:file:' . $file_uuid, ], ], $item_list->get(3)->get('inputs')->calculateDependencies($node));
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
I know @phenaproxima asked about this in chat, that's why @larowlan opened this.
So let's get him unblocked by providing a test, should be simple :)
- Merge request !1244Resolve #3534561 "Test coverage get content deps of component instance" β (Merged) created by wim leers
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Test coverage pushed.
I used
\Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::calculateFieldItemValueDependencies()
to also allow for the case of anything else besides the explicit inputs of a component instance (theinputs
property) to declarecontent
dependencies.(I could have used
$item->get('inputs')->calculateDependencies($node)['content'])
instead of$item->calculateFieldItemValueDependencies($node)['content']
, but this just seemed more prudent because truly all-encompassing all dependencies of the component instance/ComponentTreeItem
.)And in doing so β¦ this revealed a bug in
\Drupal\experience_builder\Plugin\DataType\ComponentInputs
: it should've been implementingContentAwareDependentInterface
, notDependentPluginInterface
, because that is the difference betweencalculateFieldItemValueDependencies()
executing either of these 2 branches:if ($property instanceof DependentPluginInterface) { $dependencies = NestedArray::mergeDeep($dependencies, $property->calculateDependencies()); } elseif ($property instanceof ContentAwareDependentInterface) { $dependencies = NestedArray::mergeDeep($dependencies, $property->calculateDependencies($host_entity)); }
π
Yay for tests exercising what we say/think should work. Here's hoping there's no additional bug discoveriesβ¦
- πΊπΈUnited States phenaproxima Massachusetts
For the record, this blocks site templates. Or at least, site templates that ship XB-ified content (which is likely to be all site templates).
The reason this blocks site templates is because default content (including
xb_page
entities) needs to be able to calculate its dependencies. Without a method to get the referenced entities in an XB component tree, the default content exporter will have no way to do that. - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
-
[β¦] would indeed be most helpful.
I'm sure that would be for that particular use case. But it's a single use case. And this works, doesn't it? So are you really not unblocked?
- I bet that ideally, you'd actually be able to just call
\Drupal\Core\Entity\ContentEntityBase::referencedEntities()
and that'd automatically take care of XB fields' reliance on other entities?And that's actually totally doable too, we'd just add a new
references
computed property to\Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem::propertyDefinitions()
. But that's not something we should A) rush, B) delay beta for.
So can you please confirm that this MR (point 1) would unblock you for now? Then I'd be happy to merge it right away, and we can keep this issue open (to implement point 2). ππ
-
- πΊπΈUnited States phenaproxima Massachusetts
I have not tested this (I need that damn default content exporter in core first!) but here's my inference from reading the code in HEAD and the MR...
If this results in content dependencies being added to the field item like
node:page:SOME_UUID
, that will be enough data to give the exporter what it needs. This appears to be confirmed by the test coverage.So in the interest of acceleration, I'm calling this one ready.
-
wim leers β
committed 2abc7d73 on 0.x
Issue #3534561 by wim leers, phenaproxima: Add test coverage...
-
wim leers β
committed 2abc7d73 on 0.x
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
Still needs clarifications on what exactly is needed though β probably best after is done? π
- πΊπΈUnited States phenaproxima Massachusetts
I don't think the component tree field type should need to provide another property at all.
The content exporter is currently leaning towards allowing modules to supply callback functions that know how to handle specific field/data types. So in fact, XB would have an event subscriber like this pseudocode:
$event->setCallback('component_tree', function (ComponentTreeItem $item, ExportMetadata $metadata): array { foreach ($item->referencedEntities() as $referenced) { $metadata->addDependency($referenced); } });
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
The content exporter is currently leaning towards
π
Looked through your post history and found β¨ Add a command-line utility to export content in YAML format Active , which seems to be what this is for?