decoupling form validation from error reporting and "actions" OR different use case for DANGEROUS_SKIP_CHECK
I've a process, a series of forms. Available values in different steps depends on previously inserted data. Multi-step forms are not suited, and I don't think they could solve the problem I'm going to illustrate. I want bookmarkable steps and I want the customer care to be able to reproduce what the user see. I'd consider *possible* that a user open another browser window and change the previous choices while it is in the middle of the process. Since the forms are dynamically built, when a $form is "regenerated" for submission it may be different from the one that was shown to the user. At this point form validation kick in and says "An illegal choice has been detected..." Without DANGEROUS_SKIP_CHECK I have no choice to handle the problem graciously. 1) I may want to let the process come to an end and then check for coherency of the inserted values and then kick the user at the beginning of the process or 2) I may kick him at the beginning of the process as soon as I detect some problem. 3) I may do other stuff other than re-displaying the form with a general error message. Even with DANGEROUS_SKIP_CHECK I see no clear way to recycle the FAPI validation code and go with the 2) or 3) path. _validate accept $form and $form_values or $form_state, but I can't see any place where I could intercept the errors set by drupal_validate_form, form_validate, form_error... With DANGEROUS_SKIP_CHECK I can at least let FAPI ignore the errors and then _validate by my own and act accordingly to my needs That would imply some code duplication since I'll cut&paste part of FAPI validation and just change the form_error part. Without DANGEROUS_SKIP_CHECK I can't see any way to avoid to display general error messages or redirect the user somewhere else. I did read: http://drupal.org/node/182310 but I don't see any easy way to exploit #process. The input is actually not valid. I don't want to add options to make it valid, I just would like to control the message or the action taken for not valid input. I'd say that if the errors were available in _validate in a more structured way with default messages it would be easier to handle similar situations. If you want to obtain general case where default errors and behavior is OK, you just don't touch the "form_error" structure, otherwise you may inspect it or just unset some entries. when _form_validate and drupal_validate_form finish their job a) the user may have been redirected somewhere else with some custom _validate hook b) all "form_errors" left could be sent to drupal_set_message After all "general" validation get called anyway even if you've "custom" validation. I still think that "general" validation is still useful even if you're going to provide element validation functions that will "override" it. Most general and frequent case is "general" validation is enough and it doesn't seem worth to provide a switch that make it "optional" or "additional" to custom element validation. Anyway even when custom validation function kicks in even if they receive an $elements, form_error already set a message in a place where I don't see any easy way to do cleanup. Still when I'll have to jump from D5 to D7, I can't see any path to solve my problem. I can't see any way to control how a form react to invalid input if the validation happens in "core" FAPI part and not in _validate hooks. You'll have to inject stuff in the form to make it "valid" just to avoid to display general errors and then re-validate it in _validate hook. form_error may just build up a structure like $form_error['formstructureL1']['formstructureL2']...=array( '#errortype' => (token | required | maxlength | invalid) '#message => ); passed along with $form_status or just add to form_values aka form_status['value'] the above extra proprieties... The custom user validation function may do some cleanup or provide custom messages. This should break the coupling between validation, error reporting and in general action to be taken in case a form fail to validate. This will still provide a sane default that in spite of just disabling a check will force you to provide a _validate hook, where you could still... ignore the error and avoid to do validation... but well that does look as a much more conscious decision than just flipping a flag. This could be accomplished without killing form_error or form_set_error. Otherwise changing the order of FAPI validation and custom user validation in _form_validate could let people set the #validated propriety but it will force people to duplicate validation code in FAPI if they need it and won't break the coupling. Separating the validation from error reporting may even open a path to provide better logging or "intrusion detection". -- Ivan Sergio Borgonovo http://www.webthatworks.it
On Wed, 2008-05-21 at 11:44 +0200, Ivan Sergio Borgonovo wrote:
I'd consider *possible* that a user open another browser window and change the previous choices while it is in the middle of the process. Since the forms are dynamically built, when a $form is "regenerated" for submission it may be different from the one that was shown to the user.
At this point form validation kick in and says "An illegal choice has been detected..."
Hi. I encountered the same issue working with dependent drop downs filled by Ajax and JS code. Here there is the technique I'm using in one of my projects. Not sure it is the best approach, but it is the only one I was able to implement when I ported my code from D5 to D6. Hope this helps. Roberto *** function storm_dependent_select_process($form, $edit, $form_state, $complete_form) { unset($form['#needs_validation']); return $form; } $form['group1']['project_nid'] = array( '#type' => 'select', '#title' => t('Project'), '#default_value' => $node->project_nid, '#options' => array(0 => '-') + $projects, '#process' => array('storm_dependent_select_process'), '#required' => true, '#attributes' => array('onchange' => "stormtask_project_tasks(this, 'edit-task-nid', true, '-')"), );
damn, thats sneaky. nice one. On Wed, May 21, 2008 at 9:47 AM, Roberto Gerola <roberto.gerola@speedtech.it> wrote:
On Wed, 2008-05-21 at 11:44 +0200, Ivan Sergio Borgonovo wrote:
I'd consider *possible* that a user open another browser window and change the previous choices while it is in the middle of the process. Since the forms are dynamically built, when a $form is "regenerated" for submission it may be different from the one that was shown to the user.
At this point form validation kick in and says "An illegal choice has been detected..."
Hi. I encountered the same issue working with dependent drop downs filled by Ajax and JS code.
Here there is the technique I'm using in one of my projects. Not sure it is the best approach, but it is the only one I was able to implement when I ported my code from D5 to D6.
Hope this helps.
Roberto
***
function storm_dependent_select_process($form, $edit, $form_state, $complete_form) { unset($form['#needs_validation']); return $form; }
$form['group1']['project_nid'] = array( '#type' => 'select', '#title' => t('Project'), '#default_value' => $node->project_nid, '#options' => array(0 => '-') + $projects, '#process' => array('storm_dependent_select_process'), '#required' => true, '#attributes' => array('onchange' => "stormtask_project_tasks(this, 'edit-task-nid', true, '-')"), );
On Wed, 21 May 2008 10:05:24 -0400 "Moshe Weitzman" <weitzman@tejasa.com> wrote:
damn, thats sneaky. nice one.
It is. Just it "waste" FAPI validation and it's just a longer way to write DANGEROUS_SKIP_CHECK. What I actually did today for D5 was: - set DANGEROUS_SKIP_CHECK - really copy&paste the FAPI code into a _validate element hook - set custom messages there Still I've coupling and duplication of code. Roberto's technique doesn't improve the coupling problem and it skips the #required and #maxlength check too. Somehow set DANGEROUS_SKIP_CHECK looks better even if Roberto's solution becomes a smart *necessary workaround* in D6+. What about changing: function form_set_error($name = NULL, $message = '') { static $form = array(); if (isset($name) && !isset($form[$name])) { $form[$name] = $message; if ($message) { drupal_set_message($message, 'error'); } } return $form; } to function form_set_error($name = NULL, $message = null, $type=null) { static $form = array(); if (isset($name)) { if(isset($message)) { $form[$name] = Array('#type'=>$type, '#message'=>$message); } else { unset($form[$name]); } } return $form; } + function report_errors() { $errors=form_get_error(); if(is_array($errors)) { foreach($errors as $error) { if($error['message']) { drupal_set_message($error['message'],'error'); } } } } sort of then custom validate functions could access errors calling form_get_error[s]() and change them with form_set_error() After all form_set_error should be called during a form life cycle so... sooner or later drupal_process_form is going to be called so it could be possible to call the error reporting function just after drupal_validate_form get called without any breakage of existing code. -- Ivan Sergio Borgonovo http://www.webthatworks.it
On May 21, 2008, at 15:47 , Roberto Gerola wrote:
function storm_dependent_select_process($form, $edit, $form_state, $complete_form) { unset($form['#needs_validation']); return $form; }
I use #validated instead. This is more reliable, because it also skips user-defined validators. E.g.: $form['live_preview']['example'] = array( '#type' => 'hierarchical_select', '#title' => t('Preview'), // Skip all validation. '#validated' => TRUE, ); Wim Leers ~ http://wimleers.com/work
participants (4)
-
Ivan Sergio Borgonovo -
Moshe Weitzman -
Roberto Gerola -
Wim Leers