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
Comment | File | Size | Author |
---|---|---|---|
#59 | 1886616-nr-bot.txt | 6.5 KB | needs-review-queue-bot |
#47 | 1886616-47-multistep-form-wizard.do-not-test.patch | 9.74 KB | mparker17 |
Issue fork drupal-1886616
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 #1
EclipseGc CreditAttribution: EclipseGc commentedAnd the patch. No tests yet, so I'm labeling it for no testing.
Eclipse
Comment #2
EclipseGc CreditAttribution: EclipseGc commentedoops, left condition_group_load() call in the system_wizard_form() function. fixed!
Comment #3
EclipseGc CreditAttribution: EclipseGc commentedaaaaand I named them wrong... go me.
Comment #4
EclipseGc CreditAttribution: EclipseGc commentedIn 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:
The steps in there are literally the order in which you will traverse the set of forms.
My menu items of note are thus:
Hopefully this illuminates how to use this a little better till I write up more docs.
Eclipse
Comment #5
tim.plunkettThis would be better if the class had
protected $entityInfo;
and you set it in form().drupal_set_title shouldn't be called from within form functions/methods, move it to the wrapper
Each of these lines needs a comment.
Pretty sure this is done in parent::submit(), is there a reason to not use that?
What is $definition['id']?
WHOOO array_EVERYTHING (comments please)
This doesn't call $this->setEntity() and it probably should
This should do one or the other. Could probably be two different form functions.
Comment #6
EclipseGc CreditAttribution: EclipseGc commentedAgreed, next patch
Which wrapper are you speaking about?
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.
$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...
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.
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
Comment #7
EclipseGc CreditAttribution: EclipseGc commentedOK, 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
Comment #8
tim.plunkettShould be $entityInfo
You don't need to call this again, you already have it from init()
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.
Comment #9
aspilicious CreditAttribution: aspilicious commentedMultiform 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.
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?
Comment #10
EclipseGc CreditAttribution: EclipseGc commentedGood 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:
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:
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
Comment #11
EclipseGc CreditAttribution: EclipseGc commentedOk, 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
Comment #12
EclipseGc CreditAttribution: EclipseGc commenteddidn't have time to make an interdiff. Will try to get it tonight if possible.
Eclipse
Comment #13
EclipseGc CreditAttribution: EclipseGc commentedok, the interdiff
Comment #14
tim.plunkettThere 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?
Comment #15
EclipseGc CreditAttribution: EclipseGc commentedThis 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
Comment #16
EclipseGc CreditAttribution: EclipseGc commentedRemoved 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:
Still no tests, but I'm hoping this is the final requisite cleanup before writing tests.
Eclipse
Comment #17
webchickTotal drive-by half-ass look at this code: It's weird there's a previous() but not a next().
Comment #18
EclipseGc CreditAttribution: EclipseGc commentedwell, submit() is next. but I hear you.
Comment #19
larowlanJust pasting a to-do for this issue from EclipseGC on IRC so I don't loose/forget it
Comment #20
jibranIs this still going to happen in D8?
Comment #21
chrisjlee CreditAttribution: chrisjlee commentedWhat left needs to be done?
Comment #22
gordon CreditAttribution: gordon commentedfollowing up on this, what needs to be done to get this into core?
Comment #23
philipz CreditAttribution: philipz commentedThis still needs work doesn't it?
Comment #24
EclipseGc CreditAttribution: EclipseGc commentedMoving this to a sandbox for the time being with a bit of a different approach:
#2165545: Provide a generic wizard functionality
Eclipse
Comment #25
mikeryan@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.
Comment #26
EclipseGc CreditAttribution: EclipseGc commentedKinda moved out of core for this for the time being, I'd check here: #2165545: Provide a generic wizard functionality
Eclipse
Comment #27
penyaskitoIs this feasible for 8.0.x, or should be at contrib space already?
Comment #28
webchickNo more features in 8.0.x, so this would be 8.1.x as far as I can tell.
Comment #29
mgiffordUnassigning so someone else can take it on.
Comment #30
bojanz CreditAttribution: bojanz commentedA new version of this code is now in ctools: #2468609: Rewrite Wizard Support for 8.x-3.x.
Comment #31
EclipseGc CreditAttribution: EclipseGc at Acquia commentedWe're well beyond the window for stuff like this and I just went ahead and wrote it for contrib instead. Dont' see a reason to keep this open.
Eclipse
Comment #32
tim.plunkettThis is for 8.1.x, who knows if we'll want to bring the CTools code to core later.
Comment #33
EclipseGc CreditAttribution: EclipseGc at Acquia commentedFair enough!
Comment #34
joachim CreditAttribution: joachim commentedReopening, as 8.1.x is being worked on now.
Comment #36
kenorb CreditAttribution: kenorb commentedcore/lib/Drupal/Core/CoreBundle.php
file is no present either in 8.0.x or 8.1.x so patch doesn't apply anymore, where it is then? Is the patch still valid?Comment #37
kenorb CreditAttribution: kenorb commentedComment #38
kenorb CreditAttribution: kenorb commentedComment #39
EclipseGc CreditAttribution: EclipseGc at Acquia commentedWe should rebase what's in CTools if we're interested in doing this, it's far more tested both with literal tests and actual implementations.
Eclipse
Comment #40
kenorb CreditAttribution: kenorb commented@EclipseGc Thanks. I've checked what's in CTools, it looks more up-to-date, but I'm not sure how to use that Controller in practical way without any examples. Could you point to the direction which allows to use this CTools wizard code to create multipage steps (like multipage field_group in D7)? This is what I've end up with, but I'm not sure about the next step.
Comment #41
EclipseGc CreditAttribution: EclipseGc at Acquia commentedNo, you just define a route and use _wizard instead of _controller (or whatever) and then the class it references needs to implement the \Drupal\ctools\Wizard\FormWizardInterface
I would suggest extending \Drupal\ctools\Wizard\FormWizardBase if you just need to string some forms together or \Drupal\ctools\Wizard\EntityFormWizardBase if you want to make a wizard for generating new config entities. I'd suggest looking at the ctools_wizard_test module in ctools for a simple example you can emulate.
Eclipse
Comment #42
joachim CreditAttribution: joachim commentedA few things that maybe need a bit of a spruce up?
Not just Views!
Should these perhaps go via services rather than procedural code?
Comment goes over 80 chars.
Should be inheritdoc.
Comment #46
mparker17Straight re-roll for
8.5.x
; no changes so no interdiff needed.Comment #47
mparker17Addressed points 1, 3, 4 from @joachim's review in #42. Point #2 is still outstanding (hope to get time to work on it soon).
Comment #51
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedComment #59
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #60
Akhil Yadav CreditAttribution: Akhil Yadav commentedAdded patch against #47 in 10.1 version
Comment #61
Bhanu951 CreditAttribution: Bhanu951 as a volunteer commentedPatch in #60 doesn't contains the test present in #47. Hiding patch in #60.