diff --git a/.upgradenotes/MDL-86332-2025081922154697.yml b/.upgradenotes/MDL-86332-2025081922154697.yml new file mode 100644 index 00000000000..e580c779d36 --- /dev/null +++ b/.upgradenotes/MDL-86332-2025081922154697.yml @@ -0,0 +1,8 @@ +issueNumber: MDL-86332 +notes: + core_completion: + - message: >- + The `completion_info::clear_criteria` method takes an optional + `$removetypecriteria` to determine whether to remove course type + criteria from other courses that refer to the current course + type: changed diff --git a/public/completion/criteria/completion_criteria_course.php b/public/completion/criteria/completion_criteria_course.php index c5eb9cb151c..4cd64f59567 100644 --- a/public/completion/criteria/completion_criteria_course.php +++ b/public/completion/criteria/completion_criteria_course.php @@ -74,7 +74,7 @@ class completion_criteria_course extends completion_criteria { /** * Update the criteria information stored in the database * - * @param array $data Form data + * @param stdClass $data Form data */ public function update_config(&$data) { diff --git a/public/course/completion.php b/public/course/completion.php index f83f8576db0..a251d22cfa8 100644 --- a/public/course/completion.php +++ b/public/course/completion.php @@ -95,7 +95,7 @@ if ($form->is_cancelled()){ } // Delete old criteria. - $completion->clear_criteria(); + $completion->clear_criteria(false); // Loop through each criteria type and run its update_config() method. global $COMPLETION_CRITERIA_TYPES; diff --git a/public/lib/completionlib.php b/public/lib/completionlib.php index f32eddea022..ccafce43337 100644 --- a/public/lib/completionlib.php +++ b/public/lib/completionlib.php @@ -478,17 +478,23 @@ class completion_info { /** * Clear old course completion criteria + * + * @param bool $removetypecriteria Also remove course type criteria from other courses that refer to the current course */ - public function clear_criteria() { + public function clear_criteria(bool $removetypecriteria = true): void { global $DB; + $select = 'course = :course'; + $params = ['course' => $this->course->id]; + // Remove completion criteria records for the course itself, and any records that refer to the course. - $select = 'course = :course OR (criteriatype = :type AND courseinstance = :courseinstance)'; - $params = [ - 'course' => $this->course_id, - 'type' => COMPLETION_CRITERIA_TYPE_COURSE, - 'courseinstance' => $this->course_id, - ]; + if ($removetypecriteria) { + $select .= ' OR (criteriatype = :type AND courseinstance = :courseinstance)'; + $params = array_merge($params, [ + 'type' => COMPLETION_CRITERIA_TYPE_COURSE, + 'courseinstance' => $this->course_id, + ]); + } $DB->delete_records_select('course_completion_criteria', $select, $params); $DB->delete_records('course_completion_aggr_methd', array('course' => $this->course_id)); diff --git a/public/lib/tests/completionlib_test.php b/public/lib/tests/completionlib_test.php index e2df63d8140..513e79d0da1 100644 --- a/public/lib/tests/completionlib_test.php +++ b/public/lib/tests/completionlib_test.php @@ -1466,9 +1466,70 @@ final class completionlib_test extends advanced_testcase { $this->assertFalse($c2->has_activities()); } + /** + * Data provider for {@see test_clear_criteria} + * + * @return bool[][] + */ + public static function clear_criteria_provider(): array { + return [ + [false], + [true], + ]; + } + + /** + * Test clearing criteria for current course + * + * @param bool $removetypecriteria + * + * @covers ::clear_criteria + * @dataProvider clear_criteria_provider + */ + public function test_clear_criteria(bool $removetypecriteria): void { + global $DB; + + $this->setup_data(); + + $courseprerequisite = $this->getDataGenerator()->create_course(['enablecompletion' => true]); + + /** @var completion_criteria_self $criteria */ + $criteria = completion_criteria::factory(['criteriatype' => COMPLETION_CRITERIA_TYPE_SELF]); + $criteriadata = (object) [ + 'id' => $courseprerequisite->id, + 'criteria_self' => 1, + ]; + $criteria->update_config($criteriadata); + + /** @var completion_criteria_course $criteria */ + $criteria = completion_criteria::factory(['criteriatype' => COMPLETION_CRITERIA_TYPE_COURSE]); + $criteriadata = (object) [ + 'id' => $this->course->id, + 'criteria_course' => [$courseprerequisite->id], + ]; + $criteria->update_config($criteriadata); + + // Sanity test. + $this->assertTrue($DB->record_exists('course_completion_criteria', ['course' => $courseprerequisite->id])); + + $completion = new completion_info($courseprerequisite); + $completion->clear_criteria($removetypecriteria); + + // There should be no criteria data for the course. + $this->assertFalse($DB->record_exists('course_completion_criteria', ['course' => $courseprerequisite->id])); + + // Course type criteria from other courses that refer to the course. + $this->assertEquals(!$removetypecriteria, $DB->record_exists('course_completion_criteria', [ + 'course' => $this->course->id, + 'criteriatype' => COMPLETION_CRITERIA_TYPE_COURSE, + 'courseinstance' => $courseprerequisite->id, + ])); + } + /** * Test that data is cleaned up when we delete courses that are set as completion criteria for other courses * + * @covers ::clear_criteria * @covers ::delete_course_completion_data * @covers ::delete_all_completion_data */ @@ -1479,13 +1540,12 @@ final class completionlib_test extends advanced_testcase { $courseprerequisite = $this->getDataGenerator()->create_course(['enablecompletion' => true]); + /** @var completion_criteria_course $criteria */ + $criteria = completion_criteria::factory(['criteriatype' => COMPLETION_CRITERIA_TYPE_COURSE]); $criteriadata = (object) [ 'id' => $this->course->id, 'criteria_course' => [$courseprerequisite->id], ]; - - /** @var completion_criteria_course $criteria */ - $criteria = completion_criteria::factory(['criteriatype' => COMPLETION_CRITERIA_TYPE_COURSE]); $criteria->update_config($criteriadata); // Sanity test.