Problem/Motivation

Wizards can be a really useful for things like editing complex entities. For example, #2550879: Use CTools Wizard API to add/edit Pages (and move plugin UI using PluginWizardInterface) is currently working on switching Page Manager over to using wizards for the add/edit forms. However, as wizards need a string {machine_name} in the path, they are currently incompatible with link templates without each entity overriding \Drupal\Core\Entity\Entity::urlRouteParameters to provide the machine name when needed.

Although this is not unreasonable in some cases, there are no hooks in ::urlRouteParameters, so if the module providing the route/wizards is not the same one that provides the entity, you are left with one of the following options:

  • Unable to use link templates for wizard routes
  • Have to swap out the entity class (prone to conflicts and unpleasant)
  • Get the module providing the entity to add in it's own hook (a lot of work every time it's needed)

Proposed resolution

CTools could provide a param converter which would convert a path parameter into a machine name. This would allow modules providing routes to simply add some information to their route definition and have it automatically work with link templates.

Remaining tasks

  • Implement it
  • Work out if/what test coverage is needed

User interface changes

None.

API changes

Only new addition of ways to create routes that support link templates.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andrewbelcher created an issue. See original summary.

andrewbelcher’s picture

Status: Active » Needs review
FileSize
3.94 KB

Here is a patch that allows this. As an example from Page Manager of how you would update your route to make use of this:

 entity.page.edit_form:
-  path: '/admin/structure/page_manager/manage/{machine_name}/{step}'
+  path: '/admin/structure/page_manager/manage/{page}/{step}'
   defaults:
     _entity_wizard: 'page.edit'
     _title_callback: '\Drupal\page_manager_ui\Controller\PageManagerController::editPageTitle'
     tempstore_id: page_manager.page
-    page: '{machine_name}'
+    machine_name: page
+    step: general
   options:
     parameters:
+      page:
+        type: entity:page
+      machine_name:
+        type: id_from_entity
   requirements:
     _permission: 'administer pages'

Specifically:

  • You can now use {entity_type} in your path, meaning the link template will be able to provide the correct page.
  • machin_name now is set to the name of the parameter containing the entity.
  • A default for step must be provided for the link template to work, but as link templates only make sense when there is a default, that's fine.
  • The machine_name uses the newly provided id_from_entity type. This will pull the id from {page}, whether it's a string (id) or an EntityInterface (loaded entity). You have to define page's upcasting manually as it doesn't automatically merge it in from what I can see.

The following link template is now supported:
$entity_types['page']->setLinkTemplate('edit-form', '/admin/structure/page_manager/manage/{machine_name}/{step}')

andrewbelcher’s picture

Issue tags: +Needs tests

As per conversation with @dsnopek on IRC meetup, tests should include making sure we can use access such as _entity_access: page.edit.

andrewbelcher’s picture

The test only patch is a test that checks link templates and demonstrates that they are currently broken.

The full patch contains the code that allows a fix, but also updates ctools_wizard_test module to make use of it and also \Drupal\ctools\Wizard\EntityFormWizardBase to automatically add the entity_type_id to the previous/next parameters.

As per @dsnopek's suggestion, the updates to the test module include switching over to using _entity_access rather than a simple permission check, so it also proves that now works.

The last submitted patch, 5: allow_link_templates-2690211-5-test_only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: allow_link_templates-2690211-5.patch, failed testing.

andrewbelcher’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

Marking back to needs review. The failing tests are also failing in the branch and are unrelated. The WizardRouteTest fails on the current code and correctly passes with the patch!

andrewbelcher’s picture

After a conversation with @EclipseGc, we've decided this is not a blocker for #2550879: Use CTools Wizard API to add/edit Pages (and move plugin UI using PluginWizardInterface). However, we still feel it has real benefit and when this is in will update the wizard in a follow up.

Chris Matthews’s picture

Priority: Major » Normal
Status: Needs review » Needs work
Issue tags: +Needs reroll

The 5 year old patch in #5 needs a reroll.

BS Pavan’s picture

Status: Needs work » Needs review
FileSize
8.85 KB

Hi @Chris Matthews
I have re-rolled against latest branch.

Thanks