Required fields in step >1 break multistep

Here's a patch that fixes this issue for me. It uses hook_form_alter because it was the only way I could find to dynamically modify whether fields were required. The patch also does a check to make sure it actually is a multistep form which solved some warning messages I was getting.

Here's a better patch that doesn't introduce new warnings.

Now handles nested fieldgroups.

I applied your patch. It seems to work partially. Before applying the patch all the required fields after the first step were complained off. After apply patch only a few required fields are being complained off. I think now (after patch ) the fields being complained for non completion belong to the second step. I cannot see the step as it is hidden. I may have seen it but the Multistep Menu block is also not getting rendered in any region. I have created a separate issue for this at #1094982: Multistep Menu block not being rendered/shown in any theme any region - RTBC

I have several nested filed groups and fields and I am using The setting Revisions> Any step (Default) setting.

I am am now confirmed that it is complaining about the required field in the next step.

This issue makes multistep unusable as in most cases we have a few or more required fields. Marking it critical.

The Multistep Menu block menu hasn't worked at all for me either. I don't know why it's not working for you. It's working for me even though I have required fields on each step.

The patch in #3 worked for me in a simple test setup

The patch in #3 doesn't work when using date fields. Submitting a later form step (after the one with the date field) triggers a validation error :(

Actually, even when it's not required, date fields get emptied when saving another step. Same for booleans.

This issue make the module unusable for me to

i see that for some use cases this is important, others dont need it, so marking as major.

this is also very high on my list, but as i am very busy right now i can't tell when i come to it.
so patches or funding are welcome.

i think this should be implemented with #limit_validation_errors
if someone wants to hack it feel free to ask for mentoring.

Subscribing...

This issue makes this module useless as most forms have at least one required field.

Subscribing

Subscribe

I'd like to help, and a little mentoring would be great. I see an example of using #limit_validation_errors or chx blog here: http://drupal4hu.com/node/246 but from that I'm not sure where to start on your module. Which functions do you think I need to modify. I think maybe multistep_field_attach_form.

Ok, here is a patch. It is working for me but seems too simple, what a great job from the form api guys. Let me know if this needs anything else.

Its seems to solve my problem, can't believe it was only this. Thanks a lot !

@jackbravo:
good job how you get and pass #limit_validation_errors for the "next" button with functional elegance...

i see two issues with this before a commit:
* we need to handle all buttons, not only "prev" and "next"
* you set #limit_vaildation_errors to an empty array on "prev" button...
but prev button *does* save form data to the database and mark the step as "processed"
(i guess this *will* be an issue for some people that rely on the data as soon as the step is processed...)

so the "right thing to do" might be to attach #limit_vaildation_errors to the form element so it applies to any submission.

(i think your idea was to treat the "prev" submit button like "cancel" - if you need that you should implement that as a link, not a form submission.)

subscribing.

Right!

@axel.rutz As I started using my patch I saw those errors too and you are right, we should properly use the limit_validation_errors array on all buttons, not just next. Saving the form with done or save was giving me validation errors.

Here's another patch.

I must add that things are not working properly when you have nested fieldgroups inside the mulstistep fieldgroup. The fields inside the nested fieldgroup are not being saved, probably those fields are not being sent to the limit_validation_errors array, just the parent fieldgroup.

I need to investigate. Would that be an edge case?

This is another patch that allows you to save the information inside nested fieldgroups.

Without it, nested fields won't be saved when you move to the next page. BUT, even though it does save the value, it does not validate a required field properly: if you don't fill a required nested field, it won't complain. I'm not sure why this is.

@jackbravo:
i dont get your last sentence.
does #24 still have problems or solve all problems?

as for general, i have the feeling, what you do in _multistep_gather_fieldgroup_children() should in fact be done in field_group.module.
so created #1265756: #limit_validation_errors for fieldgroups

Is there no patch for the Drupal 6 version? How can this patch be applied to Drupal 6?

@bradbrad: raising prio without explanation just for personal taste is not considered good practice.
you need friends that solve your problems without paying them money? you will only lose them that way.
setting back to major as this affects some use cases, not others, and adjusting title.

>as for general, i have the feeling, what you do in _multistep_gather_fieldgroup_children() should in fact be done in field_group.module.
>so created #1265756: #limit_validation_errors for fieldgroups

