yiisoft / validator

Yii validator library
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
110 stars 36 forks source link

Include attribute name in error messages when it's present #644

Closed dood- closed 4 months ago

dood- commented 6 months ago
Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Fixed issues #630
codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 93.25843% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 94.72%. Comparing base (e019e5c) to head (43e9f45).

Files Patch % Lines
src/Rule/Count.php 0.00% 3 Missing :warning:
src/Rule/Length.php 0.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #644 +/- ## ============================================ + Coverage 94.60% 94.72% +0.12% - Complexity 796 801 +5 ============================================ Files 93 93 Lines 2426 2483 +57 ============================================ + Hits 2295 2352 +57 Misses 131 131 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

what-the-diff[bot] commented 6 months ago

PR Summary

Please note this is a high overview of the changes and not a granular breakdown.

dood- commented 6 months ago

Since I added a new placeholder {label}, perhaps it is better to make the placeholder {attribute} always be replaced by the name of the property?

samdark commented 6 months ago

@vjik, @arogachev what do you think?

samdark commented 5 months ago

No, I can't imagine such a case and I agree with @vjik. @dood- would you please adjust the name to be {attribute}? Thanks.

dood- commented 5 months ago

@samdark, @vjik If we leave "The value" as a default value, then there is a problem with displaying the attribute names of arrays.

Here is a test for an example: https://github.com/yiisoft/validator/blob/1e73321d83d197b0effb16e5d2c32a95d60913e4/tests/Rule/OneOfTest.php#L172-L180

We can not specify a Label attribute for the array or implement the LabelsProviderInterface, so the placeholder value will always be "The value"

vjik commented 5 months ago

If we leave "The value" as a default value

There will be no default value.

Will be one attribute {attribute}. Value of this placeholder by priority:

1) Translated label (if label exist). 2) Translated attribute.

For case with arrays always will be used translated attribute.

dood- commented 5 months ago

If necessary, I can remove Attribute::TARGET_CLASS from the Label attribute. I also fixed the error messages when using "StopOnError". There, instead of the attribute name, a number was displayed

samdark commented 5 months ago

If necessary, I can remove Attribute::TARGET_CLASS from the Label attribute.

Yes, makes sense.

dood- commented 5 months ago

Also need kill mutants from mutation testing.

Is it possible to disable mutants for a specific php version or just use @infection-ignore-all?

samdark commented 4 months ago

I guess @vjik meant fixing tests so there are no mutants, not ignoring these.

arogachev commented 4 months ago

Thanks for you great work! I will review this as well.

samdark commented 4 months ago

Thank you!