Staff #15
Staff #15
Conversation
|
I should have a chance to look at this on my flight home tonight |
|
|
||
|
|
||
| @login_required | ||
| def attendance(request): |
There was a problem hiding this comment.
Follow-up notes in #16, this could use input validation and Post/Redirect/Get refactor
| return round(average * 100, 2) | ||
|
|
||
|
|
||
| def compile_attendance_averages_for_all_courses(): |
| dates = class_offering_meeting_dates(classoffering) | ||
| classroom = Classroom.objects.get(course=classoffering.name) | ||
| for day in dates: | ||
| session = Session.objects.create() |
There was a problem hiding this comment.
It seems odd that the Session has so little information associated with it (no day or class or class offering) yet it's uniquely determined by the combination of those things in this implementation.
There was a problem hiding this comment.
I originally had "date" in the Session model, but switched it to the Attendance model, and each entry in the Attendance model has a Session Id associated with it. The thought was that perhaps an Absent marking in the Attendance model could trigger an email with a file from that day's Session to be auto-generated and sent to the absent student (this comes from my experience of it being a pain to catch students up who are absent and then email me "What did I miss?)"
It wouldn't hurt to put "date" in both Session and Attendance though. Is that what you're thinking I should do?
There was a problem hiding this comment.
Session probably should have a foreign key to classroom and the date, so you can find sessions for a class without looking at attendance records. We could populate the description from a template that uses the name of the Classroom and/or ClassOffering.
I think it's fine to have the date be denormalized to both of these tables given how we're querying for it.
I'd like to merge this into master and write the majority of form signage and new tests on a new branch now that CI is working.