Comments

timbrandin’s picture

Both problems seem to occur when we used ajax, which totally skips the validate function between the steps and also the submit function on the first step as it seems.

timbrandin’s picture

Both problems seem to occur when we used ajax, which totally skips the validate function between the steps and also the submit function on the first step as it seems. I was able to solve the situation by removing the ajax part of the next button.

But I've noticed that the problem also occur or is problematic for the field_groups module, but it does at least not hinder the user from finding the problem and solving it by them self. #1972678: Multipage form validation skipped.

timbrandin’s picture

Status: Active » Needs review
StatusFileSize
new1.19 KB

Here is a workaround that does not allow for field_group default behaviour when Ajaxify is turned off.

But it does not solve the issue when Ajax is used, it just solved the problem without the ajax turned on.

timbrandin’s picture

So that did not work, still need to check so the format is of multipage-group. Here's another try at it.

sinasalek’s picture

Status: Needs review » Postponed (maintainer needs more info)

Well the information you're providing is not enough unfortunately
I'm unable to reproduce the issue. And the patch actually disables the module

timbrandin’s picture

Version: 7.x-1.0-beta1 » 7.x-1.0-beta2
StatusFileSize
new787 bytes

Updated for beta2.

hypertext200’s picture

Status: Postponed (maintainer needs more info) » Needs work

This is also happened to me and none of the above patches helped.

Steps to reproduce:

  1. Create a form with few steps and add required fields to each step.
  2. Submit without entering a value for the required field, you'll see it will show up the validation error message.
  3. Now enter the value and try to submit, it will throw a Fatal error

PHP Fatal error: Cannot unset string offsets in /PATH TO DRUPAL/modules/field/field.default.inc on line 41

hypertext200’s picture

In my case address field was added in to the content type and removing address field and geocoder fields fixed the issue.

hypertext200’s picture

hypertext200’s picture

I just found this module doesn not work with any custom element, tested with Media Element, File Manages element, Address field element. It looks like the element process function does not invoke when submit the form. Hope this will help to resolve the issue.

katannshaw’s picture

I can verify that this issue happens for me as well, as noted on my duplicate post noted on #9 (sorry). Here's what happens in my case:

I just installed this module to work with a form I developed using the EntityForm and Field Group modules, so that I could get validation errors on each page of a multipage form. I enabled "Ajaxify" on the form, but unfortunately I received several AJAX errors when I went through the form to test it out.

When I went through the form while completing all required fields, it went through fine. But once I went through the form and purposefully didn't complete required fields to see what would happen, I started receiving this error:

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: /drupal/system/ajax
StatusText: Internal Server Error
ResponseText:

I then went to my error logs at admin/reports/dblog and found these different warnings:

Type php
Date Friday, October 11, 2013 - 09:16
User jayhawkfan75
Location http://mysite/drupal/system/ajax
Referrer	http://mysite/drupal/eform/submit/myform
Message	Warning: array_values() expects parameter 1 to be array, integer given in _field_filter_items() (line 513 of \mysite\drupal\modules\field\field.module).
Severity warning
Hostname xxx.xx.x.xxx
Operations

Type php
Date Friday, October 11, 2013 - 09:16
User jayhawkfan75
Location http://mysite/drupal/system/ajax
Referrer	http://mysite/drupal/eform/submit/myform
Message	Warning: Invalid argument supplied for foreach() in list_field_validate() (line 392 of \mysite\drupal\modules\field\modules\list\list.module).
Severity warning
Hostname xxx.xx.x.xxx
Operations

Type php
Date Friday, October 11, 2013 - 09:16
User jayhawkfan75
Location http://mysite/drupal/system/ajax
Referrer	http://mysite/drupal/admin/reports/event/793
Message	Notice: Undefined index: form_build_id in ajax_get_form() (line 320 of \mysite\drupal\includes\ajax.inc).
Severity notice
Hostname xxx.xx.x.xxx
Operations

