Team attendance register #84
Conversation
raccube
force-pushed
the
feature/team-attendance
branch
from
a3f5b28 to
ea78a25
Compare
| type = forms.Select(choices=TeamAttendanceEventType.choices) | ||
| comment = forms.CharField(required=False) |
There was a problem hiding this comment.
Issue: These should be the default types for these fields. You should be able to remove these lines entirely.
|
|
||
| def get_initial(self) -> dict[str, Any]: | ||
| return { | ||
| "team": get_object_or_none(Team, tla=self.kwargs["tla"]), |
There was a problem hiding this comment.
Issue: Whilst I like the idea, it's probably better to keep the team only in the URL, rather than in the form.
| return reverse_lazy("teams:team_list_attendance") | ||
|
|
||
| def form_valid(self, form: TeamAttendanceLogForm) -> HttpResponse: | ||
| assert self.request.user.is_authenticated |
There was a problem hiding this comment.
Issue: This assertion shouldn't be necessary
| assert self.request.user.is_authenticated |
| def form_valid(self, form: TeamAttendanceLogForm) -> HttpResponse: | ||
| assert self.request.user.is_authenticated | ||
| team = form.cleaned_data["team"] | ||
| team.team_attendance_events.create( |
There was a problem hiding this comment.
Issue: You should be able to use form.save() to do this instead.
https://forum.djangoproject.com/t/modify-form-field-before-saving-it/4090/3 is a good example.
| def form_invalid(self, form: TeamAttendanceLogForm) -> HttpResponse: | ||
| return HttpResponse("Please fill out the form correctly.") |
There was a problem hiding this comment.
Issue: This is a bad idea. If you leave this out, the form will show its own errors
| {% block title %}Team Attendance{% endblock %} | ||
|
|
||
| {% block page_buttons %} | ||
| {# <input class="input" type="date">#} |
There was a problem hiding this comment.
Question: Is this meant to be commented out?
There was a problem hiding this comment.
This is a WIP thing. I wanted to add the option to filter by day. Not sure if I should keep it. We'd also want to make sure that teams which have dropped out ignore the filter.
| ARRIVED = "AR", "Arrived" | ||
| LEFT = "LE", "Left" | ||
| DELAYED = "DE", "Delayed" | ||
| DROPPED_OUT = "DO", "Dropped Out" | ||
|
|
There was a problem hiding this comment.
Nit: Since these are already small, might as well let the values be human readable
| ARRIVED = "AR", "Arrived" | |
| LEFT = "LE", "Left" | |
| DELAYED = "DE", "Delayed" | |
| DROPPED_OUT = "DO", "Dropped Out" | |
| ARRIVED = "ARRIVED", "Arrived" | |
| LEFT = "LEFT", "Left" | |
| DELAYED = "DELAYED", "Delayed" | |
| DROPPED_OUT = "DROPPED_OUT", "Dropped Out" | |
| lookups = dict(TeamAttendanceEventType.choices) | ||
| return lookups.get(value) |
There was a problem hiding this comment.
Suggestion: Because this is an enum, I think you can do:
| lookups = dict(TeamAttendanceEventType.choices) | |
| return lookups.get(value) | |
| return TeamAttendanceEventType[value].value |
Resolves #35