When writing a custom validation, we need to use addError
method of yii\base\Model
(if we write it in model) or yii\validators\Validator
(if we write separate validator).
Both methods require $attribute
parameter (attribute name) to be passed, and as we can see from the sources, we can't leave it blank:
addError
of yii\base\Model
:
/**
* Adds a new error to the specified attribute.
* @param string $attribute attribute name
* @param string $error new error message
*/
public function addError($attribute, $error = '')
{
$this->_errors[$attribute][] = $error;
}
addError
of yii\validators\Validator
:
/**
* Adds an error about the specified attribute to the model object.
* This is a helper method that performs message selection and internationalization.
* @param \yii\base\Model $model the data model being validated
* @param string $attribute the attribute being validated
* @param string $message the error message
* @param array $params values for the placeholders in the error message
*/
public function addError($model, $attribute, $message, $params = [])
{
$params['attribute'] = $model->getAttributeLabel($attribute);
if (!isset($params['value'])) {
$value = $model->$attribute;
if (is_array($value)) {
$params['value'] = 'array()';
} elseif (is_object($value) && !method_exists($value, '__toString')) {
$params['value'] = '(object)';
} else {
$params['value'] = $value;
}
}
$model->addError($attribute, Yii::$app->getI18n()->format($message, $params, Yii::$app->language));
}
Possible options:
1) Select the most important relevant field and add error to it.
2) Select multiple important relevant fields and add the same error message to them (we can store and pass message in separate variable before passing to keep code DRY).
3) We can use not existing attribute name for adding error, let's say all
, because attribute existence is not checked at that point.
class YourForm extends \yii\base\Model
{
/**
* @inheritdoc
*/
public function rules()
{
return [
['name', 'yourCustomValidationMethod'],
];
}
/**
* @return boolean
*/
public function yourCustomValidationMethod()
{
// Perform custom validation here regardless of "name" attribute value and add error when needed
if (...) {
$this->addError('all', 'Your error message');
}
}
}
Note that we still need to attach validator to existing attribute(s) (otherwise exception will be thrown). We can use the most relevant attribute.
As a result, we will see error only in error summary. We can display error summary in form like that:
<?= $form->errorSummary($model) ?>
But in most cases there are always one or multiple attributes related with error so we can use option 1 or 2. Option 3 is kind of hack but I think it's pretty good workaround for that problem.
Please also see this related question and my answer on StackOverflow, basically the text above was taken from there.
Bottom line, I think we either need:
1) Add support for adding common validation errors. For example Django has support for that.
2) If you find implementing this ambiguous or not necessary because it goes against some framework philosophy or principles, I think the solution described in option 3 proves that validation errors implementation is a bit inconsistent in terms of passing attribute.
At least we can get rid of that.
Also according docs section would be helpful.
isnt this already possible as $model->addError() is not bound to attributes.
You can already put your own errorType in there. So its up to you to do the naming:
@dynasource As you can see from the sources it is coupled with attributes, but the existence of attribute is not checked. Besides that we still need to use existing attribute(s) to attach our custom validation rule. So I consider this as workaround only. And it's not documented. As the code written it's also hard to tell it's possible by the quick look. I discovered this workaround when answering this SO question I mentioned.
I know, otherwise I wouldnt be saying it, but $model->addError() should indeed get a $mixed argument.
I think it is a good idea. I've encountered the need for model-wide error not bound to attributes myself. Was choosing the most significant attribute in the case but logically these were general errors.
I am against such change: it breaks the model workflow.
Model
has been designed as a data storage, each atttribute of which can be acquired from user input and thus should be checked to be valid.
Consider following problems:
Model::validate()
accept list of attributes to be validated. How you can trigger your "model-wide" validation here?Validator::$skipOnError
, allowing to skip validation of attribute if it is already have an error, like there is no necessity to check if value is integer if it is empty. How will "model-wide" validator behave itself in this scope?Consider a common example of 'model-wide' validation: there are 2 fields at contact form: 'phone' and 'email'. At least one of them should be filled, but if one has been filled - the other one can be left blank.
If both fields are empty they both(!) should be highlighted indicating there is a error, should not they?
My opinoin PR #12829 overcomplicates existing code without much purpose.
Instead of creating validator, which will work without attribute, we should create validator, which handles several attributes!
There is already a method Validator::validateAttributes()
, which accepts list of attributes and can process them as batch.
You can easily solve the "model-wide" validation issue creating your own validator with custom validateAttributes()
implementation and assign it to all attributes:
class MyModel extends \yii\base\Model
{
public function rules()
{
return [
[$this->attributes, 'MyCustomModelWideValidator']
];
}
}
We can use not existing attribute name for adding error, let's say all, because attribute existence is not checked at that point.
I can not see anything bad in it: there should be some way to present your 'model-wide' error to the user - place error message to the particular place inside HTML page. You can use *
for it if you want - it whould be just #12829 solution, isn't it?
Method Model::validate() accept list of attributes to be validated. How you can trigger your "model-wide" validation here?
By not specifying attributes.
Validators are applied step by step. There is an option Validator::$skipOnError, allowing to skip validation of attribute if it is already have an error, like there is no necessity to check if value is integer if it is empty. How will "model-wide" validator behave itself in this scope?
It will ignore skipping, I guess.
What about user dialog? Usually error messages are bound to the input fields, so user can see where the error occurs, where "model-wide" error should appear?
In the summary only.
Consider a common example of 'model-wide' validation: there are 2 fields at contact form: 'phone' and 'email'. At least one of them should be filled, but if one has been filled - the other one can be left blank.
If both fields are empty they both(!) should be highlighted indicating there is a error, should not they?
Yes, they should. In this case it's not general error but two errors of two fields at the same time.
Here we're talking about errors such as "Comment can't be added because it's allowed to comment at night only". In this case there's no "created_at" field so no field to attach error to. It's general model-wide error.
By not specifying attributes.
Not specified attributes already means all attributes at Validator::validate()
. So there should be somethign else.
It will ignore skipping, I guess.
Ignore skipping by which condition?
In the summary only.
What if I want place error in particular place? If there is a general error it should appear somewhere at the form top (or bottom) without errors from other attributes with it.
Here we're talking about errors such as "Comment can't be added because it's allowed to comment at night only".
This is bad examle. If there is a limitation of form submitting per some business logic it is better to check it bofore form submission - and simply hide the form at the first place instead of allowing user to fill it up, submit and then find out that submission is not allowed.
I do not understand what is the problem with usage of non-existing attribute for 'model-wide' error?
For me, it is simple solution, which provide enough flexibility and does not provide any problems.
This issue is just about creating some magic error key like null
for 'model-wide' error messages. Why twist the code for it?
By the way null
may still work:
$model->addError(null, 'Model wide error message');
If not you can use *
or anything you like:
$model->addError('*', 'Model wide error message');
What is wrong with such construction?
I agree. Adding another dimension is a overcomplication. It should be leaner and meaner.
Hmm, indeed. @arogachev what do you think about ↑ ?
@klimov-paul It's not that simple.
1) First problem with multiple or all attributes approach is validator will run so many times as how many attributes we specified.
Let's say we want to validate 3 attributes and we defined the following validation rule:
[['attributeA', 'attributeB', 'attributeC'], 'customValidation'],
public function customValidation($model, $attribute)
{
if ($model->attributeA == 'x' && $model->attributeB == 'y' && $model->attributeC == 'z') {
$message = 'This combination is incorrect.';
$model->addError('attributeA', $message);
$model->addError('attributeB', $message);
$model->addError('attributeC', $message);
}
}
With default config it will work, because of skipOnError
option. It's false
by default and since attributes attributeB
and attributeC
also received error on the first run, validation will be skipped.
Let's say I don't want to add same error to all attributes separately or just want to have some general message in error summary:
public function customValidation($model, $attribute)
{
if ($model->attributeA == 'x' && $model->attributeB == 'y' && $model->attributeC == 'z') {
$message = 'This combination is incorrect.';
$model->addError('*', $message);
}
}
By the way now $attribute
is useless here because we check attributes together and can't split this validation.
This will run 3 times. The error summary displayed with ActiveForm
's errorSummary()
or yii\helpers\Html
errorSummary
will be correct because of this check:
if (array_search($line, $lines) === false) {
$lines[] = $line;
}
But the ActiveForm
's errors
array will contain duplicate error messages and if we want to use it we need to keep it in mind.
Specifying just one attribute solves the problem:
['attributeA', 'customValidation'],
But this doesn't feels right (especially when reading the code later) since all attributes participate in validation and there is no "more relevant" or "more important" attribute.
2) It's unclear how to add such common messages. I just discovered workaround I described in first post by trying different options. Again, even implementing this goes against framework philosophy or principles, this should be at least covered in the docs with examples. And I still think the solution I initially described is a bit "hacky" even it solves the problem.
3) We can't add validation rule for not existing attribute and can add error for not existing attribute. I think this is a bit inconsistent. And it's not limited to common errors. If I do a typo and write something like this:
$this->addError('attributeAA', 'Some error');
it will be perfectly valid and no exception will be thrown.
4) We can't control position of common errors in error summary. For example in Django it's always on top. Here it depends on the moment of adding error. If some errors for other attributes already exist, common errors can lost somewhere in the middle. Actually I think it's possible to place it on top by moving according validation rule to the very beginning of rules()
, but this can't go against some existing rules. This is ambiguous though.
1) First problem with multiple or all attributes approach is validator will run so many times as how many
attributes we specified.
It seems you do listen: just here I have told you you can use Validator::validateAttributes()
to run validation only once for all affected attributes.
2) It's unclear how to add such common messages. I just discovered workaround I described in first post by trying different options. This should be at least covered in the docs with examples.
This is not a common practice, that is why it is not explicitly mentioned as well countless other code usage possibilities. Still, I am fine adding the doc example for this.
3) We can't add validation rule for not existing attribute and can add error for not existing attribute. I think is a bit inconsistent. And it's not limited to common errors. If I do a typo and write something like this:
In theory this may happen. but it does not cause any problem so far, mainly because it is raw practice to add errors specifying attribute name as a string constant.
I can see no reason to add extra check for attribute existance, because it will cause performance degradation.
It seems you do listen: just here I have told you you can use Validator::validateAttributes() to run validation only once for all affected attributes.
But what if I want to use InlineValidator
like in example I showed (separate method or closure)? This won't work. So it works only in "specific" way.
This is not a common practice, that is why it is not explicitly mentioned as well countless other code usage possibilities. Still, I am fine adding the doc example for this.
Yes, it's not needed so often, but still used, especially in complex projects.
Rails has errors[:base] for it, Django allows to add it by raising ValidationError in clean() method.
So it's more stricted, not just some abstract not existing attribute.
By the way, current implementation idea was inspired by Django. I agree that we can't simply port that as is because there are a lot of differences and nuances in each framework. But I tried to adapt it according to that. And I'm not completely satisfied with this solution, especially with delegateValidation
and validateModel
methods.
Maybe we can find better solution or design decision?
But what if I want to use InlineValidator like in example I showed (separate method or closure)? This won't work.
We can easily create a new validator, for example BatchValidator
, which will use inline method or callback for validation in the similar way as InlineValidator
does, but work at validateAttributes()
method level instead of validateAttribute()
.
InlineValidator
uses validateAttribute()
level because it is needed more often.
I consider this matter to be a different issue, since it sounds like: 'ability to validate multiple attributes at once' instead of 'ability to validate without attribute at all'.
Rails has errors[:base] for it, Django allows to add it by raising ValidationError in clean() method... current implementation idea was inspired by Django
Sorry, but this is not an argument at all: there are countless features present at other frameworks (especially if look even behind the PHP to Python or Ruby), which are not implemented in Yii. In Yii we try to keep things simple.
It is hard for me to analyze examples from Ruby or Python, but errors[:base]
looks like usage of magick constant for the attribute name - this can be done now - you just need to define it yourself - can be null
, *
and so on. I can not see a problem in the fact framework does not predefines it for you,
Also keep in mind that Model
provides such methods as beforeValidate()
and afterValidate()
which can be used for 'model-wide' validation without solving the validator-to-attribute mapping problem.
@klimov-paul
Also keep in mind that Model provides such methods as beforeValidate() and afterValidate() which can be used for 'model-wide' validation without solving the validator-to-attribute mapping problem.
Yes, I thought about that, but keeping it with other rules is better in my opinion.
So you are OK with adding BatchValidator
but against adding storage for such "common" errors (this is up for user in this case)?
I'll try to rework the PR in this case, but not sure about client validation.
@arogachev thank you for active participation in Yii life, suggesting this idea and wise discussion.
After the @yiisoft/core-developers internal council, we decided to reject this feature request for the following reasons:
I'm closing this issue and corresponding PR, but not locking them.
Please, understand us. We want to keep framework simple therefore sometimes we have to reject potentially useful ideas.
@SilverFire OK. Can I add examples for it to the docs in this case?
Sure, it will be a good solution. Thank you for understanding
@SilverFire OK, I'll send a PR soon.
Created a PR #13005. Seems like reference is not displayed when issue is closed.
@arogachev github does not reference issue if you type the id in the title. if you type the ID in the PR description it will genereate a reference.
Most helpful comment
I am against such change: it breaks the model workflow.
Model
has been designed as a data storage, each atttribute of which can be acquired from user input and thus should be checked to be valid.Consider following problems:
Model::validate()
accept list of attributes to be validated. How you can trigger your "model-wide" validation here?Validator::$skipOnError
, allowing to skip validation of attribute if it is already have an error, like there is no necessity to check if value is integer if it is empty. How will "model-wide" validator behave itself in this scope?Consider a common example of 'model-wide' validation: there are 2 fields at contact form: 'phone' and 'email'. At least one of them should be filled, but if one has been filled - the other one can be left blank.
If both fields are empty they both(!) should be highlighted indicating there is a error, should not they?
My opinoin PR #12829 overcomplicates existing code without much purpose.
Instead of creating validator, which will work without attribute, we should create validator, which handles several attributes!
There is already a method
Validator::validateAttributes()
, which accepts list of attributes and can process them as batch.You can easily solve the "model-wide" validation issue creating your own validator with custom
validateAttributes()
implementation and assign it to all attributes:I can not see anything bad in it: there should be some way to present your 'model-wide' error to the user - place error message to the particular place inside HTML page. You can use
*
for it if you want - it whould be just #12829 solution, isn't it?