Conversation
maciejSamerdak
requested review from
Karrp,
MartynaAnnaGottschling,
Szymiks,
dybi,
kbeker and
rwrzesien
|
@maciejSamerdak mea culpa for deleting branch which was based of this PR :P |
|
Ok, now it's back to it's last state |
|
@kbeker I shalt not judge thy sins, save it for thy Creator! |
dybi
left a comment
dybi
left a comment
There was a problem hiding this comment.
Please fix one minor issue and it's good to go from my PoV 👍
employees/serializers.py
Outdated
|
|
||
| def validate(self, data: OrderedDict) -> OrderedDict: | ||
| data = super().validate(data) | ||
| if "date" in data: |
There was a problem hiding this comment.
Ok, since it seems that the serializer is never created with partial flag (https://www.django-rest-framework.org/api-guide/serializers/#partial-updates), it's safe to assume, that "date" is always present in data. So, please remove the if.
There was a problem hiding this comment.
Based on that, I additionally changed data.get("work_hours", 0) to just data["work_hours"]
rwrzesien
left a comment
rwrzesien
left a comment
There was a problem hiding this comment.
Looks good, but I still think effort put to this could be put into refactoring this view to Django generic, which would auto resolve this issue, as forms always do full_clean() before save().
kbeker
force-pushed
the
bugfix-24-hours-validation-does-not-work-on-report-creation
branch
from
4676b69 to
de04803
Compare
kbeker
deleted the
bugfix-24-hours-validation-does-not-work-on-report-creation
branch
4 participants
Resolves #221