Skip to content

Optimize task email_certificate_task #634

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

Closed

Conversation

Claud10R
Copy link

Hi!

First of all, thanks for this plugin, it helped me quite a lot in generating custom certificates for my courses. I run a big Moodle installation, counting ~1.000 courses and ~60.000 registered users, and this module is now used in a few selected courses, each of which count thousands of active participants (training courses and language proficiency courses for university students).

In my platform, the task email_certificate_task took up to 15 minutes to run, and it basically ran non-stop, since the default scheduled time is once every minute.

I started digging into the code, and I figured out it was possible to reduce the processing time, mainly by implementing what @alex-rowe suggested in this topic: #387 - skipping certificates which are not visible to students. Maybe (not tested) this could be further optimized by changing the first SQL query, adding {course_modules} in the FROM clause and filtering on cm.visible = 1.

I also added another check to skip hidden courses, as suggested by @onadjarj here: #610 (comment). I'm not sure about using courses' end date though, as I'm not sure teachers use it correctly.

Finally, I added a lot of messages in the tasklog, so that it becomes clear what the task has done at every cycle. Log messages require the class attribute VERBOSE to be set to true. Maybe this could also be moved to plugin settings page.

@mdjnelson
Copy link
Owner

Hi Claude, can you rebase this on the latest release? I included a whole lot of changes from other third party devs.

@Claud10R
Copy link
Author

Claud10R commented Oct 3, 2024

Hi, sorry for the delay I just pulled your changes!
Everything looks great to me. One last optimization could be done by getting the info_module relative to the certificate and using filter_user_list function on it.

$infomodule = new \core_availability\info_module($cm);
$filteredusers = $infomodule->filter_user_list($userswithissueview);

This way, you could further reduce the list of users to be checked for every certificate, by using the availability restrictions set on the certificate. This function only takes into account those restrictions which implement filter_user_list, so you still need to check $cm->uservisible inside the loop to be 100% sure the user has access to the certificate module.

I also added a verbose mode to print some debug information on the task log, but that's definitely not needed.

@mdjnelson
Copy link
Owner

Do you want to create a separate PR for the $infomodule->filter_user_list($userswithissueview); fix?

@mdjnelson
Copy link
Owner

I authored a commit on your behalf @Claud10R with the usage of filter_user_list. Closing this now.

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

Successfully merging this pull request may close these issues.

2 participants