CI Validation Improved (bug fixes and enhancements)

[eluser]Michael Ekoka[/eluser]
Hello fellow CodeIgniters, I'm finishing hacking through the CI Validation class to give it some punch. I need for people to try it and let me know of eventual bugs, questions, problems, etc. If you're already using the native validation class, replacing it by this one in your application/libraries/ folder should not affect your application. The native validation just had some limitations and flaws that I aimed at fixing for my own purpose. You are welcome to try it.

Amongst the functionalities that I added/fixed:

Ability to name your form fields using array syntax . e.g.

Code:

<input type = 'text' name = 'data[user_name]' />
<input type = 'password' name = 'password' />

...

// validation

...

$rules['data[user_name]'] = 'trim|required';
$rules['password'] = 'trim|required';

Unsubmitted, but required radios or checkboxes can now have individual error messages displaying, just like other fields.

People who've tried to output individual validation error messages for radio buttons or checkboxes know what this is about.

Ability to pass more than one argument to the validating functions

Code:

// validation
$rules['dob[day]'] = "trim[xyz]|callback_valid_date[dob[month],dob[year],long]";

i.e. In the above code the trim() function will receive the string 'xyz' as a second parameter. php.net : trim

Note that the validated field (dob[day]) will still be the first argument to the functions. The other arguments will be passed after it, in the order they appear in the list.
i.e.
trim($validation->dob['day'],'xyz');
valid_date($validation->dob['day'],$validation->dob['month'],$validation->dob['year'],'long');

Also, note that if an argument is in the validation fields list ($validation->fields[]), the value of the corresponding field is passed, otherwise the argument is passed as is.
i.e.
In the example above, the valid_date callback takes some arguments, amongst which 'dob[month]' and 'long'. If $validation->fields['dob[month]'] is set, the value of $validation->dob['month'] will be passed otherwise the string 'dob[month]' will be passed as is. If $validation->fields['long'] is set, the value of $validation->long will be passed otherwise, the string 'long' will be passed.

You can set individual error messages for fields even if they are part of an array.

Code:

// in form
<input type='checkbox' value='1' name = 'data[collect][field1]'/>
<?php echo $this->validation->data_collect_field1_error //individual error message ?>
<input type='checkbox' value='2' name = 'data[collect][field2]'/>
<?php echo $this->validation->data_collect_field2_error ?>
<input type='checkbox' value='3' name = 'data[collect][field3]'/>
<?php echo $this->validation->data_collect_field3_error ?>
<input type='checkbox' value='4' name = 'data[collect][field4]'/>
<?php echo $this->validation->data_collect_field4_error ?>
<input type='checkbox' value='5' name = 'data[collect][field5]'/>
<?php echo $this->validation->data_collect_field5_error ?>


// in the validation
// to set individual rules and errors
$rules['data[collect][field1]'] = 'required|callback_somerule';
$rules['data[collect][field2]'] = 'required|callback_somerule';
$rules['data[collect][field3]'] = 'required|callback_somerule';
$fields['data[collect][field1]'] = 'My Field 1';
$fields['data[collect][field2]'] = 'My Field 2';
$fields['data[collect][field3]'] = 'My Field 3';

You can also alternatively set one error message for all fields

Code:

// in form
<?php echo $this->validation->data_collect_error // group error message ?>


<input type='checkbox' value='1' name = 'data[collect][field1]'/>
<input type='checkbox' value='2' name = 'data[collect][field2]'/>
<input type='checkbox' value='3' name = 'data[collect][field3]'/>
<input type='checkbox' value='4' name = 'data[collect][field4]'/>
<input type='checkbox' value='5' name = 'data[collect][field5]'/>


// in validation

// to set rules and errors for the entire group
$rules['data[collect]'] = 'required|callback_somerule';
$fields['data[collect]'] = 'My Fields Group Name';

As well, to redisplay the submitted value after validation fails, just do what you usually do

Code:

<input type='text' name='data[user1][name]' value='<?= $this->validation->data['user1']['name']?>' />
<input type='text' name='data[user1][address]' value='<?= $this->validation->data['user1']['address']?>' />

This validation class is based on the native validation, but has been thoroughly modified in some parts to allow for the new functionalities. However, if you've used CI validation before, you should intuitively be able to work with this one. So for a newbie, the first step still involves reading the manual (CI validation). This is a work in progress and I'm trying to get rid of as many bugs as possible. So everyone is welcome to test this. Simply copy the file in your /application/libraries/ directory and name it MY_Validation.php. The framework should automatically load it instead of the native class.

Download the file and store in your application's libraries :
applicaton/libraries/MY_Validation.php. The framework should automatically load it instead of the original CI_Validation.php file.

MY_Validation.php

[eluser]Michael Ekoka[/eluser]
UPDATES
I’ve added the ability to know on which rule a field has failed validation.

e.g.

Code:

// 'username' rules
$rules['username'] = 'required|alpha|min[10]|max[12]|callback__duplicate';
...
// additional validation steps
...
// the 'username' validation has failed on rule:
echo $this->validation->error_flags['username'];

You could use this in many situations:

e.g. Overwriting an error messages for a particular field.

Code:

if($this->validation->error_flags['username'] == 'alpha'){
    $this->validation->username_error = 'Sorry, letters only for the username';
};

e.g. Aborting execution of a callbacks because a previous rule has already failed.

Code:

function callback__login(){
    // if the captcha fails validation, no need to try to login. Saves us a trip to the db.
    if(!empty($this->validation->error_flags['captcha'])){
        return;
    }
    // otherwise, connect to the db, check username, password, permission etc.
}

WARNING:
For reasons that are too long to explain, the 'required' rule is evaluated as an 'isset' for radio and checkbox fields (and so is the 'isset' rule itself). To check for "isset" or "required" validation failure on checkboxes or radios do something like this:

Code:

if($this->validation->error_flags['some_checkbox'] == 'isset / required'){
    $this->validation->some_checkbox_error = 'Please check the box';
}

All other field types will properly flag the ‘isset’ or ‘required’ rule in $error_flags

[eluser]mycroes[/eluser]
I came upon this post after reading a post from you in another topic (1). I think your design is seriously flawed. There's nothing wrong with passing multiple arguments to functions (I doubt the necissity, but it's a valid thing to want) but your idea with arrays seems really strange to me. What you'd actually want to do is have one rule for two fields, not three rules for two fields. As I posted in the other topic too, a nice way to solve this would be something like this:

Code:

$rules['identifier'] = "name(trim|required|exact_length[3])password(trim|required|md5)name(callback__test)";

Better actually would be:

Code:

$rules['identifier'] = "name(trim|required|exact_length[3])password(trim|required|md5)(callback__test)";

What this should do is run functions on name (trim, required, exact_length), then run funtions on password (trim, required and md5) and run the _test function (without parameters). The only thing that sucks here is that the _test function will still need to know where to find name and password, so a better option would perhaps be (callback__test[name, password]), which seems quite reasonable to me actually... I am certainly motivated to fix these issues in the validation code, but I think your way of fixing it with arrays is really a workaround/hack, not a fix. So if we can agree on this we might be able to make this a team effort ;-)
Regards,

Michael

[eluser]mycroes[/eluser]
- Forgot to point ot the other topic: 1: ellislab.com/forums/viewthread/64873/

- $rules['identifier'] needs a $message['identifier'] so the validator can still give usable errors. Since there's no one-on-one field/rule binding anymore this may require some code overhead, or identifying the error by field name as currently done, when possible, and identifying by function name if a function is called that doesn't apply to just one field. Acutally the latter seems like a nice option ;-)
Regards,

Michael

[eluser]Michael Ekoka[/eluser]
Hey mycroes, thanks for your comments. I appreciate the inputs.

I think your design is seriously flawed.

This isn't exactly my design. Remember that my intention wasn't to build a new Validation library, but to rather make the existing one more flexible. My aim is to allow people already comfortable with the existing library to intuitively work with my modifications. Yes, it is a hack in the sense that I modified the source to allow it to go beyond its limitations. I'm open to suggestions for alternate syntax, additions and other. However, I'm pretty sure that down the line we can probably find 10 people with 10 different ideas on how to do the exact same thing. The smart thing is to think about it first.

There’s nothing wrong with passing multiple arguments to functions (I doubt the necissity, but it’s a valid thing to want)

Some built in functions require more than 1 or 2 additional arguments to effectively do their job, others have additional/different behavior when receiving an optional parameter (lookup the trim function for example). If the validation does not allow you to pass additional arguments, you can't fully use these functions in your rules. Also, like you mentioned yourself a bit later in your post, callback__test[name,password] is a perfect example of why you would need to pass multiple arguments to a function.

but your idea with arrays seems really strange to me.

Again, this is to accommodate the current Validation syntax and logic. Form input arrays, are not the most common thing, but nonetheless, many people use this technique, as it is an allowed practice that has its own benefits. People coming from Rails for example are very comfortable with this practice. Sometimes you can't go around arrays (e.g. selectbox with multiple selections). I believe that the validation should be able to handle them as naturally as it handles normal fields, whether as a group or individually.

Better actually would be:

Code:

$rules['identifier'] = "name(trim|required|exact_length[3])password(trim|required|md5)(callback__test)";

Your point here is about rules that need to validate more than one field. Your proposal works very well indeed, but with the current modifications there are also many ways to accomplish the same thing:

Code:

// e.g. 1
$rules['login[user]'] = 'trim|required|exact_length[3]'; // <input name = 'login[user]' />
$rules['login[password]'] = 'trim|required|md5'; // <input = 'login[password]' />
$rules['login'] = 'callback__test'; // login is the name of the array containing both field values

// e.g. 2
$rules['user'] = 'trim|required|exact_length[3]'; // <input name = 'user' />
$rules['password'] = 'trim|required|md5'; // <input name = 'password' />
$rules['identifier'] = 'callback__test[user|password]'; // there are no 'identifier' field but you should still be able to create a rule with an arbitrary name
$fields['identifier'] = 'User Name & Password'; // just don't forget to set the field to be able to retrieve the error message

...


// e.g. 1
function _test($login){
    $user = $login['user'];
    $password = $login['password'];
}

// e.g. 2
// although there is no 'identifier' field, the function will still expect it as its first parameter
function _test($identifier=null,$user,$password){
    ...
}

All this should work fine with the modifications that I implemented. If you find that it's not the case, then it is definitely a bug that needs fixing on my part.


The author of the other thread that you've mentioned, talked about one important feature that I'm thinking of adding: a flag that would allow you to know which rule failed validation for which field. That way, rules validating more than one field (username and password for example) would know if they are required to run.

[eluser]mycroes[/eluser]
First of all, thanks for taking the time to write such a well-thought response. I might have sounded a bit harsh starting out with that I think your design is seriously flawed (as appears to me when I read it back now). I still think working with arrays is not really going to solve the problem of validating multiple fields in one run. As I said in my earlier post, it now requires three rules to validate two fields instead of one rule to validate both of them. I must say that your code is probably working without any problems at all, and for some people might do the job, but as you and I both said it's more of a hack.

Another problem that still remains with your code is that the validator doesn't promise an order of execution (or perhaps better, an order of evaluation). So you never know which rule will be tested first. I'll try to give the whole validation process a thoroough thought to see if there's a way to really improve it, so it really is usable for most people's problems. I actually think it'll do for most people right now, because you can always add your own checks, but I really like the idea of having a decent validator.

As a sidenote to some of your other comments (bit offtopic)... You were saying that for some variables you need to use an array as field name. I really dislike giving fields strange names, unless it really benefits code layout or ease of use. For instance, you were pointing out that if you were to select multiple items in a list you'd need to use an array. Another way of approaching this is to make a list of checkboxes. Actually, the checkbox approach is a lot easier to understand for a lot of people and can do almost anything you can do with a list. No need to use funky key-combos to select multiple items, only it's not possible to do the advanced list selection stuff like selecting the second item and holding shift and then clicking the last item, but that's really about the only thing that can't be done with checkboxes.

The same also goes for drop-down boxes. They're nice, but using radio-buttons is more efficient because the item you want to click is already visible on screen. Your mind is already able to interpolate your hand movement to get to the item, thus it's faster to use than when you first have to click the drop down box and then need to see where the item is you want to select. Well I'm going way off-topic now, so I'll stop here ;-)

Last but not least: I think the validator can really be improved a lot. It's not bad or anything, but it can't do some stuff, like it really can't do it in whatever way, as in passing 2 parameters. I for one don't really like your solution, but I haven't got anything better yet either, except for the ideas I showed in my post. I'll try to think of a better way to handle validation the next couple of days. Perhaps it really needs a different approach. Anyway, the discussion will feed the mind with new ideas ;-)
Regards,

Michael

[eluser]iive[/eluser]
Great. I am trying it in my application. I will feedback here if anything that I found. Keep the good work.

