Custom Validation Callbacks "required" by default?

[eluser]Unknown[/eluser]
The following is near the top of the _execute method of the Form_validation.php library file.

Code:

// If the field is blank, but NOT required, no further tests are necessary
$callback = FALSE;
if ( ! in_array('required', $rules) AND is_null($postdata))
{
// Before we bail out, does the rule contain a callback?
if (preg_match("/(callback_\w+(\[.*?\])?)/", implode(' ', $rules), $match))
{
  $callback = TRUE;
  $rules = (array('1' => $match[1]));
}
else
{
  return;
}
}

This logic dictates that ALL callback validation function are required. It also means that there is no way to have non-required callback validation functions. So the following two statements therefore are identical.

Code:

$this->form_validation->set_rules('question_1', 'Question 1', 'callback__q1_check');
$this->form_validation->set_rules('question_1', 'Question 1', 'required|callback__q1_check');

Why should callback function be exempt from the possibility of not being required? You end up resorting to wrapping your rule setups in conditionals like the following in order to allow for the possibility of non-required callbacks.

Code:

if (!is_null($this->input->post('question_1')))
{
  $this->form_validation->set_rules('question_1', 'Question 1', 'callback__q1_check');
}

This is both non-intuitive when all the native function validation respects the absence of required and ugly.

The change is obviously very easy to make, but I also realize this will probably break a ton of applications who either don't realize that required is implicit or never thought about it. Am I the only one who finds this weird and a pain to deal with?

[eluser]Aken[/eluser]
I think in most instances when a callback is used, chances are the field is going to be required. Perhaps that's the same assumption that the original developers had as well.

As for "no way to have non-required callback validation functions", you're overlooking a very simple solution:

Code:

public function _ql_check($value)
{
// If the value is empty, return TRUE,
// since it is not required.
if ($value === '')
{
  return TRUE;
}

// Continue with your validation here if a value exists...
}

It's a valid argument, though - I can certainly see your point. I'd suggest creating an Issue on GitHub, where you're more likely to get discussions relating to changes like this.

[eluser]CroNiX[/eluser]
Or just extend the form validation class with your own rules and avoid callbacks altogether

[eluser]skunkbad[/eluser]
I never liked the callbacks being required either. I guess there's always something...

[eluser]Unknown[/eluser]
The following code does allow you to make the validation non-required within the callback itself.

Code:

if ($value === '')
{
  return TRUE;
}

But not within the setting of the rules where you are normally afforded the opportunity to easily declare required vs non-required. For instance. On the very project I'm working on tonight. There are two different forms that both contain a phone number field. The phone is required on one but not on the other. By respecting the "required" or absence of "required" you can make the determination in the controller. If you use the code above, you no longer get to choose from the controller.

Your also right, I can create a quick patch and request a pull. But I was kind of gauging if this was even worth fighting for. I obviously think it should be changed but if I'm alone I'll just leave the change in my own applications and not bother wasting the core devs time.

[eluser]Aken[/eluser]
[quote author="EricStewart" date="1342663376"]But not within the setting of the rules where you are normally afforded the opportunity to easily declare required vs non-required. ... By respecting the "required" or absence of "required" you can make the determination in the controller. If you use the code above, you no longer get to choose from the controller.[/quote]
If you include the empty string check in your callbacks, you can still make the required or not determination from the controller. Just be sure to put the required rule first. If a field fails the required rule, any further validation for that field stops, and the callback will not fire. If you do not require the field, the callback will fire, and will validate to TRUE if empty.

Your also right, I can create a quick patch and request a pull. But I was kind of gauging if this was even worth fighting for. I obviously think it should be changed but if I'm alone I'll just leave the change in my own applications and not bother wasting the core devs time.

Creating a pull request and asking for opinions is the best way to create discussion amongst those who care the most about CI's development. If you don't want to take the time to do the actual pull request, just create an issue by itself. Or if you don't want to, I can.

I personally think it's a good idea - callbacks should work just the same as, say, min_length, in that it is not checked if the field is empty and not required. The main discussion would be about how it affects existing applications, and if it's worth including (subsequently resulting in broken apps if not updated).

[eluser]skunkbad[/eluser]
Using something like this:

http://ellislab.com/forums/viewthread/205469/

You could set a rule like this:

Code:

callback_external_callbacks[some_model,some_method,some_string,another_string]

If "some_string" or "another_string" lets the callback know if the field is required or not, it is easy to test inside the callback, and setting "required" in the set of rules is no longer necessary. I do this with _check_password_strength() in Community Auth's form validation callbacks model.

[eluser]educator[/eluser]
I use your external callbacks method but I have a problem with empty fields. It seems that the method does not receive the destination model and method and therefore can't dispatch.

Here is the first line of your function followed by a debug line:

Code:

function external_callbacks( $postdata, $param )
{echo 'postdata='.$postdata.'<br>';print_r($param);echo '<br>';

The following results show it working properly for fields on the top of the form but not the last one which is blank. Not only is $postdata empty but what follows is an error because the $param array is also empty.

Code:

postdata=W1299999
formval_callbacks,string_pattern_check,/^[fwsFWS]{1}[0-9]{2}\w{5}$/
postdata=W1299999
formval_callbacks,existence_check,evals,eval_id,y
postdata=W1299999
formval_callbacks,tcase,u
postdata=nmat
formval_callbacks,existence_check,masters,id,y
postdata=nmat
formval_callbacks,tcase,l
postdata=

There are things I would like the callback to do even if the field is empty.

Have I missed something?

[eluser]CroNiX[/eluser]
I believe this is a limitation of the default validation class. If used, all fields must have a rule even if the rule is "required" or "trim", or I think even no rule like "" will work. Just as long as set_rules() was run on each field. Or else those fields aren't run through validation so they won't repopulate upon failure.