From 48520cfc6a717d7ab7ad1c22a67d9f491554a25f Mon Sep 17 00:00:00 2001 From: Anupama Sarjoshi Date: Fri, 16 Jan 2026 19:53:17 +0530 Subject: [PATCH] MDL-60912 core_completion: Fix course completion percentage calculation This change ensures that course completion percentage calculations include only completion activities visible to the user on the course home page. Hidden or unavailable activities, and those in hidden sections, are no longer included in the calculation. --- completion/classes/progress.php | 4 +- completion/tests/progress_test.php | 292 +++++++++++++++++++++++++++++ lib/completionlib.php | 10 +- lib/tests/completionlib_test.php | 9 +- 4 files changed, 310 insertions(+), 5 deletions(-) diff --git a/completion/classes/progress.php b/completion/classes/progress.php index 6689bfd295a..48c8c2bf63f 100644 --- a/completion/classes/progress.php +++ b/completion/classes/progress.php @@ -70,14 +70,14 @@ class progress { } // Get the number of modules that support completion. - $modules = $completion->get_activities(); + $modules = $completion->get_user_activities_with_completion($userid); $count = count($modules); if (!$count) { return null; } // Get the number of modules that have been completed. - $totalcompleted = $completion->count_modules_completed($userid); + $totalcompleted = $completion->count_modules_completed($userid, array_keys($modules)); return ($totalcompleted / $count) * 100; } diff --git a/completion/tests/progress_test.php b/completion/tests/progress_test.php index d0960395044..29151b08e3d 100644 --- a/completion/tests/progress_test.php +++ b/completion/tests/progress_test.php @@ -17,6 +17,8 @@ namespace core_completion; use completion_completion; +use core_availability\tree; +use availability_date\condition; /** * Test completion progress API. @@ -67,6 +69,8 @@ final class progress_test extends \advanced_testcase { // Add an activity that does *not* use completion. $this->getDataGenerator()->create_module('assign', array('course' => $course->id)); + $this->setUser($user); + // Mark two of them as completed for a user. $cmassign = get_coursemodule_from_id('assign', $assign->cmid); $cmdata = get_coursemodule_from_id('data', $data->cmid); @@ -283,4 +287,292 @@ final class progress_test extends \advanced_testcase { // Check that the result is null now. $this->assertNull(\core_completion\progress::get_course_progress_percentage($course, $user->id)); } + + /** + * Tests course progress with hidden section. + */ + public function test_course_progress_percentage_with_hidden_section(): void { + global $DB; + + // Create a course with completion enabled and two sections. + $course = $this->getDataGenerator()->create_course([ + 'enablecompletion' => 1, + 'numsections' => 2, + 'format' => 'topics', + ]); + + // Enrol a student. + $user = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + $this->getDataGenerator()->enrol_user($user->id, $course->id, $studentrole->id); + + /** @var \mod_assign_generator $assigngenerator */ + $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + + // Add visible activities to section 0 and 1. + $activity1 = $assigngenerator->create_instance([ + 'course' => $course->id, + 'section' => 0, + 'completion' => COMPLETION_ENABLED, + ]); + $activity2 = $assigngenerator->create_instance([ + 'course' => $course->id, + 'section' => 1, + 'completion' => COMPLETION_ENABLED, + ]); + $this->setUser($user); + + // Hide section 1. + set_section_visible($course->id, 1, 0); + $completion = new \completion_info($course); + + // Complete the visible activity: activity1. + $cm = get_coursemodule_from_id('assign', $activity1->cmid); + $completion->update_state($cm, COMPLETION_COMPLETE, $user->id); + + // Only the visible activity (activity1) counts toward course completion. + // Activities in hidden sections are not included in the calculation. + $this->assertEquals(100, \core_completion\progress::get_course_progress_percentage($course, $user->id)); + + // Now unhide section 1. + set_section_visible($course->id, 1, 1); + + // Course completion: 1 of 2 visible activities complete; previously hidden activity now counted. + $this->assertEquals(50, \core_completion\progress::get_course_progress_percentage($course, $user->id)); + } + + /** + * Tests course progress with hidden activity. + */ + public function test_course_progress_percentage_with_hidden_activity(): void { + global $DB; + + // Create a course with completion enabled and two sections. + $course = $this->getDataGenerator()->create_course([ + 'enablecompletion' => 1, + 'numsections' => 3, + 'format' => 'topics', + ]); + + // Enrol a student. + $user = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + $this->getDataGenerator()->enrol_user($user->id, $course->id, $studentrole->id); + + /** @var \mod_assign_generator $assigngenerator */ + $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + + // Section 0: visible activity. + $activity1 = $assigngenerator->create_instance([ + 'course' => $course->id, + 'section' => 0, + 'completion' => COMPLETION_ENABLED, + ]); + + // Section 1: hidden activity. + $activity2 = $assigngenerator->create_instance([ + 'course' => $course->id, + 'section' => 1, + 'completion' => COMPLETION_ENABLED, + 'visible' => 0, + ]); + + $completion = new \completion_info($course); + $this->setUser($user); + + // Complete activity1 (visible). + $cm1 = get_coursemodule_from_id('assign', $activity1->cmid); + $completion->update_state($cm1, COMPLETION_COMPLETE, $user->id); + + // Only 1 visible activity: course completion = 100%. + $this->assertEquals(100, \core_completion\progress::get_course_progress_percentage($course, $user->id)); + + $cm2 = get_coursemodule_from_id('assign', $activity2->cmid); + // Show activity2. + $DB->set_field('course_modules', 'visible', 1, ['id' => $cm2->id]); + $completion->update_state($cm2, COMPLETION_COMPLETE, $user->id); + rebuild_course_cache($course->id, true); + + // Course completion: both activities are complete. + $this->assertEquals(100, \core_completion\progress::get_course_progress_percentage($course, $user->id)); + + // Hide activity1 so it is excluded from course progress. + $DB->set_field('course_modules', 'visible', 0, ['id' => $cm1->id]); + rebuild_course_cache($course->id, true); + + // Course completion: activity1 is hidden and excluded from calculation. + // Only activity2 remains visible and complete, so progress stays at 100%. + $this->assertEquals(100, \core_completion\progress::get_course_progress_percentage($course, $user->id)); + } + + /** + * Tests course progress percentage with availability restrictions. + */ + public function test_course_progress_percentage_with_availability_restrictions(): void { + global $DB; + + set_config('enableavailability', 1); + + // Create a course with completion enabled. + $course = $this->getDataGenerator()->create_course([ + 'enablecompletion' => 1, + 'format' => 'topics', + ]); + + // Create and enrol a student. + $user = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + $this->getDataGenerator()->enrol_user($user->id, $course->id, $studentrole->id); + + /** @var \mod_assign_generator $assigngenerator */ + $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + + // Activity 1: no restrictions (always visible). + $assign['activity1'] = $assigngenerator->create_instance([ + 'course' => $course->id, + 'completion' => COMPLETION_ENABLED, + ]); + + // Activity 2: date restriction from past (visible). + $assign['activity2'] = $assigngenerator->create_instance([ + 'course' => $course->id, + 'completion' => COMPLETION_ENABLED, + ]); + $availabilityjson = json_encode(tree::get_root_json( + [ + condition::get_json(condition::DIRECTION_FROM, time() - 3600), + ], + tree::OP_AND, + false, + )); + $DB->set_field('course_modules', 'availability', $availabilityjson, ['id' => $assign['activity2']->cmid]); + + // Activity 3: date restriction from future, shows info (open eye). + $assign['activity3'] = $assigngenerator->create_instance([ + 'course' => $course->id, + 'completion' => COMPLETION_ENABLED, + ]); + $availabilityjson = json_encode(tree::get_root_json( + [ + condition::get_json(condition::DIRECTION_FROM, time() + 3600), + ], + tree::OP_AND, + true, + )); + $DB->set_field('course_modules', 'availability', $availabilityjson, ['id' => $assign['activity3']->cmid]); + + // Activity 4: date restriction from future, hidden (closed eye). + $assign['activity4'] = $assigngenerator->create_instance([ + 'course' => $course->id, + 'completion' => COMPLETION_ENABLED, + ]); + $availabilityjson = json_encode(tree::get_root_json( + [ + condition::get_json(condition::DIRECTION_FROM, time() + 7200), + ], + tree::OP_AND, + false, + )); + $DB->set_field('course_modules', 'availability', $availabilityjson, ['id' => $assign['activity4']->cmid]); + + rebuild_course_cache($course->id, true); + + // Set user context and get completion info. + $this->setUser($user); + $completion = new \completion_info($course); + + // Initial completion: 0%. + $this->assertEquals(0, \core_completion\progress::get_course_progress_percentage($course, $user->id)); + + // Complete activity 1. + $cm1 = get_coursemodule_from_id('assign', $assign['activity1']->cmid); + $completion->update_state($cm1, COMPLETION_COMPLETE, $user->id); + // 1 of 3 user-visible activities is complete; activity4 is hidden and ignored. + $this->assertEquals(33.33, round(\core_completion\progress::get_course_progress_percentage($course, $user->id), 2)); + + // Complete activity 2 (available from past). + $cm2 = get_coursemodule_from_id('assign', $assign['activity2']->cmid); + $completion->update_state($cm2, COMPLETION_COMPLETE, $user->id); + // Now 2 of the 3 user-visible activities are complete. + $this->assertEquals(66.67, round(\core_completion\progress::get_course_progress_percentage($course, $user->id), 2)); + + // Mark the restricted activity (with open eye) as complete. + $cm3 = get_coursemodule_from_id('assign', $assign['activity3']->cmid); + $completion->update_state($cm3, COMPLETION_COMPLETE, $user->id); + // All 3 user-visible activities are now complete; activity4 remains hidden. + $this->assertEquals(100, round(\core_completion\progress::get_course_progress_percentage($course, $user->id), 2)); + } + + /** + * Tests course progress percentage with group restrictions. + */ + public function test_course_progress_percentage_with_group_restrictions(): void { + global $DB; + + set_config('enableavailability', 1); + + // Create a course with completion enabled. + $course = $this->getDataGenerator()->create_course([ + 'enablecompletion' => 1, + 'format' => 'topics', + ]); + + // Create and enrol two students in the course. + $user1 = $this->getDataGenerator()->create_user(); + $user2 = $this->getDataGenerator()->create_user(); + $studentrole = $DB->get_record('role', ['shortname' => 'student']); + $this->getDataGenerator()->enrol_user($user1->id, $course->id, $studentrole->id); + $this->getDataGenerator()->enrol_user($user2->id, $course->id, $studentrole->id); + + // Create a group and add user1. + $group1 = $this->getDataGenerator()->create_group([ + 'courseid' => $course->id, + 'name' => 'Group 1', + ]); + $this->getDataGenerator()->create_group_member([ + 'groupid' => $group1->id, + 'userid' => $user1->id, + ]); + + /** @var \mod_assign_generator $assigngenerator */ + $assigngenerator = $this->getDataGenerator()->get_plugin_generator('mod_assign'); + + // Activity 1: restricted to group1. + $assign['activity1'] = $assigngenerator->create_instance([ + 'course' => $course->id, + 'completion' => COMPLETION_ENABLED, + ]); + $availabilityjson = json_encode(tree::get_root_json( + [ + \availability_group\condition::get_json($group1->id), + ], + tree::OP_AND, + false, + )); + $DB->set_field('course_modules', 'availability', $availabilityjson, ['id' => $assign['activity1']->cmid]); + + // Activity 2: Visible to all users. + $assign['activity2'] = $assigngenerator->create_instance([ + 'course' => $course->id, + 'completion' => COMPLETION_ENABLED, + ]); + rebuild_course_cache($course->id, true); + + $cm1 = get_coursemodule_from_id('assign', $assign['activity1']->cmid); + $cm2 = get_coursemodule_from_id('assign', $assign['activity2']->cmid); + + // Test user1 (in group): complete both the activities. + $this->setUser($user1); + $completion = new \completion_info($course); + $completion->update_state($cm1, COMPLETION_COMPLETE, $user1->id); + $this->assertEquals(50, \core_completion\progress::get_course_progress_percentage($course, $user1->id)); + $completion->update_state($cm2, COMPLETION_COMPLETE, $user1->id); + $this->assertEquals(100, \core_completion\progress::get_course_progress_percentage($course, $user1->id)); + + // Test user2 (not in group1): completes only the visible activity. + $this->setUser($user2); + $completion = new \completion_info($course); + $completion->update_state($cm2, COMPLETION_COMPLETE, $user2->id); + $this->assertEquals(100, \core_completion\progress::get_course_progress_percentage($course, $user2->id)); + } } diff --git a/lib/completionlib.php b/lib/completionlib.php index 234be2fbf7a..5c7c1ab94da 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -1693,9 +1693,10 @@ class completion_info { * Return the number of modules completed by a user in one specific course. * * @param int $userid The User ID. + * @param array $moduleids The course modules to check. * @return int Total number of modules completed by a user */ - public function count_modules_completed(int $userid): int { + public function count_modules_completed(int $userid, array $moduleids = []): int { global $DB; $sql = "SELECT COUNT(1) @@ -1703,10 +1704,17 @@ class completion_info { JOIN {course_modules_completion} cmc ON cm.id = cmc.coursemoduleid WHERE cm.course = :courseid AND cmc.userid = :userid + AND cm.visible = 1 AND (cmc.completionstate = " . COMPLETION_COMPLETE . " OR cmc.completionstate = " . COMPLETION_COMPLETE_PASS . ")"; $params = ['courseid' => $this->course_id, 'userid' => $userid]; + if (!empty($moduleids)) { + [$insql, $inparams] = $DB->get_in_or_equal($moduleids, SQL_PARAMS_NAMED); + $sql.= " AND cm.id $insql"; + $params = array_merge($params, $inparams); + } + return $DB->count_records_sql($sql, $params); } } diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index f276e173d34..03cc3039c4a 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -2289,6 +2289,11 @@ final class completionlib_test extends advanced_testcase { return $module; }, $modules); + $moduleids = []; + $moduleids = array_map(function (array $module): int { + return $module['id']; + }, $modules); + $completion = new completion_info($this->course); if ($existinguser) { @@ -2311,11 +2316,11 @@ final class completionlib_test extends advanced_testcase { $DB->insert_records('course_modules_completion', $cmcompletionrecords); foreach ($users as $user) { - $this->assertEquals($expectedcount, $completion->count_modules_completed($user->id)); + $this->assertEquals($expectedcount, $completion->count_modules_completed($user->id, $moduleids)); } } else { $nonexistinguserid = 123; - $this->assertEquals($expectedcount, $completion->count_modules_completed($nonexistinguserid)); + $this->assertEquals($expectedcount, $completion->count_modules_completed($nonexistinguserid, $moduleids)); } }