Type ajax
Date Friday, October 11, 2013 - 09:16
User jayhawkfan75
Location http://mysite/drupal/system/ajax
Referrer	http://mysite/drupal/admin/reports/event/793
Message	Invalid form POST data.
Severity warning
Hostname xxx.xx.x.xxx
Operations

When I disabled "Ajaxify" on the form, it then went through and submitted fine without any of these errors. One thing that I did try was to raise the memory limit on my php.ini file, but that had no effect.

hypertext200’s picture

I'm having a little bit of luck by adding following lines to the hook_form_alter().. But I need to test a little bit deeper to confirm it, it's great to have another pair of eyes on that.

if (isset($form_state['node'])) {
  $form += _field_invoke_default('form', 'node', $form_state['node'], $form, $form_state);
}

Other thing I see in this module is this module should use #access => FALSE instead of unsetting the field values. I'll work around it to fix it.

katannshaw’s picture

@heshan.lk: On which line of the field_group_ajaxified_multipage.module file are you placing those lines of code?

hypertext200’s picture

Status: Needs work » Needs review
StatusFileSize
new7.58 KB

I cleaned up a little bit and added rebuilding the node object on validate as well as moved submit callback to the validate since this error occured in the validate. Removed unsetting field elements and added #access to disable it. Seems it works fine and I tried a lot of users cases and those were fine. Tested with Media and Addressfield elements too.

Patch attached.

hypertext200’s picture

This patch has lot more stuff and rolled up againest the dev branch.
1. Fixed the validation errors
2. Fixed using #access so form values are exists on hook_form_alter.
3. Added scroll top when ajax submit.
4. Lot of bug fixes and clean up.

katannshaw’s picture

I ended up installing Field Validation module and it's been working fine for me with my multipage entity form.

But I'll still test out this patch on my install to see if it works and I'll report those results soon. Thanks heshan.lk.

hypertext200’s picture

Thanks, let me know what your findings are so that I can push this to dev.

katannshaw’s picture

When I tried the patch:

  • I received this message: "The patch was partially applied."
  • On the site, the JS code appeared at the top of the page. When I checked the code, that JS code appeared at the top of the field_group_ajaxified_multipage.module file.
  • When I moved that code into its own file at js/field_group_ajaxified_multipage.js per the patch, the text at the top of the page went away.
  • I don't get any more error messages, but it does seem to have caused another issue noted below.

I noticed that with an already existing form, when I replaced a required form field's value with something else and click the "Next" button, the validation fires but the form doesn't collect the new value I've entered. It just gets stuck on that step. I have to revert back to the previous version using these setting with this module enabled for it to work properly again:

Non Javascript Multistep = Yes
Ajaxify = No

hypertext200’s picture

Version: 7.x-1.0-beta2 » 7.x-1.x-dev
Status: Needs review » Postponed (maintainer needs more info)

@jayhawkfan75, this patch is against the dev branch. Did you get any hunks those are rejected.

katannshaw’s picture

@heshan.lk: Sorry about that. The patch worked *great* against the dev branch. I'll definitely be using this module. I like the smooth transition from page to page. Nice touch.

One thing that I would note that got mentioned on this issue report for the Field Group module is that it may make more logical sense for some developers and users alike if the "Next step | Previous step" buttons were switched in order to "Previous step | Next step".

I'm still a newbie, so I don't know how to do patches as of yet. However I was able to accomplish this by switching the buttons placement on Line 323 of field_group_ajaxified_multipage.module:

    #--(Begin)--> Add navigation buttons
    if ($step < count($page_group['children'])) {
      $form['actions']['next'] = array(
        '#type' => 'submit',
        '#value' => t('Next step'),
        '#validate' => array('_field_group_ajaxified_multipage_form_validate'),
        '#ajax' => array(
          'wrapper' => $wrapper_id,
          'callback' => '_field_group_ajaxified_multipage_form_callback',
          'effect' => 'fade',
        ),
      );
    }
    if ($step > 1) {
      $form['actions']['prev'] = array(
        '#type' => 'submit',
        '#value' => t("Previous step"),
        // Since all info will be discarded, don't validate on 'prev'.
        '#limit_validation_errors' => array(),
        // #submit is required to use #limit_validation_errors
        '#submit' => array('_field_group_ajaxified_multipage_form_submit'),
        '#ajax' => array(
          'wrapper' => $wrapper_id,
          'callback' => '_field_group_ajaxified_multipage_form_callback',
        ),
      );
    }
    #--(End)--> Add navigation buttons

