feat: Password-protect assessment instance creation #5367
Conversation
| @@ -2,6 +2,7 @@ var ERR = require('async-stacktrace'); | |||
| var express = require('express'); | |||
| var router = express.Router(); | |||
|
|
|||
| const { checkPasswordOrRedirect } = require('../../middlewares/studentAssessmentAccess'); | |||
There was a problem hiding this comment.
I鈥檓 not in love with requiring middleware from page routes, but I don鈥檛 strongly object either. We could shift this function to lib/, but I鈥檓 not convinced it would be neater. I wanted to at least raise the question to see whether you had any other ideas!
There was a problem hiding this comment.
Yeah, I don't love this either, but if we move this to lib/, I think we'd have to move much of studentAssessmentAccess too (along with the .ejs file), which basically feels like we're still importing middleware, just without /middleware/ in the path 馃槄 I'm inclined to leave this as-is.
There was a problem hiding this comment.
Agreed. Let's stick with things as you have them.
Just for my own curiosity, can you say something a bit more about why it wasn鈥檛 enough to do the type of simple check that I was thinking of in #5365? |
Yes! The main issue is that the The alternative would be to move the "does assessment instance exist for this assessment" logic into middleware that runs before |
Excellent point! We don't load the AI until we are in the page handler, which is after the middleware has run.
Yeah, I'm not sure this is cleaner in any way. |
This PR ensures, for exams that require a password, that the student has already entered it before they are allowed to create a new assessment instance. This avoids a problem with prompting for the password after the assessment instance has been created: for time-limited exams, the timer would start counting down as soon as the instance is created, but before the user had entered the password.
Regarding the implementation, I don't love the fact that we now have two pieces of code that need to check for the password, but I don't see a great way around that - the logic of when to actually prompt for a password when hitting a
studentAssessment{Exam,Homework}page can't be really easily expressed up at the middleware level. I tried to compromise by reusing code and leaving plenty of comments 馃しCloses #5365.