[eluser]Michael Ekoka[/eluser]

I might have sounded a bit harsh starting out with that I think your design is seriously flawed (as appears to me when I read it back now).

It's all good. A keyboard easily gives the impression of a harsher tone, but as long as you type with good intentions there's no real harm done.

I must say that your code is probably working without any problems at all, and for some people might do the job, but as you and I both said it's more of a hack.

Actually, I did say that I hacked through the current validation code to allow it to be more flexible. The validation itself doesn't stray from the current CI Validation philosophy, which itself is not a hack, but rather an approach.
I'm myself more comfortable with the 3 steps validation. I guess, this is really a matter of preference. The way I see it in the case of the login validation, it makes sense to separately validate form submission requirements and login requirements. In my mind the 3 steps have nothing to do with one another. On one end you want your fields to fulfill certain criteria upon submission and on the other you want your database to have a record corresponding to both entries. Keep in mind that I'm not trying to convince you, but merely explain my view.

Another problem that still remains with your code is that the validator doesn't promise an order of execution (or perhaps better, an order of evaluation). So you never know which rule will be tested first.

Again, I'm just the hacker trying to dope the CI Validation while maintaining backward compatibility. The improvements I made are intuitive because they fall right within the lines of what I thought would be the logical possibilities offered by the native library, until I hit a wall. I didn't like the idea of limiting my usage of forms because the Validation could not handle field arrays, so I tried to fix it. Your concerns are legitimate, but it may be that you don't wholeheartedly agree with certain aspects of the CI Validation approach itself. Maybe some things can be done differently, but this seems to be more a matter of preference. Maybe, at one point in the future we could work on a new validation library (you'll write the user guide ). For now, I aim at fixing certain limitations of the current library without deviating too much from the original intent.

Coming back to the order of evaluation, it is also possible that I'm more comfortable with this because I went through the validation class code and I understand exactly what it does. It is also true that this aspect has not been extensively documented. I'll try to share for those who have the same concerns as you do.

The validation class evaluates rules set in the order that they are given in the $rules array.

Code:

$rule['field1'] = 'rule1|rule2|required|rule4'; // this rules set will be evaluated 1st
$rule['field3'] = 'rule1|rule2|required|rule4'; // this will be 2nd
$rule['field2'] = 'rule1|rule2|rule3|isset'; // this will be 3rd

Within each rules set, the order that you enter them dictates which comes first, with some exceptions.
- If the field being evaluated does NOT have a 'required' rule AND does NOT have a callback AND is not set (or is null) or is an empty string: the validation skips to the next field in the array.
- If the field currently being validated is not set (or is null) (e.g. unchecked checkboxes) AND does have a 'required' or 'isset' rule, the validation will register the 'isset' error message right away without even running the other rules.
- If none of the two conditions above is met, the validation proceeds to evaluate each rule in the set in the order they have been listed for the field.
- If your field is evaluated against a callback rule and the method returns true, the validation skips to the next field (even if you have other rules coming after the callback) UNLESS you have a 'required' rule present in the list, in which case the validation will continue evaluating the next rules after the callback (if any). So, as a general practice, I'd recommend to put your callback rule at the end of the list.
In a few words this is it. Maybe I'm skipping some details. If you wanna know more, I invite you to open the Validation.php file and look for the run() method .

For instance, you were pointing out that if you were to select multiple items in a list you’d need to use an array. Another way of approaching this is to make a list of checkboxes.

This is more of a design and usability concern. This is the type of compromise you don't wanna have to make. I work with a team of designer and I would not want to tell them how to or not to design their forms because my validator only likes certain types of fields.

[eluser]mycroes[/eluser]
[quote author="CodeIgniter" date="1195172610"]

I must say that your code is probably working without any problems at all, and for some people might do the job, but as you and I both said it's more of a hack.

Actually, I did say that I hacked through the current validation code to allow it to be more flexible. The validation itself doesn't stray from the current CI Validation philosophy, which itself is not a hack, but rather an approach.[/quote]
In short, I think that the CI Validation is flawed and that your fix is a hack.
[quote author="CodeIgniter" date="1195172610"]I'm myself more comfortable with the 3 steps validation. I guess, this is really a matter of preference. The way I see it in the case of the login validation, it makes sense to separately validate form submission requirements and login requirements. In my mind the 3 steps have nothing to do with one another. On one end you want your fields to fulfill certain criteria upon submission and on the other you want your database to have a record corresponding to both entries. Keep in mind that I'm not trying to convince you, but merely explain my view.[/quote]
I (almost) always try to convince. I'm not trying to walk over you, but at this point some people may not describe me as a social person :-P

