Problem/Motivation
Instance ID in State Manager (Display Builder instances, not the sources instances, we will clarify this confusing vocabulary in an other issues) are supposed to be opaques except for the class implementing WithDisplayBuilderInterface().
However we are checking them:
$ grep -r "str_starts_with.*__" modules/
modules/display_builder_devel/src/Form/EditForm.php: if (\str_starts_with($builder_id, 'page_layout__') || \str_starts_with($builder_id, 'view__') || \str_starts_with($builder_id, 'entity_view__')) {
modules/display_builder_page_layout/src/Plugin/DisplayVariant/DisplayBuilderPageVariant.php: if (\str_starts_with($renderable_id, 'page__') || \str_starts_with($renderable_id, 'region__')) {
modules/display_builder_entity_view/src/EventSubscriber/DisplayBuilderSubscriber.php: if (\str_starts_with($builder_id, 'entity_view_override__')) {
modules/display_builder_entity_view/src/EventSubscriber/DisplayBuilderSubscriber.php: elseif (\str_starts_with($builder_id, 'entity_view__')) {
modules/display_builder_views/src/Controller/ViewsManagementController.php: if (!\str_starts_with($builder_id, 'view__')) {
And manipulating them:
$ grep -r "explode..__" modules/
modules/display_builder_devel/src/Form/ImportForm.php: $display_id = \explode('__', $builder_id)[2];
modules/display_builder_page_layout/src/Entity/PageLayout.php: $page_layout = \explode('__', $instance_id)[1];
modules/display_builder_entity_view/src/Field/DisplayBuilderItemList.php: [, $entity_type_id, $entity_id, $field_name] = \explode('__', $instance_id);
modules/display_builder_entity_view/src/EventSubscriber/DisplayBuilderSubscriber.php: [, $entity_type_id, $entity_id, $field_name] = \explode('__', $builder_id);
modules/display_builder_entity_view/src/Entity/EntityViewDisplayTrait.php: [, $entity, $bundle, $view_mode] = \explode('__', $instance_id);
modules/display_builder_views/src/Form/ConfirmViewsBuilderDeleteForm.php: $view_id = \explode('__', $this->builderId)[1];
modules/display_builder_views/src/Form/ConfirmViewsBuilderDeleteForm.php: $display_id = \explode('__', $this->builderId)[2];
modules/display_builder_views/src/Plugin/views/display_extender/DisplayExtender.php: $view = \explode('__', $instance_id)[1];
modules/display_builder_views/src/Plugin/views/display_extender/DisplayExtender.php: $display = \explode('__', $instance_id)[2];
modules/display_builder_views/src/EventSubscriber/DisplayBuilderSubscriber.php: $display_id = \explode('__', $builder_id)[2];
modules/display_builder_views/src/Controller/ViewsManagementController.php: $view_id = \explode('__', $builder_id)[1];
modules/display_builder_views/src/Controller/ViewsManagementController.php: $display_id = \explode('__', $builder_id)[2];
This is messy and risky.
Proposed resolution
Add a WithDisplayBuilderInterface::checkInstanceId() to extract parameters from Instance ID.
It is called like that and it is nullable, because the primary use of this method is to check the string is handled by the interface implementation.
It will also be a way to simplify the implementations of WithDisplayBuilderInterface::getUrlFromInstanceId()
Issue fork display_builder-3541423
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:
Comments
Comment #3
pdureau commentedWork started: https://git.drupalcode.org/project/display_builder/-/merge_requests/78
To be tested more locally and we need to wait the last merges of alpha4 anyway.
Comment #4
pdureau commentedComment #5
pdureau commentedFinished. The logic is now only located in classes impelmtning
WithDisplayBuilderInterface:But before sending that to review to others, let's test a bit more locally
Comment #6
pdureau commentedTests:
Comment #7
pdureau commentedComment #9
mogtofu33 commentedComment #11
pdureau commented