to this:

    #--(Begin)--> Add navigation buttons
    if ($step > 1) {
      $form['actions']['prev'] = array(
        '#type' => 'submit',
        '#value' => t("Previous step"),
        // Since all info will be discarded, don't validate on 'prev'.
        '#limit_validation_errors' => array(),
        // #submit is required to use #limit_validation_errors
        '#submit' => array('_field_group_ajaxified_multipage_form_submit'),
        '#ajax' => array(
          'wrapper' => $wrapper_id,
          'callback' => '_field_group_ajaxified_multipage_form_callback',
        ),
      );
    }
    
    if ($step < count($page_group['children'])) {
      $form['actions']['next'] = array(
        '#type' => 'submit',
        '#value' => t('Next step'),
        '#validate' => array('_field_group_ajaxified_multipage_form_validate'),
        '#ajax' => array(
          'wrapper' => $wrapper_id,
          'callback' => '_field_group_ajaxified_multipage_form_callback',
          'effect' => 'fade',
        ),
      );
    }
    #--(End)--> Add navigation buttons

...just in-case anyone's interested in this change. Thanks so much for your help with this heshan.lk!

UPDATE:
Another couple of issues that I just noticed with the dev version is...

Before applying the patch on the dev version, the validation fires for checkbox fields on other pages of my multipage form, so I cannot move to the next step. The data in the form fields also disappears when the validation fires.

After applying the patch, that validation issue is still there, but the data stays in the form fields as it should.

Reverting back to 7.x-1.0-beta2 made those bugs go away. If you need for me to test things further against my install, let me know. Sorry.

hypertext200’s picture

Thanks for reviewing the patch. Do this, after applying the patch change line 596 to below. Remove the #access = FALSE

      $form[$child_name] = array('#language' => NULL);
      // $form[$child_name]['#access'] = FALSE;
katannshaw’s picture

You're welcome heshan.lk. I gave it a try, and this is what happened with different settings for the multipage field:

Attempt #1:
Move additional settings to last multipage (if available): No
Non Javascript Multistep: No
Ajaxify: Yes
- Went through pages of form without a problem, but didn't collect any of the entered data.

Attempt #2:
Move additional settings to last multipage (if available): Yes
Non Javascript Multistep: No
Ajaxify: Yes
- When validation fires, entered data disappears from form fields.

Attempt #3:
Move additional settings to last multipage (if available): No
Non Javascript Multistep: Yes
Ajaxify: Yes
- Went through pages of form without a problem, but didn't collect any of the entered data.

Attempt #4:
Move additional settings to last multipage (if available): Yes
Non Javascript Multistep: Yes
Ajaxify: Yes
- When validation fires, entered data disappears from form fields.
- Won't move past step 1 because checkbox validation keeps firing even when it's checked.

When I changed Ajaxify > "No", it still didn't work properly, so I reverted back to the beta version. If there's another way that you'd like me to test the settings or anything else, please let me know. I appreciate your working on this.

hypertext200’s picture

Thanks for the feedback and the test cases. Additional settings is a strange issue and looks like some other module (field_group) take control over it so we don't get access to change it. I'll digg more deep in to this and see if I could find a solution. Thanks.

katannshaw’s picture

Very good. Thank *you*.

hypertext200’s picture

@jayhawkfan75, I was looking at much deeper in to the issue and found out that we don't need to rework on additional settings in this module, so after patch commented out the from "Remove non grouped fields in all other steps except the first one" to it's end. Because this should normally handle by a #prerender handler and field_group it self has that facility, and we can't just unset additional settings like we do in this module, we must move them to the related group.

