Closed JackAidley closed 3 months ago
I had to think about this one for a while to understand why I put that there. Turns out it is needed.
This is needed because we get all the issued users in $issuedusers = $DB->get_records_sql($sql, ['customcertid' => $customcert->id]);
. It then gets all the enrolled users in the course, and if they can see the certificate adds them to that list. The loop you pointed out removes the ones that are already emailed that were retrieved from the DB call I listed. We can not filter by email in the get_records_sql
call because we want all users. If we just got the people who werent emailed then it would keep emailing users who already have it because they are in the enrolled users call.
Ah gotcha. Sorry I missed that on reading the code. One thought though, wouldn't it be better to screen out the issuedusers with emailed set to non-zero in the SQL query that returns them instead of fetching unwanted records and then screening them out? I don't suppose it's particularly performance critical either way.
The problem with that is that the $issuedusers
is used to check if the user has already been issued.
I've been looking at this task because it is taking a long time even when nothing is happening - hopefully that'll be resolved by https://github.com/mdjnelson/moodle-mod_customcert/pull/596 - but while looking I noticed that this loop:
Appears to do nothing since, as far as I can tell, the
emailed
flag is only ever set in the code a few lines above where it is set to 0:And so can never have a truthy value. I'm unclear whether this is actually causing a bug or whether it's just redundant.