Drupal core has been lacking a fairly easy to use form wizard for a long time. CTools has a good example of how these can work, and with the advent of tempstore in core, officially blessing an approach is likely a good idea.

Config entities are the new way of creating most of our forms and storing new data. After working with them for a while, I began to notice similarities in the way they work and the way CTools wizard component works. With very minimal effort and a single class I managed to get a multi-step wizard working for my configuration entity.

The basic approach requires the developer to add "steps" to their entity annotation. The real magic lies in the WizardFormController which is just an extension of the EntityFormController which I hope means we can do this for config and content entities. I'm actively working with a config entity, but if we could start building up tests around this class, I think we could begin to illustrate how this works, and how easy it is.

Eclipse

Comments

Status:Active» Needs review
StatusFileSize
new5.97 KB
PASSED: [[SimpleTest]]: [MySQL] 49,835 pass(es).
[ View ]

And the patch. No tests yet, so I'm labeling it for no testing.

Eclipse

StatusFileSize
new5.98 KB
PASSED: [[SimpleTest]]: [MySQL] 49,771 pass(es).
[ View ]

oops, left condition_group_load() call in the system_wizard_form() function. fixed!

aaaaand I named them wrong... go me.

In case anyone gets to writing tests before me, a hand full of code snippets I used to make it to this point:

My Config entity class's annotation reads thus:

<?php
/**
* Defines the condition entity.
*
* @Plugin(
*   id = "condition_group",
*   label = @Translation("Condition Group"),
*   module = "condition",
*   controller_class = "Drupal\Core\Config\Entity\ConfigStorageController",
*   list_controller_class = "Drupal\Core\Entity\EntityListController",
*   form_controller_class = {
*     "edit" = "Drupal\condition\ConditionGroupAddFormController",
*     "configure" = "Drupal\condition\ConditionGroupConfigureFormController",
*     "delete" = "Drupal\condition\ConditionGroupDeleteFormController"
*   },
*   steps = {
*     "edit" = @Translation("Edit details"),
*     "configure" = @Translation("Configure conditions")
*   },
*   config_prefix = "condition.group",
*   entity_keys = {
*     "id" = "id",
*     "label" = "label",
*     "uuid" = "uuid"
*   }
* )
*/
?>

The steps in there are literally the order in which you will traverse the set of forms.

My menu items of note are thus:

<?php
/**
* Implements hook_menu().
*/
function condition_menu() {
 
$items = array();
 
$items['admin/config/system/conditions/add'] = array(
   
'title' => 'New Condition Group',
   
'page callback' => 'system_wizard_form',
   
'page arguments' => array('condition_group', 'edit'),
   
'access arguments' => array('administer conditions'),
   
'type' => MENU_LOCAL_ACTION,
  );
 
$items['admin/config/system/conditions/%/%'] = array(
   
'title' => 'New Condition Group',
   
'page callback' => 'system_wizard_form',
   
'page arguments' => array('condition_group', 5, 4),
   
'access arguments' => array('administer conditions'),
   
'type' => MENU_CALLBACK,
  );
  return
$items;
}
?>

Hopefully this illuminates how to use this a little better till I write up more docs.

Eclipse

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+    $definition = $entity->entityInfo();
...
+    $entity = $this->buildEntity($form, $form_state += array('values' => array()));
+    $definition = $entity->entityInfo();

