Conversation
Szymiks
requested review from
MartynaAnnaGottschling,
dybi,
kbeker,
maciejSamerdak and
rwrzesien
Szymiks
force-pushed
the
feature-add-dynamic-filter-for-task-activities-per-project
branch
from
b5903a4 to
c067352
Compare
rwrzesien
left a comment
rwrzesien
left a comment
There was a problem hiding this comment.
Nice one, please just add validation for url GET data .
dybi
left a comment
dybi
left a comment
There was a problem hiding this comment.
Looks good, just one thing to potentially improve.
employees/forms.py
Outdated
| self.fields["task_activities"].queryset = TaskActivityType.objects.filter(projects=project_id).order_by( | ||
| "name" | ||
| ) | ||
| except (ValueError, TypeError): |
There was a problem hiding this comment.
When can TypeError occur? If my understanding is correct, ValueError can happen when i.e. string is passed as project.
Besides, for suppressing exceptions I would recommend using: with suppress(MyException) - it's a bit shorter and better reveals intention in my opinion: https://docs.python.org/3/library/contextlib.html#contextlib.suppress
There was a problem hiding this comment.
@dybi, when someone will pass to self.data something lik {"project": "abcd" + 2}
Szymiks
force-pushed
the
feature-project-should-have-custamisavle-task-activity-list
branch
from
73824e2 to
f9d4457
Compare
Szymiks
force-pushed
the
feature-project-should-have-custamisavle-task-activity-list
branch
from
f9d4457 to
7bd31fb
Compare
Szymiks
force-pushed
the
feature-add-dynamic-filter-for-task-activities-per-project
branch
from
566aeb5 to
ed82965
Compare
kbeker
left a comment
kbeker
left a comment
There was a problem hiding this comment.
You did a great job here. Approve :)
kbeker
force-pushed
the
feature-add-dynamic-filter-for-task-activities-per-project
branch
from
ed82965 to
fefe685
Compare
kbeker
changed the base branch from
feature-project-should-have-custamisavle-task-activity-list
to
master
kbeker
force-pushed
the
feature-add-dynamic-filter-for-task-activities-per-project
branch
from
fefe685 to
cfac76f
Compare
kbeker
deleted the
feature-add-dynamic-filter-for-task-activities-per-project
branch
4 participants
Related: #460
On this branch I added dynamic filter for task activity list per each project.
After approving this PR: #460 I will start to create a panel admin view for task activities.
After
full_checkyou can notice two failed test but It has been reported because it occured on master branch.