moodle-checklist
moodle-checklist copied to clipboard
davosmith
Checklist doesn't detect completion for courses completed before enrollment
If a user is enrolled in a course that has a checklist item for completing a course that they've already completed, it will never auto update. This is because the plugin is waiting to update that item until it hears the course completion event that has already happened.
Here is a possible fix: https://github.com/moonami/moodle-checklist/commit/7229a91f0af8fd214ffbcd415d2441476e7170f7
Do you have any thoughts? It solves the problem but feels a bit gratuitous to be effectively re-running all of a user's course completion events every time they are assigned a course role. There is refining I could to by checking the course they are assigned a role in for checklist items that need to be updated, but I wanted to get some feedback before spending too much time on this.
I'll have to think about it and will try to get back to you - please prod me if you don't hear back in a couple of weeks (as I have a lot on at the moment and I may not get onto it for a while).
Hi thanks for the prod - I've been pretty busy and not had a chance to really look at any code for this.
The code you've written looks like it would basically work, but I'd be worried about the performance impact if there were a lot of courses that a user had already completed (or how this would scale if, for example, audience enrolment was used to enrol a large number of users at the same time).
I'd probably feel happier with a custom SQL statement that matched course completions for the user against checklist items linked to those completed courses in the course that the user was being enrolled on - in most cases, that would mean that no further processing was needed, but in those rare cases where it was relevant, it would be good to just loop through those identified checklist items and update them directly (rather than triggering a lot of code to load them all over again).
Other thoughts include:
- It would be good to write a PHPUnit test to test the scenario (especially one which could be run both before and after the fix, to prove it works)
- It would be good to check that the role being assigned is one that is able to update checklists on this course (but then it gets tricky if you really want to be picky and check that per-checklist, just in case there is a role override somewhere ...)
- Ideally context level should be checked against CONTEXT_COURSE, rather than "50" (even though they're the same, CONTEXT_COURSE is easier to read/maintain)
- As much as possible, I've tried to make sure all the code follows the Moodle coding guidelines, I can spot the spacing is slightly out and one of the lines looks like it might be a bit too long (but I've not checked) - this last one I can always fix after accepting a commit, so I'm not going to be too picky about it
Does that give you something to work with?
I may try and look at it myself, but I'm finding it harder these days to motivate to get fixes to my plugins done in my spare time, as I spend all day writing Moodle code for other people.
Yeah, that's helpful. I'll try to put something together this week and get back to you.
Hi frederickOtus, Did you ever get this sorted? We have a similar problem, which doesn't occur very often, nor for many students on our site, but when it does it causes a lot of confusion for the affected students so would be good to resolve. It happens to us when a student completes a CII Qual. and then goes on to do a CIII Qual. Because they are unenrolled in all the courses and then re-enrolled under the new qual, none of the completed courses update on the checklist.
I haven't gotten around to getting it merged back in (need to write some tests) but this is the code we are running and it's been working for us:
https://github.com/moonami/moodle-checklist/tree/update_on_enrol