Conversation
dybi
requested review from
Karrp,
MartynaAnnaGottschling,
Szymiks,
kbeker,
maciejSamerdak and
rwrzesien
|
@dybi Also please remember when you are creating PR, please add milestone, projects, label and assign yourself to this PR. Same thing please do with issue which is related to this PR. :) |
|
|
||
| def setUp(self): | ||
|
|
||
| super().setUp() |
| @@ -1,46 +1,44 @@ | |||
| # pylint: disable=line-too-long | |||
| from django.utils.translation import ugettext_lazy | |||
| from django.utils.translation import ugettext_lazy as _ | |||
users/serializers.py
Outdated
| raise serializers.ValidationError(CustomValidationErrorText.VALIDATION_ERROR_SIGNUP_EMAIL_MESSAGE) | ||
| return email | ||
|
|
||
| def validate_date_of_birth(self, date_of_birth: datetime.date) -> datetime.date: # pylint: disable=no-self-use |
There was a problem hiding this comment.
This should be done directly on CustomUser.date_of_birth field as validators=[MaxValidator(), MinValidator] (https://docs.djangoproject.com/en/2.2/ref/validators/#maxvaluevalidator). This way we control it on lowest possible level in the app and don't have to worry about it anywhere else. If for some reason it won't work there, then clean() method of the model. The case is that we want to remove all remaining of django-rest-framework so this serializer will be removed soon, and we shouldn't loose this logic.
There was a problem hiding this comment.
@rwrzesien Ok, I will move it, but ther's a problem with MinValueValidator / MaxValueValidator. Namely, the min and max value are not set in stone, they depend on the current date (for example, if yesterday user was not 18 years old validation should rasie exception, but if today user turned 18, it should pass).
I will create a custom validator and use it in model.
|
@dybi Looks good, please just move the check to model instead of serializer, as it will be removed soon. |
dybi
force-pushed
the
bugfix-user-birth-day-has-no-limit
branch
from
5e47c0a to
fbde5a7
Compare
- introduce setup - reduce code duplication - remove unnecessary / unused code - improve asserts (add descriptive message)
- use `_` as shortcut for `ugettext_lazy - add `VALIDATION_ERROR_AGE_NOT_ACCEPTED`
- delete user's age validation from serializers - add `UserAgeValidator` to `CustomUser` model for `date_of_birth` field - adapt `UserAgeValidator` model's tests
dybi
force-pushed
the
bugfix-user-birth-day-has-no-limit
branch
from
fbde5a7 to
25cc3bf
Compare
3 participants
resolves:
#225