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

scheduled task fails if reengagement_inprogress contains non-completed entries for deleted activites #91

Closed kiklop74 closed 1 year ago

kiklop74 commented 3 years ago

Let us assume that you have a 5 reengagement activities and appropriate inprogress records are created. Than for whatever reason one or two activities are deleted before their inprogress records are processed. In such cases scheduled task (cron) ALWAYS fails and never finishes it's work. This is resolved only by manually deleting orphaned records from the table with query like this (mariadb/mysql):

DELETE FROM mdl_reengagement_inprogress
WHERE reengagement NOT IN (SELECT mr.id FROM mdl_reengagement mr WHERE mr.id = reengagement) AND completed = 0;

When task fails it outputs something like this:

Scheduled task failed: Reengagement cron task (mod_reengagement\task\cron_task),Invalid course module ID
Debug info:
SELECT id,course FROM {course_modules} WHERE id IS NULL
[array (
)]
Backtrace:
* line 1575 of /lib/dml/moodle_database.php: call to moodle_database->get_record_select()
* line 7289 of /lib/accesslib.php: call to moodle_database->get_record()
* line 278 of /mod/reengagement/lib.php: call to context_module::instance()
* line 54 of /mod/reengagement/classes/task/cron_task.php: call to reengagement_crontask()
* line 248 of /lib/cronlib.php: call to mod_reengagement\task\cron_task->execute()
* line 150 of /admin/cli/scheduled_task.php: call to cron_run_inner_scheduled_task()

The fix for this would be to ensure only valid entries for reengagement inporgress are processed.

So this:

    $inprogresssql = 'SELECT ri.*
                        FROM {reengagement_inprogress} ri
                        JOIN {user} u ON u.id = ri.userid
                       WHERE u.deleted = 0 AND
                       completiontime < ? AND completed = 0';

Should become this:

    $inprogresssql = 'SELECT ri.*
                        FROM {reengagement_inprogress} ri
                        JOIN {reengagement} r ON r.id = ri.reengagement
                        JOIN {user} u ON u.id = ri.userid
                       WHERE u.deleted = 0 AND
                       completiontime < ? AND completed = 0';
kiklop74 commented 3 years ago

If you want I can provide PR.

danmarsden commented 3 years ago

Cool - thanks! - I've merged that in. I've left this open because I'd like to find some time to investigate why those records weren't cleaned up - just in case we're mising something on activity/course deletion.

danmarsden commented 1 year ago

flagging this as closed as the cron now deals with this.