this is now answered by maintainer that fieldgroups does not feel responsible, so the way for patch #24 is free now.
anyone like to test it? or even better: write tests ;-) ?

>@jackbravo:
>i dont get your last sentence.
>does #24 still have problems or solve all problems?

any answer for this?

It does not work, how did you check it?

Sorry for the late answer. #24 does solve problems when saving multistep forms. I have not tested since September. @pegah_m, did you tested #24 and it did not work? How it did not worked?

Yes I add patch in #24 . but when I press on next button, it load me to a page that shows an strange error.
I have nested field group and i use field collection also and i have required field in both steps!

Hmmmmm, the field collection could be adding some noise to the mix. Could you paste the error you get?

Thanks for your quick reply
It shows an empty page with this error:
EntityMalformedException: Missing bundle property on entity of type node. in entity_extract_ids() ( in line 7389 C:\wamp\www\radaahang\includes\common.inc)

I can confirm the problem of #32 (even though it states 'in line 7409' which, i wildly guess, is due to me having a younger version of drupal)

also, using a date field causes far more errors:
Warning: Invalid argument supplied for foreach() in date_combo_validate() when not needed field
Warning: date_timezone_set() expects parameter 1 to be DateTime, null given in date_combo_validate() a lot of those when needed (which seems only logical, since date_combo_validate() isn't set, however, it is (maybe) strange that he only complains about those when marking the date field as needed)

hope that helps,
alex

Any update on this?
Patch in #24 handles the validation correctly but when click to next step you get EntityMalformedException which breaks the system.

did a quick code review and found a flaw in _multistep_gather_fieldgroup_children()
i think it must go like this - anyone to reroll and test?
(_multistep_array_wrap() can be kicked out then)
(and the function _multistep_gather_fieldgroup_children() might get a better name...)

/**
 * recursively get all form elements of a given group
 */
function _multistep_gather_fieldgroup_children($form, $group) {
  $children = $form['#groups'][$group]->children;
  foreach ($children as $child) {
    if (in_array($child, array_keys($form['#groups']))) {
      $form_elements = array_merge($form_elements, _multistep_gather_fieldgroup_children($form, $child));
    } else {
      $form_elements[] = array($child);
    }
  }
  return $form_elements;
}

This issue has now been around for a year, is there a working patch available? Working without 'required' fields makes this module useless for most cases!

@yuri: So far it looks like multistep's current implementation is fundamentally flawed and no amount of hackery is truly going to fix it. It's smarter to use something else.

For a project at work we moved away to simple fieldgroups with horizontal tabs, with some custom javascript to provide a "next"/"previous" function.

I have bin using the module Field group with it's multipage functionality for quite some time now and I'm very happy with it.
http://drupal.org/project/field_group

to make the status of this issue precise:
the patch from #24 needs a reroll with the info from #35
the resulting patch needs to be tested
\me is quite involved so any help appreciated

The existing patch is the wrong way round.

He explicitly limits validation to only the fields which are known to be in the current step. However, there are many fields outside of that, for example all the hidden fields, additional settings and the like. The problem is that limit_validation_errors also limits what's in $form_state['values']. This causes the patch to fail hard if you e.g. have locale.module installed, which expects to be able to create a $node object from $form_state['values']. That is wrong anyway, but there are many other possible side effects of doing it like this. Like breaking all kinds of menu/path/... settings.

So, the only way that works is reverting the pattern. The attached patch now explicitly ignores all fields which are known to be on another step and limits validation to everything else.

I also had to move the logic into hook_form_alter() and increase the module weight to 50 (arbitrary number, but e.g. entity_translation is at 12 and breaks if it's at 10) to catch all fields added to the node form through hook_form_alter(). Another approach would be to implement hook_module_implements_alter() and force ourself at the end of the hook_form_alter() hook implementations, no matter what weight. But maybe someone does want to do something after us...

The advantage of this patch is that it will also allow to fix #1470482: Image/File fields lose their values when submitting another step with ~3 additional lines of code (throw an #access = FALSE on all excluded fields).

This is working on our test site, but nested groups within steps will probably actually not work correctly. We will need a way to include them. On the other side, the recursive part of dealing with child groups should actually be irrelevant now. Please test.

No need for [PATCH] in the issue title, that's what the status needs review/work is for...

patch on #40 made the publishing option and author fields ignored. I cant change them and the default values are saved. After removing the patch the options are working again.

berdirs comment and work completely make sense, but it seems this needs work.

It appears that when you go back to a previous step, any required fields on the current step will also throw up errors, as a user I would expect to be able to go back to a previous step without having to fill in the current step.

Possibly a patch to come.

That is currently by design and is not supposed to be fixed by this patch.

The Back button saves the current page and to do that, the fields need to be required.

agree with #45 so only needswork as of #42

Yeah.

So this additionally needs to be extended to recursively grab current form elements, so I added a new helper function to gather thase. Given the following conditions, to function works through the form elements recursively: a) #tree is not TRUE and b) there are child elements and the type is not radios/checkboxes (because they use sub-elements for the actual form elements, but this is then again merged together).

This seems to work for me fine for me, including nested fieldsets in multisteps and so on, please test.

Rerolled patch adding _multistep_gather_fieldgroup_children from #35 and a bug fix to prevent Notice: Undefined index: alias in path_node_update()error notice when using pathauto (see : http://drupal.org/node/1546128#comment-5921788)

Laurent

I'm having some difficulty getting this to work right. A lot of different patches are listed in this issue. Can anyone sum up which patches to install?

Thanks!

@mmagas You always should use the latest patch I think.

After applied #48 patch, it still showing some required field errors.

I received a series of repeated errors after applying the patch from #48. Below are the unique error messages.

Notice: Undefined variable: form_elements in _multistep_gather_fieldgroup_children() (line 902 of htdocs\sites\all\modules\contrib\multistep\multistep.module).

Warning: array_merge() [function.array-merge]: Argument #1 is not an array in _multistep_gather_fieldgroup_children() (line 902 of htdocs\sites\all\modules\contrib\multistep\multistep.module).

Warning: array_merge() [function.array-merge]: Argument #1 is not an array in _multistep_gather_excluded_fieldgroup_children() (line 445 of htdocs\sites\all\modules\contrib\multistep\multistep.module).

Warning: array_merge() [function.array-merge]: Argument #2 is not an array in _multistep_gather_child_fieldgroup_children() (line 463 of htdocs\sites\all\modules\contrib\multistep\multistep.module).

Warning: array_merge() [function.array-merge]: Argument #2 is not an array in _multistep_gather_excluded_fieldgroup_children() (line 445 of htdocs\sites\all\modules\contrib\multistep\multistep.module).

Can those of you that have problems with the patch from #48 try mine in #47? I'm not sure what #48 has exactly changed and my patch has worked for us.

Hi Berdir. I tried using your patch from #47 but im still hitting errors. Here's what I did...

• downloaded latest dev (2011-Jun-27)
• applied patch from #47
• received an error "Fatal error: Call to undefined function _multistep_gather_fieldgroup_children() in htdocs\sites\all\modules\contrib\multistep\multistep.module on line 463"
• copied and pasted the _multistep_gather_fieldgroup_children() function from #35
• received a series of errors like the ones that I posted on #52.

@berdir - here's #47 vs #48 - 48 adds a couple of extra functions.

chris.burgess@debbie:/tmp/multistep$ git diff 7.x-1.x-1065458-47..7.x-1.x-1065458-48
diff --git a/multistep.module b/multistep.module
index 5c065c7..8a96597 100644
--- a/multistep.module
+++ b/multistep.module
@@ -890,3 +890,26 @@ function multistep_views_api() {
     'api' => 2,
   );
 }
