Yii2: CompareValidator and targetAttribute with error

[PowerGamer1]

Lets say a form has two inputs 'min', 'max' for numbers to specify some range and the corresponding model has the following rules:

['min', 'integer'],
['max', 'integer'],
['max', 'compare', 'operator' => '>=', 'compareAttribute' => 'min'],

When user enters invalid (for ex. some text) value into the 'min' input, the last rule will still perform validation using invalid value of 'min' attribute. This is wrong by itself and also will produce error message that does not make any sense (considering the 'min' input contains some invalid text that is not even a number): "max must be less than or equal to "min"."

I think CompareValidator must perform validation only when compareAttribute does not have errors. But then I am not sure how to treat skipOnError for this validator in this case.

[imxyb]

@PowerGamer1,hi,i think you can use the CompareValidator's attribute 'type',for ex:

['max', 'compare', 'operator' => '>=', 'compareAttribute' => 'min', 'type' => 'number'],

i try it and solve this problem.

[dynasource]

duplicate https://github.com/yiisoft/yii2/issues/12302

[PowerGamer1]

@dynasource
Setting type => 'number' does not solve the problem of inherently broken design of CompareValidator. The validator STILL performs validation using model attributes for which $model->hasError() is true.

And the wrong and confusing error message "max must be less than or equal to "min"." is STILL displayed when you enter text into "min" and negative number into "max" even when setting type => 'number'.

Needless to say type => 'number' will not help in any way when the values of min, max fields are supposed to be strings and not numbers.

Please reopen this issue.

[dynasource]

@PowerGamer1, can you point us to the source code in which you think something has to be changed?

[PowerGamer1]

@dynasource
If I knew how to fix it to retain BC I would submitted a PR. Unfortunately, in my opinion fixing is impossible without significant BC-breaking redesign of the validator and probably Model::validate() method too. In particular, validation design in Yii2 works well only for a SINGLE attribute onto which a validator is attached. If the validator needs to use any OTHER model attribute the problems I described here and in https://github.com/yiisoft/yii2/issues/12191 arise.

[dynasource]

@PowerGamer1, would you be willing to post a unit test here, proving your issue? This will speed up the fixing process

[PowerGamer1]

@dynasource
Ok, I'll try to explain it once more:

class TestModel extends \yii\base\Model
{
  public $min, $max;
  public function rules()
  {
    ['min', 'integer'],
    ['max', 'integer'],
    ['max', 'compare', 'operator' => '>=', 'compareAttribute' => 'min'], // 'type' => 'number' makes no difference
  }
}

// controller action:
$model = new TestModel();
$model->load(\Yii::$app->getRequest()->post());
$model->validate();
return $this->render('test.php', ['model' => $model]);

// view:
<?php $form = ActiveForm::begin(['method' => 'post', 'enableClientValidation' => false]); ?>
<?= $form->field($model, 'min')->textInput() ?>
<?= $form->field($model, 'max')->textInput() ?>
<?= Html::submitButton('Submit') ?>
<?php ActiveForm::end(); ?>

What to do: enter text into "min" and a negative number into "max" form inputs and submit the form.
Expected results: compare validator is SKIPPED ALTOGETHER and does not try to compare anything at all due to the fact that previous validator integer put an error on "min" attribute. No error is displayed on "max" input field on a form.
Actual results: compare validator tries to compare an INVALID value of "min" with a value of "max" and displays confusing and wrong error message on "max" input field.

[dynasource]

nice. This is way better. Thanx!

[Faryshta]

how about this

[
    ['min', 'max'],
    'compare',
    'targetAttribute' => 'max',
    'compareAttribute' => 'min',
    'operator' => '>=',
]

if validation fails then both min and max attribute get an error message.

[PowerGamer1]

I think I may have an idea of how to solve the issue with multi-attribute validators (i.e. validators using values of more than one attribute to perform validation, note that includes not only CompareValidator but also UniqueValidator, ExistValidator, etc). The skipOnError property of yii\validators\Validator class should have new meaning. skipOnError should accept an array of model attribute names. If any of the attributes in skipOnError have error the validator is skipped altogether. For backwards compatibility old possible values of skipOnError should be treated as follows:

Old value | New value
---------- | -----------
true | ['name-of-the-attribute-itself']
false | [] - empty array

Example:

['min', 'integer'],
['max', 'integer'],
['max', 'compare', 'operator' => '>=', 'compareAttribute' => 'min', 'skipOnError' => ['min', 'max']],

```PHP
['attr', 'integer', 'skipOnError' => true],
// equivalent to:
['attr', 'integer', 'skipOnError' => ['attr']],

['attr', 'integer', 'skipOnError' => false],
// equivalent to:
['attr', 'integer', 'skipOnError' => []],

[['attr1', 'attr2'], 'integer', 'skipOnError' => true],
// equivalent to:
['attr1', 'integer', 'skipOnError' => ['attr1']],
['attr2', 'integer', 'skipOnError' => ['attr2']],

The example from http://www.yiiframework.com/doc-2.0/guide-input-validation.html#multiple-attributes-validator would then correctly look like this:
```PHP
['childrenCount', 'validateChildrenFunds',
 'skipOnError' => ['personalSalary', 'spouseSalary', 'childrenCount'],
 'when' => function ($model) {
    return $model->childrenCount > 0;
}],

[samdark]

Sounds good.