diff --git a/core/modules/layout_builder/layout_builder.module b/core/modules/layout_builder/layout_builder.module index 3bf7378..12545bc 100644 --- a/core/modules/layout_builder/layout_builder.module +++ b/core/modules/layout_builder/layout_builder.module @@ -43,6 +43,8 @@ function layout_builder_form_entity_view_display_edit_form_alter(&$form, FormSta '#title' => t('Layout options'), '#tree' => TRUE, ]; + // @todo Unchecking this box is a destructive action, this should be made + // clear to the user. $form['layout']['allow_custom'] = [ '#type' => 'checkbox', '#title' => t('Allow each @entity to have its layout customized.', [ diff --git a/core/modules/layout_builder/layout_builder.permissions.yml b/core/modules/layout_builder/layout_builder.permissions.yml index 0d66577..1fca7af 100644 --- a/core/modules/layout_builder/layout_builder.permissions.yml +++ b/core/modules/layout_builder/layout_builder.permissions.yml @@ -1,3 +1,4 @@ +# @todo Expand permissions to be more granular. configure any layout: title: 'Configure any layout' restrict access: true diff --git a/core/modules/layout_builder/layout_builder.routing.yml b/core/modules/layout_builder/layout_builder.routing.yml index 718e678..78e7410 100644 --- a/core/modules/layout_builder/layout_builder.routing.yml +++ b/core/modules/layout_builder/layout_builder.routing.yml @@ -29,6 +29,8 @@ layout_builder.configure_section: defaults: _title: 'Configure section' _form: '\Drupal\layout_builder\Form\ConfigureSectionForm' + # Adding a new section requires a plugin_id, while configuring an existing + # section does not. plugin_id: null requirements: _permission: 'configure any layout' diff --git a/core/modules/layout_builder/layout_builder.services.yml b/core/modules/layout_builder/layout_builder.services.yml index ba8e0cc..a717a02 100644 --- a/core/modules/layout_builder/layout_builder.services.yml +++ b/core/modules/layout_builder/layout_builder.services.yml @@ -13,8 +13,6 @@ services: layout_builder.routes: class: Drupal\layout_builder\Routing\LayoutBuilderRoutes arguments: ['@entity_type.manager'] - tags: - - { name: event_subscriber } layout_builder.route_enhancer: class: Drupal\layout_builder\Routing\LayoutBuilderRouteEnhancer arguments: ['@entity_type.manager'] diff --git a/core/modules/layout_builder/src/Access/LayoutSectionAccessCheck.php b/core/modules/layout_builder/src/Access/LayoutSectionAccessCheck.php index 8525eea..d756ca3 100644 --- a/core/modules/layout_builder/src/Access/LayoutSectionAccessCheck.php +++ b/core/modules/layout_builder/src/Access/LayoutSectionAccessCheck.php @@ -43,7 +43,10 @@ public function __construct(EntityTypeManagerInterface $entity_type_manager) { * The access result. */ public function access(RouteMatchInterface $route_match, AccountInterface $account) { - $entity = $route_match->getParameter('entity'); + // Attempt to retrive the generic 'entity' parameter, otherwise look up the + // specific entity via the entity type ID. + $entity = $route_match->getParameter('entity') ?: $route_match->getParameter($route_match->getParameter('entity_type_id')); + // If we don't have an entity, forbid access. if (empty($entity)) { return AccessResult::forbidden()->addCacheContexts(['route']); diff --git a/core/modules/layout_builder/src/Controller/AddSectionController.php b/core/modules/layout_builder/src/Controller/AddSectionController.php index 56af3f2..5f6b9ce 100644 --- a/core/modules/layout_builder/src/Controller/AddSectionController.php +++ b/core/modules/layout_builder/src/Controller/AddSectionController.php @@ -12,7 +12,7 @@ use Symfony\Component\HttpFoundation\RequestStack; /** - * Returns responses for Layout Builder routes. + * @todo. */ class AddSectionController implements ContainerInjectionInterface { @@ -37,8 +37,8 @@ class AddSectionController implements ContainerInjectionInterface { * The request stack. */ public function __construct(LayoutTempstoreRepositoryInterface $layout_tempstore_repository, ClassResolverInterface $class_resolver, RequestStack $request_stack) { - $this->classResolver = $class_resolver; $this->layoutTempstoreRepository = $layout_tempstore_repository; + $this->classResolver = $class_resolver; $this->requestStack = $request_stack; } diff --git a/core/modules/layout_builder/src/Controller/AjaxHelperTrait.php b/core/modules/layout_builder/src/Controller/AjaxHelperTrait.php index 9aee82e..66a0d47 100644 --- a/core/modules/layout_builder/src/Controller/AjaxHelperTrait.php +++ b/core/modules/layout_builder/src/Controller/AjaxHelperTrait.php @@ -6,6 +6,8 @@ /** * Provides a helper to determine if the current request is via AJAX. + * + * @todo Move to \Drupal\Core in https://www.drupal.org/node/2896535. */ trait AjaxHelperTrait { diff --git a/core/modules/layout_builder/src/Controller/ChooseBlockController.php b/core/modules/layout_builder/src/Controller/ChooseBlockController.php index 7bb5819..3d2ff8b 100644 --- a/core/modules/layout_builder/src/Controller/ChooseBlockController.php +++ b/core/modules/layout_builder/src/Controller/ChooseBlockController.php @@ -10,7 +10,7 @@ use Symfony\Component\HttpFoundation\RequestStack; /** - * Returns responses for Layout Builder routes. + * @todo. */ class ChooseBlockController implements ContainerInjectionInterface { diff --git a/core/modules/layout_builder/src/Controller/ChooseSectionController.php b/core/modules/layout_builder/src/Controller/ChooseSectionController.php index 6dd89eb..7055f6e 100644 --- a/core/modules/layout_builder/src/Controller/ChooseSectionController.php +++ b/core/modules/layout_builder/src/Controller/ChooseSectionController.php @@ -12,7 +12,7 @@ use Symfony\Component\HttpFoundation\RequestStack; /** - * Returns responses for Layout Builder routes. + * @todo. */ class ChooseSectionController implements ContainerInjectionInterface { @@ -91,6 +91,7 @@ public function build(EntityInterface $entity, $delta) { } $items[] = $item; } + // @todo Look into removing this details wrapper, or rewording the title. $output['layouts'] = [ '#type' => 'details', '#title' => $this->t('Basic Layouts'), diff --git a/core/modules/layout_builder/src/Controller/LayoutBuilderController.php b/core/modules/layout_builder/src/Controller/LayoutBuilderController.php index 4b1410b..0195599 100644 --- a/core/modules/layout_builder/src/Controller/LayoutBuilderController.php +++ b/core/modules/layout_builder/src/Controller/LayoutBuilderController.php @@ -16,7 +16,7 @@ use Symfony\Component\HttpFoundation\RedirectResponse; /** - * Returns responses for Layout Builder routes. + * @todo. */ class LayoutBuilderController implements ContainerInjectionInterface { diff --git a/core/modules/layout_builder/src/Controller/MoveBlockController.php b/core/modules/layout_builder/src/Controller/MoveBlockController.php index ff0d5e8..b1b771d 100644 --- a/core/modules/layout_builder/src/Controller/MoveBlockController.php +++ b/core/modules/layout_builder/src/Controller/MoveBlockController.php @@ -11,7 +11,7 @@ use Symfony\Component\HttpFoundation\RequestStack; /** - * Returns responses for Layout Builder routes. + * @todo. */ class MoveBlockController implements ContainerInjectionInterface { @@ -64,6 +64,8 @@ public static function create(ContainerInterface $container) { * An AJAX response. */ public function build(EntityInterface $entity, Request $request) { + // @todo Either enforce the presence of each part of $data, or convert this + // to use URL parameters. $data = $request->request->all(); /** @var \Drupal\layout_builder\LayoutSectionItemInterface $field */ diff --git a/core/modules/layout_builder/src/Form/ConfigureBlockForm.php b/core/modules/layout_builder/src/Form/ConfigureBlockForm.php index 8a47a5f..f377a3f 100644 --- a/core/modules/layout_builder/src/Form/ConfigureBlockForm.php +++ b/core/modules/layout_builder/src/Form/ConfigureBlockForm.php @@ -204,8 +204,8 @@ public function buildForm(array $form, FormStateInterface $form_state, EntityInt * {@inheritdoc} */ public function validateForm(array &$form, FormStateInterface $form_state) { - $sub_form_state = SubformState::createForSubform($form['settings'], $form, $form_state); - $this->getPluginForm($this->block)->validateConfigurationForm($form['settings'], $sub_form_state); + $subform_state = SubformState::createForSubform($form['settings'], $form, $form_state); + $this->getPluginForm($this->block)->validateConfigurationForm($form['settings'], $subform_state); } /** @@ -213,12 +213,12 @@ public function validateForm(array &$form, FormStateInterface $form_state) { */ public function submitForm(array &$form, FormStateInterface $form_state) { // Call the plugin submit handler. - $sub_form_state = SubformState::createForSubform($form['settings'], $form, $form_state); - $this->getPluginForm($this->block)->submitConfigurationForm($form, $sub_form_state); + $subform_state = SubformState::createForSubform($form['settings'], $form, $form_state); + $this->getPluginForm($this->block)->submitConfigurationForm($form, $subform_state); // If this block is context-aware, set the context mapping. if ($this->block instanceof ContextAwarePluginInterface) { - $this->block->setContextMapping($sub_form_state->getValue('context_mapping', [])); + $this->block->setContextMapping($subform_state->getValue('context_mapping', [])); } $configuration = $this->block->getConfiguration(); @@ -234,7 +234,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) { } /** - * Retrieves the plugin form for a given block and operation. + * Retrieves the plugin form for a given block. * * @param \Drupal\Core\Block\BlockPluginInterface $block * The block plugin. diff --git a/core/modules/layout_builder/src/Form/ConfigureSectionForm.php b/core/modules/layout_builder/src/Form/ConfigureSectionForm.php index 082eaec..1ee97f0 100644 --- a/core/modules/layout_builder/src/Form/ConfigureSectionForm.php +++ b/core/modules/layout_builder/src/Form/ConfigureSectionForm.php @@ -7,7 +7,11 @@ use Drupal\Core\Form\FormBase; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Form\SubformState; +use Drupal\Core\Layout\LayoutInterface; use Drupal\Core\Layout\LayoutPluginManagerInterface; +use Drupal\Core\Plugin\PluginFormFactoryInterface; +use Drupal\Core\Plugin\PluginFormInterface; +use Drupal\Core\Plugin\PluginWithFormsInterface; use Drupal\layout_builder\Controller\LayoutRebuildFormTrait; use Drupal\layout_builder\LayoutTempstoreRepositoryInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -42,6 +46,13 @@ class ConfigureSectionForm extends FormBase { protected $layoutManager; /** + * The plugin form manager. + * + * @var \Drupal\Core\Plugin\PluginFormFactoryInterface + */ + protected $pluginFormFactory; + + /** * The entity. * * @var \Drupal\Core\Entity\EntityInterface @@ -73,12 +84,15 @@ class ConfigureSectionForm extends FormBase { * The class resolver. * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. + * @param \Drupal\Core\Plugin\PluginFormFactoryInterface $plugin_form_manager + * The plugin form manager. */ - public function __construct(LayoutTempstoreRepositoryInterface $layout_tempstore_repository, LayoutPluginManagerInterface $layout_manager, ClassResolverInterface $class_resolver, RequestStack $request_stack) { + public function __construct(LayoutTempstoreRepositoryInterface $layout_tempstore_repository, LayoutPluginManagerInterface $layout_manager, ClassResolverInterface $class_resolver, RequestStack $request_stack, PluginFormFactoryInterface $plugin_form_manager) { $this->layoutTempstoreRepository = $layout_tempstore_repository; $this->layoutManager = $layout_manager; $this->classResolver = $class_resolver; $this->requestStack = $request_stack; + $this->pluginFormFactory = $plugin_form_manager; } /** @@ -89,7 +103,8 @@ public static function create(ContainerInterface $container) { $container->get('layout_builder.tempstore_repository'), $container->get('plugin.manager.core.layout'), $container->get('class_resolver'), - $container->get('request_stack') + $container->get('request_stack'), + $container->get('plugin_form.factory') ); } @@ -120,7 +135,7 @@ public function buildForm(array $form, FormStateInterface $form_state, EntityInt $form['#tree'] = TRUE; $form['layout_settings'] = []; $subform_state = SubformState::createForSubform($form['layout_settings'], $form, $form_state); - $form['layout_settings'] = $this->layout->buildConfigurationForm($form['layout_settings'], $subform_state); + $form['layout_settings'] = $this->getPluginForm($this->layout)->buildConfigurationForm($form['layout_settings'], $subform_state); $form['actions']['submit'] = [ '#type' => 'submit', @@ -139,7 +154,7 @@ public function buildForm(array $form, FormStateInterface $form_state, EntityInt */ public function validateForm(array &$form, FormStateInterface $form_state) { $subform_state = SubformState::createForSubform($form['layout_settings'], $form, $form_state); - $this->layout->validateConfigurationForm($form['layout_settings'], $subform_state); + $this->getPluginForm($this->layout)->validateConfigurationForm($form['layout_settings'], $subform_state); } /** @@ -148,7 +163,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) { public function submitForm(array &$form, FormStateInterface $form_state) { // Call the plugin submit handler. $subform_state = SubformState::createForSubform($form['layout_settings'], $form, $form_state); - $this->layout->submitConfigurationForm($form['layout_settings'], $subform_state); + $this->getPluginForm($this->layout)->submitConfigurationForm($form['layout_settings'], $subform_state); $plugin_id = $this->layout->getPluginId(); $configuration = $this->layout->getConfiguration(); @@ -172,4 +187,25 @@ public function submitForm(array &$form, FormStateInterface $form_state) { $form_state->setRedirect("entity.{$this->entity->getEntityTypeId()}.layout", [$this->entity->getEntityTypeId() => $this->entity->id()]); } + /** + * Retrieves the plugin form for a given layout. + * + * @param \Drupal\Core\Layout\LayoutInterface $layout + * The layout plugin. + * + * @return \Drupal\Core\Plugin\PluginFormInterface + * The plugin form for the layout. + */ + protected function getPluginForm(LayoutInterface $layout) { + if ($layout instanceof PluginWithFormsInterface) { + return $this->pluginFormFactory->createInstance($layout, 'configure'); + } + + if ($layout instanceof PluginFormInterface) { + return $layout; + } + + throw new \InvalidArgumentException(sprintf('The "%s" layout does not provide a configuration form', $layout->getPluginId())); + } + } diff --git a/core/modules/layout_builder/src/LayoutTempstoreRepository.php b/core/modules/layout_builder/src/LayoutTempstoreRepository.php index af6f2d1..652e9c7 100644 --- a/core/modules/layout_builder/src/LayoutTempstoreRepository.php +++ b/core/modules/layout_builder/src/LayoutTempstoreRepository.php @@ -46,7 +46,11 @@ public function get(EntityInterface $entity) { list($collection, $id) = $this->generateTempstoreId($entity); $tempstore = $this->tempStoreFactory->get($collection)->get($id); if (!empty($tempstore['entity'])) { - return $tempstore['entity']; + $entity = $tempstore['entity']; + + if (!($entity instanceof EntityInterface)) { + throw new \UnexpectedValueException(sprintf('The entry for collection "%s" and ID "%s" is not a valid entity', $collection, $id)); + } } return $entity; } diff --git a/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php b/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php index 8043c84..af6f382 100644 --- a/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php +++ b/core/modules/layout_builder/src/LayoutTempstoreRepositoryInterface.php @@ -18,6 +18,9 @@ * @return \Drupal\Core\Entity\EntityInterface * Either the version of this entity from tempstore, or the passed entity if * none exists. + * + * @throw \UnexpectedValueException + * Thrown if a value exists, but is not an entity. */ public function get(EntityInterface $entity); @@ -32,6 +35,9 @@ public function get(EntityInterface $entity); * @return \Drupal\Core\Entity\EntityInterface * Either the version of this entity from tempstore, or the entity from * storage if none exists. + * + * @throw \UnexpectedValueException + * Thrown if a value exists, but is not an entity. */ public function getFromId($entity_type_id, $entity_id); diff --git a/core/modules/layout_builder/src/Plugin/Field/FieldType/LayoutSectionItem.php b/core/modules/layout_builder/src/Plugin/Field/FieldType/LayoutSectionItem.php index 7fdfc64..ab7aae7 100644 --- a/core/modules/layout_builder/src/Plugin/Field/FieldType/LayoutSectionItem.php +++ b/core/modules/layout_builder/src/Plugin/Field/FieldType/LayoutSectionItem.php @@ -78,11 +78,13 @@ public static function schema(FieldStorageDefinitionInterface $field_definition) 'layout_settings' => [ 'type' => 'blob', 'size' => 'normal', + // @todo Is this okay? 'serialize' => TRUE, ], 'section' => [ 'type' => 'blob', 'size' => 'normal', + // @todo Is this okay? 'serialize' => TRUE, ], ], @@ -97,6 +99,7 @@ public static function schema(FieldStorageDefinitionInterface $field_definition) public static function generateSampleValue(FieldDefinitionInterface $field_definition) { $values['layout'] = 'layout_onecol'; $values['layout_settings'] = []; + // @todo Expand this in https://www.drupal.org/node/2912331. $values['section'] = []; return $values; } diff --git a/core/modules/layout_builder/src/Plugin/Menu/LayoutBuilderLocalTask.php b/core/modules/layout_builder/src/Plugin/Menu/LayoutBuilderLocalTask.php index 329f5d3..d2feff1 100644 --- a/core/modules/layout_builder/src/Plugin/Menu/LayoutBuilderLocalTask.php +++ b/core/modules/layout_builder/src/Plugin/Menu/LayoutBuilderLocalTask.php @@ -16,6 +16,9 @@ class LayoutBuilderLocalTask extends LocalTaskDefault { public function getRouteParameters(RouteMatchInterface $route_match) { $parameters = parent::getRouteParameters($route_match); + // @todo This assumes that the route match contains a valid entity, + // investigate whether that assumption is safe, or if this code is even + // needed. $parameters['entity'] = $route_match->getParameter('entity'); return $parameters; } diff --git a/core/modules/layout_builder/src/Routing/LayoutBuilderRouteEnhancer.php b/core/modules/layout_builder/src/Routing/LayoutBuilderRouteEnhancer.php index 8aee689..0faca8f 100644 --- a/core/modules/layout_builder/src/Routing/LayoutBuilderRouteEnhancer.php +++ b/core/modules/layout_builder/src/Routing/LayoutBuilderRouteEnhancer.php @@ -3,6 +3,7 @@ namespace Drupal\layout_builder\Routing; use Drupal\Core\Routing\Enhancer\RouteEnhancerInterface; +use Symfony\Cmf\Component\Routing\RouteObjectInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; @@ -23,6 +24,10 @@ public function applies(Route $route) { * {@inheritdoc} */ public function enhance(array $defaults, Request $request) { + if (!isset($defaults['entity_type_id'])) { + throw new \RuntimeException(sprintf('Failed to find the entity type ID in route named %s', $defaults[RouteObjectInterface::ROUTE_NAME])); + } + // Copy the entity by reference so that any changes are reflected. $defaults['entity'] = &$defaults[$defaults['entity_type_id']]; return $defaults; diff --git a/core/modules/layout_builder/src/Routing/LayoutBuilderRoutes.php b/core/modules/layout_builder/src/Routing/LayoutBuilderRoutes.php index 97adac6..3bb14ab 100644 --- a/core/modules/layout_builder/src/Routing/LayoutBuilderRoutes.php +++ b/core/modules/layout_builder/src/Routing/LayoutBuilderRoutes.php @@ -5,14 +5,12 @@ use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Entity\FieldableEntityInterface; -use Drupal\Core\Routing\RouteSubscriberBase; use Symfony\Component\Routing\Route; -use Symfony\Component\Routing\RouteCollection; /** * Provides routes for the Layout Builder UI. */ -class LayoutBuilderRoutes extends RouteSubscriberBase { +class LayoutBuilderRoutes { /** * The entity type manager. @@ -110,30 +108,6 @@ public function getRoutes() { } /** - * {@inheritdoc} - */ - protected function alterRoutes(RouteCollection $collection) { - $templates = ['canonical', 'edit_form', 'delete_form']; - foreach ($this->getEntityTypes() as $entity_type) { - foreach ($templates as $template) { - // Mark this as a Layout Builder route so that links like local tasks - // will be enhanced. - if ($route = $collection->get('entity.' . $entity_type->id() . '.' . $template)) { - $parameters = $route->getOption('parameters'); - $parameters[$entity_type->id()]['type'] = 'entity:{entity_type_id}'; - $parameters[$entity_type->id()]['layout_builder_tempstore'] = TRUE; - $route->setOption('parameters', $parameters); - $route->setOption('_layout_builder', TRUE); - $route->addDefaults([ - 'entity' => NULL, - 'entity_type_id' => $entity_type->id(), - ]); - } - } - } - } - - /** * Returns an array of relevant entity types. * * @return \Drupal\Core\Entity\EntityTypeInterface[] diff --git a/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php b/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php index 919b02c..faef394 100644 --- a/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php +++ b/core/modules/layout_builder/tests/src/Unit/LayoutTempstoreRepositoryTest.php @@ -107,4 +107,27 @@ public function testGetFromIdRevisionable() { $this->assertSame($entity->reveal(), $result); } + /** + * @covers ::get + */ + public function testGetInvalidEntity() { + $tempstore = $this->prophesize(SharedTempStore::class); + $tempstore->get('the_entity_id.en')->willReturn(['entity' => 'this_is_not_an_entity']); + + $tempstore_factory = $this->prophesize(SharedTempStoreFactory::class); + $tempstore_factory->get('the_entity_type_id.layout_builder__layout')->willReturn($tempstore->reveal()); + + $entity_type_manager = $this->prophesize(EntityTypeManagerInterface::class); + + $repository = new LayoutTempstoreRepository($tempstore_factory->reveal(), $entity_type_manager->reveal()); + + $entity = $this->prophesize(EntityInterface::class); + $entity->language()->willReturn(new Language(['id' => 'en'])); + $entity->getEntityTypeId()->willReturn('the_entity_type_id'); + $entity->id()->willReturn('the_entity_id'); + + $this->setExpectedException(\UnexpectedValueException::class, 'The entry for collection "the_entity_type_id.layout_builder__layout" and ID "the_entity_id.en" is not a valid entity'); + $repository->get($entity->reveal()); + } + }