[quote author="CodeIgniter" date="1195172610"]

Another problem that still remains with your code is that the validator doesn't promise an order of execution (or perhaps better, an order of evaluation). So you never know which rule will be tested first.

Again, I'm just the hacker trying to dope the CI Validation while maintaining backward compatibility. The improvements I made are intuitive because they fall right within the lines of what I thought would be the logical possibilities offered by the library, until I hit a wall. I didn't like the idea of limiting my usage of forms because the Validation could not handle field arrays, so I tried to fix it.[/quote]
Your problem may very well be fixed, but your solution is still based on the flawed (IMHO) CI Validation. This is not your fault, it's not your job to fix it either (or is it?) and I'm not trying to say you should do. Actually my post should probably have been a topic on itself, because now I kind off highjacked your topic to talk about problems with the CI Validation itself.
[quote author="CodeIgniter" date="1195172610"]Your concerns are legitimate, but it may be that you don't wholeheartedly agree with certain aspects of the CI Validation approach itself. Maybe some things can be done differently, but this seems to be more a matter of preference. Maybe, at one point in the future we could work on a new validation library (you'll write the user guide ). For now, I aim at fixing certain limitations of the current library without deviating too much from the original intent.[/quote]
This problem needs a fresh touch, I think there must be a better way to do validation, but I don't know how either. I do want to write that documentation if we (or you or me) get to rewriting the validation library.

[quote author="CodeIgniter" date="1195172610"]Coming back to the order of evaluation, it is also possible that I'm more comfortable with this because I went through the validation class code and I understand exactly what it does. It is also true that this aspect has not been extensively documented. I'll try to share for those who have the same concerns as you do.[/quote]
Code does talk of course, but is this the intended behaviour of the code, or is it coincidence? As long as this kind off stuff isn't documented you should not rely on it. If it ever gets a code cleanup the order might be totally different, because as far as I can see there's no documentation stating this is the expected order of execution, and that's more troubling to me.

[quote author="CodeIgniter" date="1195172610"]The validation class evaluates rules set in the order that they are given in the $rules array.

Code:

$rule['field1'] = 'rule1|rule2|required|rule4'; // this rules set will be evaluated 1st
$rule['field3'] = 'rule1|rule2|required|rule4'; // this will be 2nd
$rule['field2'] = 'rule1|rule2|rule3|isset'; // this will be 3rd

Within each rules set, the order that you enter them dictates which comes first, with some exceptions.
- If the field being evaluated does NOT have a 'required' rule AND does NOT have a callback AND is not set (or is null) or is an empty string: the validation skips to the next field in the array.
- If the field currently being validated is not set (or is null) (e.g. unchecked checkboxes) AND does have a 'required' or 'isset' rule, the validation will register the 'isset' error message right away without even running the other rules.
- If none of the two conditions above is met, the validation proceeds to evaluate each rule in the set in the order they have been listed for the field.
- If your field is evaluated against a callback rule and the method returns true, the validation skips to the next field (even if you have other rules coming after the callback) UNLESS you have a 'required' rule present in the list, in which case the validation will continue evaluating the next rules after the callback (if any). So, as a general practice, I'd recommend to put your callback rule at the end of the list.
In a few words this is it. Maybe I'm skipping some details. If you wanna know more, I invite you to open the Validation.php file and look for the run() method .[/quote]
As stated above, is this coincidence or is this intended. Also there's quite some exceptions, not something you really want to think about even though for most people the exceptions won't occur...

Let me continue in the next post...

[eluser]mycroes[/eluser]
[quote author="CodeIgniter" date="1195172610"]

For instance, you were pointing out that if you were to select multiple items in a list you’d need to use an array. Another way of approaching this is to make a list of checkboxes.

This is more of a design and usability concern. This is the type of compromise you don't wanna have to make. I work with a team of designer and I would not want to tell them how to or not to design their forms because my validator only likes certain types of fields.[/quote]
You are most certainly right. I'd try to convince them of the better usability, but that's just my approach. This is actually exactly what's the problem with the current validation, it won't allow for the flexibility some people want...
Regards,

Michael