So to be clear follow following steps.

  1. Patch the module.
  2. Comment or remove from "Remove non grouped fields in all other steps except the first one", Approx 248 to 265.

Then test. Looks like it fixed the all the issue we are having, that for the previous test it helps a lot to get me here.

8bitplateau’s picture

Issue summary: View changes

With the patch from #15 the added function "_field_group_ajaxified_multipage_field_unset" causes errors with required fields, or at least required booleans ( this is all i've tested )

In my case I had two steps, each with a required boolean.
Pressing next on step one causes the whole form to validate, this obviously shows form errors for both booleans, but you are still stuck on step one and cant get to step two to check the boolean.

This is because the form item attribute "#access" applied to the group children in the function does not play well with #required ie the item is still required just not rendered in the form.

I removed _field_group_ajaxified_multipage_field_unset entirely and in its place wrote

if ($n > $step) {
  foreach ($group['children'] as $key => $formitem) {
    $form[$formitem][LANGUAGE_NONE]['#required'] = FALSE;
  }
  field_group_hide_field_groups($form, array($group['name']));
}
if ($n < $step) {
  field_group_hide_field_groups($form, array($group['name']));
}

in the loop

   foreach ($groups as $group_name => $group) {
      if (in_array($group_name, $page_group['children'])) {
        $n++;
        $group['format_type'] = 'fieldset';
        if (isset($form['#groups'])) {
          if (is_object($form['#groups'][$group_name])) {
            $form['#groups'][$group_name]->format_type = $group['format_type'];
          }
          else {
            $form['#groups'][$group_name]['format_type'] = $group['format_type'];
          }
        }
        if ($n > $step) {
          foreach ($group['children'] as $key => $formitem) {
            $form[$formitem][LANGUAGE_NONE]['#required'] = FALSE;
          }
          field_group_hide_field_groups($form, array($group['name']));
        }
        if ($n < $step) {
          field_group_hide_field_groups($form, array($group['name']));
        }
      }
    }

in field_group_ajaxified_multipage_form_alter() function

it basically makes sure and required fields in future steps are NOT required, then hides the group, and simply hides the group in all past steps.

thijsvdanker’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new18.59 KB

I had validation errors and used the patch in #15 with the comment in #25 (commenting out some lines), and validation works!

To reproduce the bug I was experiencing: add a file field to one of your steps, and that step and all following steps will only validate once.

The attached patch is #15 applied against the master branch!! (as that branch contained 2 extra commits in comparison to 7.x-1.x) with the lines commented out as per #25.

Please review, proper validation is kind of critical I suppose.

