Failing test for #5410#5411
Conversation
|
Thanks! Want to try implementing the fix you propose in #5410, so we can see if:
|
|
Do you want me to try to implement that fix? |
|
That's up to you. |
|
Sure. I'll give it a shot in the next couple of days. |
rvignesh89
force-pushed
the
validator-message
branch
2 times, most recently
from
a4704e8 to
a0a6c3b
Compare
|
I've pushed a draft version which passes the test case that I wrote earlier. Apart from the bug fix I made a simple change in DecimalField class to simplify the creation of MinValueValidator because I noticed string formatting was being done by the BaseValidator classes. If the changes are ok then I still need to do the following,
|
lovelydinosaur
left a comment
lovelydinosaur
left a comment
There was a problem hiding this comment.
Great stuff, thanks! I've made some comments that would need addressing.
| # rather than as a validator. | ||
| max_value = next(( | ||
| validator.limit_value for validator in validator_kwarg | ||
| max_value, messsage = next(( |
There was a problem hiding this comment.
Typo in messsage here.
| ), (None, None)) | ||
| if min_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES): | ||
| kwargs['min_value'] = min_value | ||
| kwargs['error_messages'] = {**kwargs['error_messages'], **{'min_value': messsage}} |
There was a problem hiding this comment.
We can't use that syntax, as it's not available in all the versions of python that we support.
| """ | ||
| Ensure validators can be explicitly removed.. | ||
| """ | ||
|
|
There was a problem hiding this comment.
Let's remove these unrelated styling changes in the pull request.
rvignesh89
force-pushed
the
validator-message
branch
from
a0a6c3b to
6334809
Compare
|
Still a WIP. Added 4 more tests for Have to still look into the following,
|
Specifically, what? |
|
Closing in favour of #5572 |
3 participants
Edited by @carltongibson
Closes #5410