+
+
+/**
+ * recursively get all form elements of a given group
+ */
+function _multistep_gather_fieldgroup_children($form, $group) {
+  $children = $form['#groups'][$group]->children;
+  foreach ($children as $child) {
+    if (in_array($child, array_keys($form['#groups']))) {
+      $form_elements = array_merge($form_elements, _multistep_gather_fieldgroup_children($form, $child));
+    } else {
+      $form_elements[] = array($child);
+    }
+  }
+  return $form_elements;
+}
+
+/**
+ * Implements hook_node_presave().
+ */
+function multistep_node_presave($node) {
+  if (empty($node->path['alias'])) $node->path['alias'] = '';
+}

I'm testing out this (a colleague was using a modified version and I need to add a couple of his fixes once cleaned up). WIP @ https://gist.github.com/4340180

Slightly modified, but mostly as per #47. Adds an additional array check when gathering children because some groups (1) may not have any children at all, or (2) may have an empty string in $group->children. (I can't yet explain the latter, but I don't think it hurts to check a variable before iterating over it.)

This probably solves the "undefined $form_elements" from #52.

EDIT: Unintentionally left committer in submitted patch, please credit the person who wrote the bulk of it not me!

Just a simple reminder that } else { on the same line isn't according to the Drupal code style ... just a reminder! (don't shoot)

