Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attendance follow-up #16

Open
4 of 5 tasks
etrepum opened this issue Jul 15, 2019 · 0 comments
Open
4 of 5 tasks

Attendance follow-up #16

etrepum opened this issue Jul 15, 2019 · 0 comments

Comments

@etrepum
Copy link
Member

etrepum commented Jul 15, 2019

These could be broken down into separate issues/PRs, collecting into one issue as that's more convenient to do for the review of #15. I'll be editing this issue as I go along to collect the feedback.

attendance.views.attendance

  • Presumably these views should require staff level authorization, rather than just a login
  • Input validation is missing
  • The POST handling code (in the success case) could be simplified using Post/Redirect/Get
  • Date parsing should be factored out to a function, possibly made more robust (as in More robust birthdate parsing #13)

attendance.views.compile_attendance_averages_for_all_courses

  • This is likely to be a bottleneck, should be computed out of band from the request (maybe with a trigger of some kind on update, and/or updated periodically) and then cached. Salesforce probably has a mechanism for this with Reports, but there are of course other ways to offer this functionality. The manner in which this is computed is also likely to be quite slow, as it fetches all of the data over the course of several queries and then aggregates it from Python rather than doing this work at the database.
@etrepum etrepum mentioned this issue Jul 15, 2019
@tylerIams tylerIams mentioned this issue Aug 12, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant