From 4d8bcbece861f9a26144525cad580c1e2f90690b Mon Sep 17 00:00:00 2001 From: Christian Abila Date: Wed, 19 Nov 2025 06:23:54 +0100 Subject: [PATCH] MDL-86593 Course completion: Disabled modules not returned as criteria - Filter out disabled modules - Update get_criteria method signature - Fix code style --- lib/completionlib.php | 65 +++++++++++++++------------ lib/tests/completionlib_test.php | 76 ++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 28 deletions(-) diff --git a/lib/completionlib.php b/lib/completionlib.php index ccafce43337..19df8dbccf4 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -399,52 +399,61 @@ class completion_info { /** * Get course completion criteria * - * @param int $criteriatype Specific criteria type to return (optional) + * @param int|null $criteriatype Specific criteria type to return (optional) + * @return array */ - public function get_criteria($criteriatype = null) { - - // Fill cache if empty + public function get_criteria(?int $criteriatype = null): array { + // Fill cache if empty. if (!is_array($this->criteria)) { global $DB; - $params = array( - 'course' => $this->course->id - ); + $params = ['course' => $this->course->id]; - // Load criteria from database - $records = (array)$DB->get_records('course_completion_criteria', $params); + // Load criteria from database. + $records = $DB->get_records('course_completion_criteria', $params); - // Order records so activities are in the same order as they appear on the course view page. - if ($records) { - $activitiesorder = array_keys(get_fast_modinfo($this->course)->get_cms()); - usort($records, function ($a, $b) use ($activitiesorder) { - $aidx = ($a->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) ? - array_search($a->moduleinstance, $activitiesorder) : false; - $bidx = ($b->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) ? - array_search($b->moduleinstance, $activitiesorder) : false; - if ($aidx === false || $bidx === false || $aidx == $bidx) { - return 0; - } - return ($aidx < $bidx) ? -1 : 1; - }); + if (empty($records)) { + return []; } - // Build array of criteria objects - $this->criteria = array(); + // Order records so activities are in the same order as they appear on the course view page. + $activitiesorder = array_keys(get_fast_modinfo($this->course)->get_cms()); + + // Remove disabled modules. + foreach ($records as $key => $record) { + if ($record->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) { + if (!in_array($record->moduleinstance, $activitiesorder)) { + unset($records[$key]); + } + } + } + + usort($records, function ($a, $b) use ($activitiesorder) { + $aidx = ($a->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) ? + array_search($a->moduleinstance, $activitiesorder) : false; + $bidx = ($b->criteriatype == COMPLETION_CRITERIA_TYPE_ACTIVITY) ? + array_search($b->moduleinstance, $activitiesorder) : false; + if ($aidx === false || $bidx === false || $aidx == $bidx) { + return 0; + } + return ($aidx < $bidx) ? -1 : 1; + }); + + // Build array of criteria objects. + $this->criteria = []; foreach ($records as $record) { $this->criteria[$record->id] = completion_criteria::factory((array)$record); } } - // If after all criteria + // If after all criteria. if ($criteriatype === null) { return $this->criteria; } - // If we are only after a specific criteria type - $criteria = array(); + // If we are only after a specific criteria type. + $criteria = []; foreach ($this->criteria as $criterion) { - if ($criterion->criteriatype != $criteriatype) { continue; } diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index 513e79d0da1..a2b84ab8fde 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -2295,6 +2295,82 @@ final class completionlib_test extends advanced_testcase { $this->assertEquals($expectedcount, $completion->count_modules_completed($nonexistinguserid)); } } + + /** + * Disabled activities are not returned. + * + * @covers ::get_criteria + */ + public function test_disabled_activities_are_not_returned(): void { + global $DB; + + $this->resetAfterTest(); + $assignmodule = 'assign'; + $bookmodule = 'book'; + $pagemodule = 'page'; + + // Create a course with enabled completion tracking. + $course = $this->getDataGenerator()->create_course(['enablecompletion' => 1]); + + // Add activities to the course and set completion conditions for the activities. + $assign = $this->getDataGenerator()->create_module( + $assignmodule, + ['course' => $course->id, 'completion_assign' => COMPLETION_TRACKING_MANUAL], + ); + + $book = $this->getDataGenerator()->create_module( + $bookmodule, + ['course' => $course->id, 'completion_book' => COMPLETION_TRACKING_MANUAL], + ); + + $page = $this->getDataGenerator()->create_module( + $pagemodule, + ['course' => $course->id, 'completion_page' => COMPLETION_TRACKING_MANUAL], + ); + + // Add the activities as course completion criterias. + $DB->insert_record( + 'course_completion_criteria', + [ + 'course' => $course->id, + 'criteriatype' => COMPLETION_CRITERIA_TYPE_ACTIVITY, + 'module' => $assignmodule, + 'moduleinstance' => $assign->cmid, + ], + ); + + $DB->insert_record( + 'course_completion_criteria', + [ + 'course' => $course->id, + 'criteriatype' => COMPLETION_CRITERIA_TYPE_ACTIVITY, + 'module' => $bookmodule, + 'moduleinstance' => $book->cmid, + ], + ); + + $DB->insert_record( + 'course_completion_criteria', + [ + 'course' => $course->id, + 'criteriatype' => COMPLETION_CRITERIA_TYPE_ACTIVITY, + 'module' => $pagemodule, + 'moduleinstance' => $page->cmid, + ], + ); + + // Disable the book module. + $manager = core_plugin_manager::resolve_plugininfo_class('mod'); + $manager::enable_plugin($bookmodule, 0); + + // Calling get_criteria() should return only the 2 enabled activities. + // The disabled module should be filtered out. + $completioninfo = new completion_info($course); + + $criteria = $completioninfo->get_criteria(COMPLETION_CRITERIA_TYPE_ACTIVITY); + + $this->assertCount(2, $criteria); + } } class core_completionlib_fake_recordset implements Iterator {