ps: @digitisation (#26) I could not reproduce your problems with the boolean with this patch.

sinasalek’s picture

Thanks, i'll commit this if someone else confirm that it solves the problem

thijsvdanker’s picture

I've changed the validate function to check for other entity types then just node.

function _field_group_ajaxified_multipage_form_validate($form, &$form_state) {
  $entity_type = $form['#entity_type'];
  if (isset($form_state[$entity_type])) {
    // Update node object based on submitted form values.
    entity_form_submit_build_entity($entity_type, $form_state[$entity_type], $form, $form_state);
  }

@sinasalek, thanks. against what branch would you like to have a patch? Any good reason why master and 7.x-1.x have diverged?

thijsvdanker’s picture

And changed the code comment to reflect the change.

function _field_group_ajaxified_multipage_form_validate($form, &$form_state) {
  $entity_type = $form['#entity_type'];
  if (isset($form_state[$entity_type])) {
    // Update entity object based on submitted form values.
    entity_form_submit_build_entity($entity_type, $form_state[$entity_type], $form, $form_state);
  }
iniKim’s picture

@thijsvdanker: I reviewed your latest patch (https://www.drupal.org/node/1972616#comment-9506279) and everything works fine! Thanks Thijs!

hydra’s picture

Status: Needs review » Needs work

The patch in #30 worked for me on the master branch. Not sure how this is handled in this module, but normaly there should be no master branch and 7.x-1.x should contain the newest codebase.
Manual testing only in UI mode showed me, that this works, thx for fixing it!
A quick code review was showing tons of wrong code formatting. Also there are some bad comments and code which is not used anymore. I guess this should be clean upped before commiting. More infos about coding standards in drupal can be found here https://www.drupal.org/coding-standards

Below just some examples.

  1. +++ b/field_group_ajaxified_multipage.module
    @@ -85,13 +85,14 @@ function field_group_ajaxified_multipage_field_group_formatter_info() {
    -    $mode = $group->mode == 'form' ? 'form' : 'display';
    -    $formatter = $field_group_types[$mode][$group->format_type];
    +    $mode              = $group->mode == 'form' ? 'form' : 'display';
    +    $formatter         = $field_group_types[$mode][$group->format_type];
    
    @@ -112,7 +113,7 @@ function field_group_ajaxified_multipage_field_group_format_settings($group) {
    -      '#description' => t('If enabled and ajaxify option is disabled no javascript will be used for form submision or navigration between steps, the form will be refreshed. useful for debugging or very complex multistep forms')
    +      '#description' => t('If enabled and ajaxify option is disabled no javascript will be used for form submision or navigration between steps, the form will be refreshed. useful for debugging or very complex multistep forms'),
    
    @@ -121,15 +122,15 @@ function field_group_ajaxified_multipage_field_group_format_settings($group) {
    -      '#description' => t('If enabled navigation to next/prev pages will be done using ajax instead of simple javascript')
    +      '#description' => t('If enabled navigation to next/prev pages will be done using ajax instead of simple javascript'),
    

    Not sure if this code formatting stuff belongs to this issue

  2. +++ b/field_group_ajaxified_multipage.module
    @@ -244,22 +246,23 @@ function field_group_ajaxified_multipage_form_alter(&$form, &$form_state, $form_
    +//    if (($step > 1 && !$page_group['format_settings']['instance_settings']['move_additional']) || ($step < count($page_group['children']) && $page_group['format_settings']['instance_settings']['move_additional']) ) {
    +//      foreach ($form as $form_item => $form_item_name) {
    +//        $form['#field_group_ajaxified_multipage_complete_form'][$form_item] = &$form[$form_item];
    +//        if (is_array($form_item_name)) {
    +//          if (isset($form_item_name['#group'])) {
    +//            if ($form_item_name['#group'] == 'additional_settings') {
    +//              // locale_field_node_form_submit() looks for field language
    +//              // information in a hard-coded part of $form.
    +//              // @todo Other modules may as well, so would be best to move form
    +//              // elements within #pre_render rather than in hook_form_alter().
    +//              $form[$form_item] = array('#language' => NULL);
    +//            }
    +//          }
    +//        }
    +//      }
    +//    }
    

    This can be removed completly

  3. +++ b/field_group_ajaxified_multipage.module
    @@ -319,10 +325,11 @@ function field_group_ajaxified_multipage_form_alter(&$form, &$form_state, $form_
    +        '#validate' => array('_field_group_ajaxified_multipage_form_validate'), // nieuw
    

    Inline comments are bad style

  4. +++ b/field_group_ajaxified_multipage.module
    @@ -344,15 +351,16 @@ function field_group_ajaxified_multipage_form_alter(&$form, &$form_state, $form_
    +      // $form['#validate'] = array_merge(array('_field_group_ajaxified_multipage_form_validate') , $form['#validate']);
    ...
    +      // $form['actions']['submit']['#validate'] = array_merge(array('_field_group_ajaxified_multipage_form_validate') , $form['actions']['submit']['#validate']);
    

    This also looks like it could be removed

  5. +++ b/field_group_ajaxified_multipage.module
    @@ -384,30 +392,67 @@ function field_group_ajaxified_multipage_form_alter(&$form, &$form_state, $form_
    +      // nieuw
    

    This is no comment

sgurlt’s picture

I tried it now with the dev and the 1.0 beta 2 version but in both cases after applying the patch the form is submitted, but all values are lost. I am currently working on this

sgurlt’s picture

Status: Needs work » Needs review
StatusFileSize
new24.79 KB

I rerolled the patch #30. During it I had to refactor some code to get the submit running again.
In addition I adjusted as much code as possible to meet the drupal coding guidelines.

The patch is based on the version 7.x-1.0-beta2, I could not get it running with the latest dev version.

sgurlt’s picture

Missed one thing ...

sgurlt’s picture

And another reroll of that patch as the posted patch resultet in undefined indexes when placing groups in groups.

sgurlt’s picture

StatusFileSize
new25.16 KB

Just saw 3 minutes to late why the unset is at the bottom of the module file.
Instead of unsetting (which results in undefinied indexes, I hide the field groups which are not in the current steps.)

m.attar’s picture

Hi,
The solution you suggested provide problem if I try to save|update a node on click Next button using custom validate handler.
The problem that the node updated inside the handler validate, but if i reload the form I noticed nothing updated,
for details you can see my code below:

//add the callback in order to save the node
function HOOK_field_group_ajaxified_multipage_form_alter(&$form, &$form_state, $form_id) {
$form['actions']['next']['#validate'][] = 'company_node_form_save';
}

//the callback where i save node
function company_node_form_save($form, $form_state){
    node_form_submit($form, $form_state);
   dpm(node_load($form_state['node']->nid, null, true));//The node has been updated correctly
}

If i refreshed the form and tried to load the node(on the hook_form_alter()) I noticed that the node doesn't updated,
the data that saved by handler company_node_form_save() gone doesn't appear.

Can anybody help me out with this, or give me a way(handler) to update a node.

Thanks

sgurlt’s picture

Hey,
I think what are you looking for is not related to that issue. You should open a new one.
I will have a look at it tomorrow.

Greetings

m.attar’s picture

Thank you @sg88,

Before I applied your solution it was work properly.
I was used the handler submit for button Next to add my handler(company_node_form_save), explained in the following code:

//add the callback in order to save the node
function HOOK_field_group_ajaxified_multipage_form_alter(&$form, &$form_state, $form_id) {
$form['actions']['next']['#submit'][] = 'company_node_form_save';
}

And it work properly, but when I applied your solution I wont be able to use this handler, in addition if I add the handler submit for button Next still not working.

Thank you

sgurlt’s picture

Hm very strange, could you please try to use it with patch #30.
I just refactored it to meet the drupal coding standards and did some bug fixes inside it.

7thkey’s picture

sg88 patch #37 works for me, getting rid of this nasty fatal errors

m.attar’s picture

@sg88 Ok thank you

sgurlt’s picture

Thanks for the feedback, could you please adjust the issue status to tested? :)

m.attar’s picture

Assigned: Unassigned » m.attar
Status: Needs review » Reviewed & tested by the community

@sg88 you should remove the following code from your last patch(#37) because you repeated:

+    $hooks = array('field_group_ajaxified_multipage_form');
+    if (isset($form_state['build_info']['base_form_id'])) {
+      $hooks[] = 'form_' . $form_state['build_info']['base_form_id'];
+    }
+    $hooks[] = 'field_group_ajaxified_multipage_form_' . $form_id;
+    drupal_alter($hooks, $form, $form_state, $form_id);

Thanks

sgurlt’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new25.13 KB

Alright, thanks for the hint, during removing the code duplication, I removed even more "bad" code.

Please have a look on it again.

@attar_eweev: I just rechecked your posting, could you please try to get your values for your node save form $form_state['storage']['all'] and try again.

m.attar’s picture

Status: Needs review » Reviewed & tested by the community

@sg88 it work properly.

  • sg88 committed dcd2f6a on 7.x-1.x
    Issue #1972616 by sg88: Validation fails
    
sgurlt’s picture

Assigned: m.attar » Unassigned
Status: Reviewed & tested by the community » Closed (fixed)
sgurlt’s picture

Fixed a lot of stuff in the latest commits.
Dev branch is now up to date again.