This would be better if the class had protected $entityInfo; and you set it in form().

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+    drupal_set_title($definition['steps'][$this->getOperation()]);
+++ b/core/modules/system/system.moduleundefined
@@ -1147,6 +1147,35 @@ function system_plugin_ui_access($plugin, $facet = NULL) {
+function system_wizard_form($entity_type, $step, $id = NULL) {

drupal_set_title shouldn't be called from within form functions/methods, move it to the wrapper

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+    $before = array_slice($definition['steps'], 0, array_search($this->getOperation(), $steps));
+    $after = array_slice($definition['steps'], array_search($this->getOperation(), $steps) + 1);

Each of these lines needs a comment.

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+      $entity = $this->buildEntity($form, $form_state);

Pretty sure this is done in parent::submit(), is there a reason to not use that?

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+      drupal_container()->get('user.tempstore')->get($definition['id'])->set($entity->id(), $entity);

What is $definition['id']?

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+      $steps = array_keys($definition['steps']);
+      $after = array_slice($definition['steps'], array_search($this->getOperation(), $steps) + 1);
+      $after = array_keys($after);
+      $after = array_reverse($after);
...
+    $before = array_slice($definition['steps'], 0, array_search($this->getOperation(), $steps));
+    $before = array_keys($before);
+    $before = array_reverse($before);

WHOOO array_EVERYTHING (comments please)

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+  public function previous(array $form, array &$form_state) {

This doesn't call $this->setEntity() and it probably should

+++ b/core/modules/system/system.moduleundefined
@@ -1147,6 +1147,35 @@ function system_plugin_ui_access($plugin, $facet = NULL) {
+    $entity = entity_create($entity_type, array());
...
+    if (!method_exists($entity, 'entityType')) {
+      $entity = entity_load($entity_type, $id);

This should do one or the other. Could probably be two different form functions.

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+    $definition = $entity->entityInfo();
...
+    $entity = $this->buildEntity($form, $form_state += array('values' => array()));
+    $definition = $entity->entityInfo();
This would be better if the class had protected $entityInfo; and you set it in form().

Agreed, next patch

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+    drupal_set_title($definition['steps'][$this->getOperation()]);
+++ b/core/modules/system/system.moduleundefined
@@ -1147,6 +1147,35 @@ function system_plugin_ui_access($plugin, $facet = NULL) {
+function system_wizard_form($entity_type, $step, $id = NULL) {
drupal_set_title shouldn't be called from within form functions/methods, move it to the wrapper

Which wrapper are you speaking about?

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+      $entity = $this->buildEntity($form, $form_state);
Pretty sure this is done in parent::submit(), is there a reason to not use that?

yes, parent submit does a bunch of stuff I'm pretty sure I don't need yet. I do call it in the final "finish" phase of the system. Until then we are just building the entity from the form_values and updating the tempstore.

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+      drupal_container()->get('user.tempstore')->get($definition['id'])->set($entity->id(), $entity);

What is $definition['id']?

$definition = $entity->entityInfo(); It's the entity type... I'm guessing there's probably a better way to get that then asking the plugin definition...

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,127 @@
+  public function previous(array $form, array &$form_state) {

This doesn't call $this->setEntity() and it probably should

I still don't see a reason to call setEntity() right as I leave the class and go elsewhere, but I'll look at the method closer to make a better call on this.

+++ b/core/modules/system/system.moduleundefined
@@ -1147,6 +1147,35 @@ function system_plugin_ui_access($plugin, $facet = NULL) {
+    $entity = entity_create($entity_type, array());
...
+    if (!method_exists($entity, 'entityType')) {
+      $entity = entity_load($entity_type, $id);

This should do one or the other. Could probably be two different form functions.

The logic of deciding whether we're a.) creating a new entity, b.) pulling one from the tempstore or c.) loading an existing entity, is pretty interrelated, but I'm open to suggestions. It just seemed like a fairly small utility function that did the logic for people to know what we were dealing with for each step of the form. Again, open to suggestions here.

Eclipse

OK, I responded to most of tim's comments in code here. I know I need to document the entity_info variable in the class, sorry I missed it, I'll get it on the next pass. There are still a handful of outstanding things from tim's comment that need clarification and addressing, specifically the use of setEntity() as we move from one class to the next and my desire to not invoke parent::submit() until we finish the wizard. Otherwise I think I got most of the points.

Eclipse

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,130 @@
+  protected $entity_info;
...
+    $this->entity_info = $entity->entityInfo();

Should be $entityInfo

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,130 @@
+    $this->entity_info = $entity->entityInfo();

You don't need to call this again, you already have it from init()

+++ b/core/lib/Drupal/Core/Wizard/WizardFormController.phpundefined
@@ -0,0 +1,130 @@
+  public function submit(array $form, array &$form_state) {
...
+   */
+  public function finish(array $form, array &$form_state) {

I would rename your submit() to next() and then finish() to submit(), so the parent::submit makes more sense. You'll need to tweak actions() more, but that's not a huge deal.

---
Other than the wrapper that does 3 things I'm rather liking this code.

Multiform wizards are a pita with drupal. I hoped that we could have a form api solution for this. As with this patch you need to create a custom entity, just do that.

$form_state['redirect'] = 'admin/config/system/conditions';

Seems like a hardcoded solution for your case.

And where do you define your wizard controller? Can you have multiple wizard controllers for each entity?

Good catch on the redirect, sorry some of my use case snuck in. I'll remove it next patch.

With regard to where we define stuff and if we can have multiple:

Defining your wizard:

The current approach does this in the entity class itself. The entity class annotations contain the various EntityFormController extensions defined by simple names. For example:

*   form_controller_class = {
*     "edit" = "Drupal\condition\ConditionGroupAddFormController",
*     "configure" = "Drupal\condition\ConditionGroupConfigureFormController",
*     "delete" = "Drupal\condition\ConditionGroupDeleteFormController"
*   },

This is how things are done today, and I'm not proposing changing that any. In the case of using the code I've provided here, instead of your entity form controller classes extending EntityFormController, they would extend WizardFormController (which is already extending EntityFormController). This would then introspect the Entity class further to determine the order of steps.

Example:

*   steps = {
*     "edit" = @Translation("Edit details"),
*     "configure" = @Translation("Configure conditions")
*   },

If you'll notice, the first code I pasted included edit, configure, delete, but since I don't want delete to be part of the typical steps, it's not in the steps list here.

Defining multiple wizards for the same entity:

All the WizardFormController does here is introspect the 'steps' section of the annotation. If we wanted to make that a variable on the Wizard class, that would obviously be trivial, and then you could just override the value of that variable with your own form controller when you extend it. This seems pretty edge-casey to me, but also pretty trivial to support.

My bigger concern:

My bigger concern in all of this is the "choose your own adventure" style wizard. I think this can be done through each class overriding their own submit method and inspecting the $form_state['values']. I intend on trying it out to see in the tests that need to be written.

Eclipse

StatusFileSize
new7.97 KB

Ok, I think this takes care of all the current feedback. I chose not to make it next() and submit() since submit() will be fired no matter what in any forward moving wizard progression. I've tried to add a comment to this effect to finish().

I moved the hardcoded 'steps' to a protected $stepKey = 'steps'; in the class. This should allow multiple wizards to be supported by a single entity if necessary. I consider that pretty much an edge case, but I'm supporting it (and will write tests for it) none the less. Aspilicious pointed out that I had left some of my use case behind. Removing that required a protected $destinationKey = 'destination'; This corresponds to the key in the entity definition. A base url will be expected there. I also added a redirect method to the class in order to give individual form controllers greater control over what the uri of the next step is. The default logic there should work for the vast vast majority of cases.

Tests should be the next thing required here unless anyone has more objections to the code as it currently stands.

Eclipse

didn't have time to make an interdiff. Will try to get it tonight if possible.

Eclipse

StatusFileSize
new5.84 KB

ok, the interdiff

+++ b/core/modules/system/system.moduleundefined
@@ -1176,6 +1176,41 @@ function system_plugin_ui_access($plugin, $facet = NULL) {
+    if (!method_exists($entity, 'entityType')) {

There has got to be a better way to do this.
What else comes out of the tempstore for this? The object you want, or NULL?

Title:Multistep Form WIzardMultistep Form Wizard
StatusFileSize
new8.4 KB

This is my most recent version to go along with #1912452: Condition Group User Interface. A few changes here to make it more generic. Effulgentsia suggested moving the system function I've added into entity.inc. I'm fine with that if others concur.

Eclipse

StatusFileSize
new9.27 KB

Removed the system_wizard_form() function and moved it to a create method on the controller itself. We now register the controller stand alone in the DIC. Routes can use this pretty simply doing something similar to this:

condition.step:
  pattern: '/admin/config/system/conditions/{id}/{operation}'
  defaults:
    _controller: 'entity.wizard:create'
    entity_type: 'condition_group'
  requirements:
    _permission: 'administer conditions'

Still no tests, but I'm hoping this is the final requisite cleanup before writing tests.

Eclipse

Total drive-by half-ass look at this code: It's weird there's a previous() but not a next().

well, submit() is next. but I hear you.

Just pasting a to-do for this issue from EclipseGC on IRC so I don't loose/forget it

so we should move the create method of to a getForm() method and then we should write tests and make sure the docs are all sane

Component:other» forms system

Is this still going to happen in D8?

What left needs to be done?

following up on this, what needs to be done to get this into core?

Issue summary:View changes
Status:Needs review» Needs work

This still needs work doesn't it?

Moving this to a sandbox for the time being with a bit of a different approach:

#2165545: Provide a generic wizard functionality

Eclipse

@EclipseGc: What's the status on this? Do you think the Page Manager approach you're working on in the sandbox could make it into core, or would we be better off pursuing the approach here?

Thanks.

Kinda moved out of core for this for the time being, I'd check here: #2165545: Provide a generic wizard functionality

Eclipse