Part of the EllisLab Network
   
3 of 12
3
Who wants to test the new CI Form Validation class?
Posted: 27 August 2008 04:27 PM   [ Ignore ]   [ # 31 ]  
Administrator
Avatar
RankRank
Total Posts:  165
Joined  12-21-2001

steelaz—I don’t think we should use empty() there, because a numeric zero submitted will cause empty() to return TRUE.  That’s not a desired behavior.

I believe the bug is due to empty strings being set, rather then considered NULL.  Here’s my proposed fix:

Open Form_validation.php

At line 321, change this:

if (isset($_POST[$field])) 

To this:

if (isset($_POST[$field]) AND $_POST[$field] != ""
 Signature 
Profile
MSG
 
 
Posted: 27 August 2008 04:38 PM   [ Ignore ]   [ # 32 ]  
Administrator
Avatar
RankRank
Total Posts:  165
Joined  12-21-2001

xwero—I agree there should be a way to use a custom language file.

...actually, there already is.  You can load any language file you want, using:

$this->lang->load('file_name'); 

And the validation class should use it.  The one issue that I’ll have to test is whether doing so will let you override default array indexes, in the event of a naming collision.

 Signature 
Profile
MSG
 
 
Posted: 27 August 2008 06:20 PM   [ Ignore ]   [ # 33 ]  
Lab Assistant
RankRank
Total Posts:  172
Joined  01-24-2008

Very excited to have a more consistent validation library.  Big thanks to the CI team!

Things are working pretty well for me so far.  I’ve even updated Wireddesignz’s Callback in to Models extension so it will work with 1.7.

One thing I’ve noticed, does the new validator return the set_error_delimiters regardless of pass or fail?  I’m sending AJAX requests to the form validator and then scraping any errors off of the output.  My delimeter is set to , and if I enter a correct value in a field the AJAX call is scraping an empty .  On a standard page reload, no empty errors are reported and the controller flows as previously expected.

If that is by design, I can filter it out of my scrape.  If not, it still might be on my end, since I’ve only been playing with this for the last few hours.

Thanks again, CI crew.

Profile
 
 
Posted: 27 August 2008 06:47 PM   [ Ignore ]   [ # 34 ]  
Grad Student
Avatar
Rank
Total Posts:  83
Joined  04-25-2006

Another difference between the new form validation class and the old one: the new class doesn’t ignore empty rules. For example if the rules for a login form are setup like this:

$this->form_validation->set_rules('username''Username''');
$this->form_validation->set_rules('password''Password'''); 

and the form is submitted with empty fields, it will not validate ($this->from_validation->run() returns false). With the old validation class, a field with empty rules would always validate as true even if left empty.

I don’t know if this behavior is intentional or not, I just thought I’d point it out. I had gotten into the habit of listing all a form’s fields in set_rules() even if the field was totally optional.

 Signature 

My website: http://jamesgifford.com

Profile
 
 
Posted: 27 August 2008 07:02 PM   [ Ignore ]   [ # 35 ]  
Lab Assistant
RankRank
Total Posts:  172
Joined  01-24-2008
James Gifford - 27 August 2008 10:47 PM

Another difference between the new form validation class and the old one: the new class doesn’t ignore empty rules.

Won’t that affect form repopulation?  I used to set fields alone without rules, just because I wanted to repopulate the ungoverned fields as well as the ones that need validation.  Now, I’ve had to set rules across the board.  Perhaps this is too closely coupled?

Profile
 
 
Posted: 27 August 2008 07:38 PM   [ Ignore ]   [ # 36 ]  
Research Assistant
Avatar
RankRankRank
Total Posts:  549
Joined  07-28-2008
Rick Ellis - 27 August 2008 08:27 PM

steelaz—I don’t think we should use empty() there, because a numeric zero submitted will cause empty() to return TRUE.  That’s not a desired behavior.

I believe the bug is due to empty strings being set, rather then considered NULL.  Here’s my proposed fix:

Open Form_validation.php

At line 321, change this:

if (isset($_POST[$field])) 

To this:

if (isset($_POST[$field]) AND $_POST[$field] != ""

The above fix makes the form no longer use custom error messages as set by set_message(). It reverts back to the original, and as far as I can tell still throws an error on an empty field that is not required but has further validation rules if something is entered.

With fix in place:
“The last_name field must have a value.” displays

Without fix:
“Field Required” displays

My fix follows more of the line of steelaz. Can you foresee any issues with the following at line 483?

if ( ! in_array('required'$rulesTRUE) AND $postdata == ''

 

 

$fields = array(
                        array(
'field'     => 'last_name',
                              
'label'     => '',
                              
'rules'     => 'trim|htmlspecialchars|required|min_length[1]|max_length[50]'),

                        array(
'field'     => 'middle_initial',
                              
'label'     => '',
                              
'rules'     => 'trim|htmlspecialchars|exact_length[1]|alpha'),
// **SNIP**

$this->form_validation->set_rules($fields); 

Last name properly errors out with my fix in.
Middle Initial only shows an error if I enter a 1 or something non-alpha. If I enter nothing all is well.

 Signature 

~ 4 All the Right Reasons ~

Profile
 
 
Posted: 27 August 2008 07:44 PM   [ Ignore ]   [ # 37 ]  
Research Assistant
Avatar
RankRankRank
Total Posts:  549
Joined  07-28-2008
beemr - 27 August 2008 11:02 PM
James Gifford - 27 August 2008 10:47 PM

Another difference between the new form validation class and the old one: the new class doesn’t ignore empty rules.

Won’t that affect form repopulation?  I used to set fields alone without rules, just because I wanted to repopulate the ungoverned fields as well as the ones that need validation.  Now, I’ve had to set rules across the board.  Perhaps this is too closely coupled?

I agree. There could potentially be fields set without rules that could need re-population.

Is this a new intended behavior?

 Signature 

~ 4 All the Right Reasons ~

Profile
 
 
Posted: 28 August 2008 02:59 AM   [ Ignore ]   [ # 38 ]  
Sr. Research Associate
RankRankRankRankRank
Total Posts:  4785
Joined  07-14-2006

beemr & drewbee i think if you don’t want to validate fields they shouldn’t be known by the validation class.

I extended the form helper with this function

function value($key,$default '')
{
   
if(isset($_POST[$key]))
   
{
       
return form_prep($_POST[$key]);
   
}
   
else
   
{
       
return form_prep($default);
   
}

This is a better way to repopulate not validated fields i think.

Profile
 
 
Posted: 28 August 2008 03:17 AM   [ Ignore ]   [ # 39 ]  
Grad Student
Avatar
Rank
Total Posts:  83
Joined  04-25-2006
xwero - 28 August 2008 06:59 AM

beemr & drewbee i think if you don’t want to validate fields they shouldn’t be known by the validation class.

The problem with this idea is that then you have to handle non-validated fields differently. Its much easier and cleaner to treat all fields in the form in the same manner regardless of whether they need validating or not. This also makes it a simpler process to add rules to these fields in the future should the need arise.

This idea of field population is somewhat related to another issue I’m having with the new validation class. Sometimes I like to use the same form for both adding and editing records. This means that such forms sometimes need to start out populated with data. Before, I would do something like this:

foreach ($this->validation->_fields as $field => $value{
    $this
->validation->$field $record[$field]

which would pre-load the form values into the validation class. I know this deviates a bit from the intended usage of the class, but it was quite convenient. With the new form validation class, I can’t find a way to pre-populate the data before the form is submitted. I know that the set_value() helper function has an option for a default value, but this is problematic for those times when the form is used to add a new record rather than edit an existing one.

Does anyone have any tips for validating a form that is used both for adding and editing?

EDIT: I just noticed that the reason I can’t use this same technique with the new form validation class is that the first few lines of the set_rules() function skip setting the field data if there is no $_POST data. I removed lines 72-76 of form_validation.php and my technique worked fine. I know it helps improve performance and all and that my needs are no doubt in the minority, but I for one would like this feature removed.

 Signature 

My website: http://jamesgifford.com

Profile
 
 
Posted: 28 August 2008 03:30 AM   [ Ignore ]   [ # 40 ]  
Sr. Research Associate
RankRankRankRankRank
Total Posts:  4785
Joined  07-14-2006
Rick Ellis - 27 August 2008 08:38 PM

xwero—I agree there should be a way to use a custom language file.

...actually, there already is.  You can load any language file you want, using:

$this->lang->load('file_name'); 

And the validation class should use it.  The one issue that I’ll have to test is whether doing so will let you override default array indexes, in the event of a naming collision.

The biggest problem i see with the fieldnames is putting them in a config file.
My first idea of a hack is to load a field label file and in the config file do

$config = array(
   array(
'username',$this->lang->line('username'),'required')
); 

But it feels a bit creepy.

Profile
 
 
Posted: 28 August 2008 04:05 AM   [ Ignore ]   [ # 41 ]  
Grad Student
Avatar
Rank
Total Posts:  83
Joined  04-25-2006

One more difference I’ve found: the default value for an empty field is now NULL instead of ‘’ (empty string).

On line 141 of form_validation.php, the postdata value of the _field_data array for each field is set to NULL by default. This is the value returned if no data was entered for that field in the form. This is also the value returned by set_value() even when the default value is ‘’.

I’m not sure if this makes much difference for most people. I noticed it when storing form data in a table with fields marked as not null. Before, the empty fields caused no problems, but now a database error is thrown.

 Signature 

My website: http://jamesgifford.com

Profile
 
 
Posted: 28 August 2008 04:29 AM   [ Ignore ]   [ # 42 ]  
Sr. Research Associate
RankRankRankRankRank
Total Posts:  4785
Joined  07-14-2006
James Gifford - 28 August 2008 07:17 AM

Does anyone have any tips for validating a form that is used both for adding and editing?

The set_value function has a second parameter to set the default or in your case the preloaded value. In that way it’s similar to my function.

As for your repopulation point of view i think it also should be handled by the set_value function, the one in the Form_validation should have the same check as the one in the form_helper IMO.

// current code in the library
function set_value($field ''$default '')
    
{
        
if ( ! isset($this->_field_data[$field]))
        
{
            
return $default;
        
}

        
return $this->_field_data[$field]['postdata'];
    
}
// suggested change
function set_value($field ''$default '')
    
{
        
if ( ! isset($this->_field_data[$field]))
        
{
            
if ( ! isset($_POST[$field]))
            
{
                
return $default;
            
}

            
return $_POST[$field];
        
}

        
return $this->_field_data[$field]['postdata'];
    

This way you can add set_value even if the field isn’t in the rules. This way all values are handled with the same function.

Profile
 
 
Posted: 28 August 2008 04:49 AM   [ Ignore ]   [ # 43 ]  
Grad Student
Avatar
Rank
Total Posts:  83
Joined  04-25-2006
xwero - 28 August 2008 08:29 AM

The set_value function has a second parameter to set the default or in your case the preloaded value. In that way it’s similar to my function.

But what do I do if that default value isn’t always available? In my add/edit combo form, I can only get the default values in edit mode. If I fill an array with the record’s data and use the elements of this array in the set_value() calls for each field, it works great. But when adding a new record I’d have to have an array with the same fields, but with empty values. I’m sure this solution is feasible it just seems less efficient and/or messier than my old way of doing it.

 Signature 

My website: http://jamesgifford.com

Profile
 
 
Posted: 28 August 2008 05:01 AM   [ Ignore ]   [ # 44 ]  
Sr. Research Associate
RankRankRankRankRank
Total Posts:  4785
Joined  07-14-2006

EDIT : bad solution removed

I don’t see how setting an array with empty strings is more messier than setting up fields you don’t validate and then use those fields to set empty strings?

$field1 $field2 '';
$data['fields'compact($field1,$field2);
// or in php5 only
$data['fields'array_fill_keys(array('field1','$field2'),''); 
Profile
 
 
Posted: 28 August 2008 05:05 AM   [ Ignore ]   [ # 45 ]  
Grad Student
Avatar
Rank
Total Posts:  83
Joined  04-25-2006

xwero, I like your last set_value() example. It seems to cover all the bases and would allow for clean controllers and view files. I think this would be a great change to the new form validation class.

EDIT: Well, you took it down, but I like the idea of the set_value() helper function working regardless of validation rules and nonexistant default values.

 Signature 

My website: http://jamesgifford.com

Profile
 
 
   
3 of 12
3
 
‹‹ first_row, last_row      Nice Dates ››