if (...) {
  ...
}
else {
  ...
}

I also glanced at it briefly and noticed the lack of some Drupal coding standards, so it's worth marking as Needs work for the minute.

Agreed. Thought I caught all of those, hoping to at least ensure the patch didn't introduce any new coding standards probs.

Looks like the module could use a cleanup generally, I removed a couple of coding standards fixes from my submitted patch because they were outside of the issue scope.

A bit better, but I left an Xmas treat for anyone who wants extra christmas credit for coding standards. From #47.

--- a/multistep.module
+++ b/multistep.module
@@ -386,6 +394,92 @@ function multistep_field_attach_form($entity_type, $entity, &$form, &$form_state
+    $children = count(element_children($element[$key])) > 0 && (!isset($element[$key]['#type']) || !in_array($element[$key]['#type'], array('radios', 'checkboxes')));

Before patch, 137 coding standards issues in multistep.module; after patch, 134. A net improvement, and I don't care enough to drop time on an overlong line right now, the weather's too good.

+++ b/multistep.moduleundefined
@@ -807,3 +901,27 @@ function multistep_views_api() {
+function multistep_node_presave($node) {
+  if (empty($node->path['alias'])) $node->path['alias'] = '';

Only one nitpick, again coding standards, this should really be:

  if (empty($node->path['alias'])) {
    $node->path['alias'] = '';
  }

Otherwise it all looks good. However that's only superficially, I haven't tested it, and unfortunately I ended up going with a much simplified approach for my UI not requiring any form of multistep, so I won't likely be testing this anytime soon, but hopefully someone else can do that aspect of the review.

applying the patch is ok, but i got this error

PHP Parse error: syntax error, unexpected $end in /var/www/mysite/sites/all/modules/multistep/multistep.module on line 928

Thanks @joeysantiago

The patch applied with no problems, thanks.

Now i get this error (yes, i have a date at the second step of my form):

Warning: date_offset_get() expects parameter 1 to be DateTime, null given in date_combo_validate()

patch #64 doesn't work where I have field group in the second step but #47
does work.

#47 works fine! Thanks!

I tried the patch in #64. It applied cleanly but I then got the following errors:

Warning: date_offset_get() expects parameter 1 to be DateTime, null given in date_combo_validate() 
Warning: date_format() expects parameter 1 to be DateTime, null given in date_combo_validate() 
Warning: date_timezone_set() expects parameter 1 to be DateTime, null given in date_combo_validate() 

I then tried the patch in #47, but this hasn't fixed it for me. I think it is because I have a required pop up date on the first page. I still get the original error A valid date is required for Event Start and End Date Start date. This seems to be related to Dates fields set to #access=FALSE are emptied on save.

I may give up on the multistep module. It is proving quite problematic.

Patch 47 and 48 work for me.

With patch 64 I get a required field error when submitting first page. The field is a List (text) radio on later step.

When testing each patch I uninstalled the multistep module first.

Patch #64 worked for me.

Patch #64 also worked for me.

Patch #47 works for me. Thanks.

Patch from #64 works for me. I've also manually applied it on top of another patch I've submitted to add support for any entity type in https://www.drupal.org/node/1078992#comment-10095582, patch attached. The new patch also skips any validation for the 'Previous' button - I'm in two minds about it, what are your thoughts? ie. should it be 'Back without saving' or 'Save and back'? I think some users would assume the first, in which case should we also remove the entity form submit for this button so possibly invalid values aren't saved to the entity?

Applied patch #47 works great. Fixedthe error for

Notice: Undefined variable: form_elements in _multistep_gather_fieldgroup_children() (line 902 of htdocs\sites\all\modules\contrib\multistep\multistep.module).