catalyst / moodle-mod_reengagement

Allows timed release of content and emails users to remind them to complete a course activity
23 stars 40 forks source link

RE-73: Fix restrict access by groups not working #74

Closed cwarwicker closed 2 years ago

cwarwicker commented 4 years ago

The method used to check if a course module should be 'available' to a user was not taking into account the possible restrict access settings of specific user groups.

This has been changed to use the filter_user_list method which should resolve that issue.

cwarwicker commented 4 years ago

This commit should fix #73

pnbowen commented 3 years ago

I had the issue resolved in #73 and tried this fix. Unfortunately, it then ignores activity requirements, so if you need to complete x activity before it activates - it activates anyway. (As soon as you enroll in the course). I think it requires some more though. :(

danmarsden commented 3 years ago

Thanks @pnbowen - I wasn't convinced that the new code made sense either which is why I hadn't merged it and wanted to spend some time testing it first (but hadn't managed to find the time!) - thanks heaps for taking a look! Whatever we do I'd like to make sure it's as efficient as possible - the re-engagement cron task can take ages on a site that uses a lot of them, so we want to make sure we don't add lots of extra db calls in that existing loop.

cwarwicker commented 3 years ago

I have no recollection about this commit to be honest, but I can try and remember what it was supposed to be doing.