diff --git a/completion/criteria/completion_criteria_duration.php b/completion/criteria/completion_criteria_duration.php index 17e6c2705eb..d5d684828d1 100644 --- a/completion/criteria/completion_criteria_duration.php +++ b/completion/criteria/completion_criteria_duration.php @@ -241,8 +241,8 @@ class completion_criteria_duration extends completion_criteria { AND cc.id IS NULL AND ( - ue.timestart > 0 AND ue.timestart + cr.enrolperiod < ? - OR ue.timecreated > 0 AND ue.timecreated + cr.enrolperiod < ? + (ue.timestart > 0 AND (ue.timestart + cr.enrolperiod) < ?) + OR (ue.timestart = 0 AND ue.timecreated > 0 AND (ue.timecreated + cr.enrolperiod) < ?) ) '; diff --git a/completion/tests/completion_criteria_test.php b/completion/tests/completion_criteria_test.php index 4fbdb38d98f..949992f6119 100644 --- a/completion/tests/completion_criteria_test.php +++ b/completion/tests/completion_criteria_test.php @@ -31,6 +31,7 @@ class completion_criteria_test extends \advanced_testcase { */ public function setUp(): void { global $CFG; + require_once($CFG->dirroot.'/completion/criteria/completion_criteria.php'); require_once($CFG->dirroot.'/completion/criteria/completion_criteria_course.php'); require_once($CFG->dirroot.'/completion/criteria/completion_criteria_activity.php'); require_once($CFG->dirroot.'/completion/criteria/completion_criteria_duration.php'); @@ -113,6 +114,41 @@ class completion_criteria_test extends \advanced_testcase { $ccompletion = new \completion_completion(['userid' => $user->id, 'course' => $course->id]); $this->assertEquals($timestarted + $durationperiod, $ccompletion->timecompleted); $this->assertTrue($ccompletion->is_complete()); + + // Now we want to check the scenario where "now" sits in the middle of the timestart + duration + // and timecreated + duration window. + $nowtime = time(); + $timestarted = $nowtime - $durationperiod + (2 * DAYSECS); + $timecreated = $nowtime - $durationperiod - (2 * DAYSECS); + + // Using a new user for this. + $user = $this->getDataGenerator()->create_and_enrol($course, 'student', null, 'manual', $timestarted); + + // We need to manually update the enrollment's time created. + $DB->set_field('user_enrolments', 'timecreated', $timecreated, ['userid' => $user->id]); + + // Run the completion cron. See MDL-33320. + $task->execute(); + sleep(1); + $task->execute(); + + // We do NOT expect the user to be complete currently. + $ccompletion = new \completion_completion(['userid' => $user->id, 'course' => $course->id]); + $this->assertFalse($ccompletion->is_complete()); + + // Now, finally, we will move the timestart to be in the past, but still after the timecreated. + $timestarted = $timecreated + DAYSECS; + $DB->set_field('user_enrolments', 'timestart', $timestarted, ['userid' => $user->id]); + + // Run the completion cron. See MDL-33320. + $task->execute(); + sleep(1); + $task->execute(); + + // Now they should be complete. + $ccompletion = new \completion_completion(['userid' => $user->id, 'course' => $course->id]); + $this->assertEquals($timestarted + $durationperiod, $ccompletion->timecompleted); + $this->assertTrue($ccompletion->is_complete()); } /**