fuel / core

Fuel PHP Framework - The core of the Fuel v1 framework
http://fuelphp.com
813 stars 345 forks source link

Custom validation error message #303

Closed dream4ge closed 13 years ago

dream4ge commented 13 years ago

This is an old issue, but still haven't been fixed in the RC3 (the bug report have been lost somehow).

The bug occurs when the method get_message from the Validation_Error class is used. In order to reproduce this issue, you will need to use a custom error message, because with the default message, the function returns earlier (line 102).

The docs say that it is possible to add a rule, giving the function name (i.e. 'valid_string') and some array of params to pass to this function. The problem is this array should be flattened before it is used with str_replace.

Let's try something like this, along with our custom error message :

$val = Validation::factory('login_user');
$val->add('username', 'Username') 
    ->add_rule('valid_string', array('alpha', 'numeric', 'dashes', 'dots'));

Now, the get_message() function of the Validation_Error class uses str_replace() to assign to each param a value. Which can eventually be an array (as shown in the example above), and produces an error.

As a temporary fix, I flattened the $params array before entering the foreach loop (line 113). We can achieve that by writing something like this :

//flatten the params array        
$this->params = $this->flatten_array($this->params);    //added line
foreach($this->params as $key => $val)
{
    $find[]       = ':param:'.($key + 1);
    $replace[]    = $val;
}

[...]

//added function
function flatten_array(array $array)
{
    $flat_array = array();
    foreach(new \RecursiveIteratorIterator(new \RecursiveArrayIterator($array)) as $value)
    {
        $flat_array[] = $value;
    }
    return $flat_array;
}

This implies modifying the core (see http://scrp.at/Uu) and was already discussed on the forum (http://fuelphp.com/forums/topics/view/1111).

Tell me what you think..

jschreuder commented 13 years ago

This has already been fixed in develop, though a lot easier as I don't really see the point of user-feedback with flattened arrays.

jschreuder commented 13 years ago

You did make me think though. I do believe your way will add a lot of unwanted overhead for most users, for example a Orm\Model instance with multiple relations and relations of those relations will take some time to flatten - while no one would ever use that output.
But that shouldn't disallow others from using it, without having to replace the entire get_message() method. I'll extract the replace bit to a new method which can be easily replaced by extending the Validation_Error class.

jschreuder commented 13 years ago

Here you go: https://github.com/fuel/core/compare/aa05766...d448ed62

Now you can make the replacement work however you want it to by extending the Validation_Error class.