From ea6d8984bacda7152d8c438d99d7b6cec902b66d Mon Sep 17 00:00:00 2001 From: Pedro Jordao Date: Tue, 3 Dec 2024 17:10:43 -0300 Subject: [PATCH] MDL-83917 core_completion: Create function count_modules_completed The function returns the number of modules completed by a user and executes a COUNT aggregate function to avoid running many queries to obtain this information, aiming to optimize performance. Co-authored-by: Carlos Castillo --- .upgradenotes/MDL-83917-2024121004111313.yml | 7 + completion/classes/progress.php | 12 +- lib/completionlib.php | 21 +++ lib/tests/completionlib_test.php | 149 +++++++++++++++++++ 4 files changed, 179 insertions(+), 10 deletions(-) create mode 100644 .upgradenotes/MDL-83917-2024121004111313.yml diff --git a/.upgradenotes/MDL-83917-2024121004111313.yml b/.upgradenotes/MDL-83917-2024121004111313.yml new file mode 100644 index 00000000000..78f31812505 --- /dev/null +++ b/.upgradenotes/MDL-83917-2024121004111313.yml @@ -0,0 +1,7 @@ +issueNumber: MDL-83917 +notes: + core_completion: + - message: >- + The method `count_modules_completed` now delegate the logic to count the + completed modules to the DBMS improving the performance of the method. + type: improved diff --git a/completion/classes/progress.php b/completion/classes/progress.php index c95352259f5..6689bfd295a 100644 --- a/completion/classes/progress.php +++ b/completion/classes/progress.php @@ -77,16 +77,8 @@ class progress { } // Get the number of modules that have been completed. - $completed = 0; - foreach ($modules as $module) { - $data = $completion->get_data($module, true, $userid); - if (($data->completionstate == COMPLETION_INCOMPLETE) || ($data->completionstate == COMPLETION_COMPLETE_FAIL)) { - $completed += 0; - } else { - $completed += 1; - }; - } + $totalcompleted = $completion->count_modules_completed($userid); - return ($completed / $count) * 100; + return ($totalcompleted / $count) * 100; } } diff --git a/lib/completionlib.php b/lib/completionlib.php index b18d4768b58..e6444645bc2 100644 --- a/lib/completionlib.php +++ b/lib/completionlib.php @@ -1641,6 +1641,27 @@ class completion_info { return (array)$data; } + + /** + * Return the number of modules completed by a user in one specific course. + * + * @param int $userid The User ID. + * @return int Total number of modules completed by a user + */ + public function count_modules_completed(int $userid): int { + global $DB; + + $sql = "SELECT COUNT(1) + FROM {course_modules} cm + JOIN {course_modules_completion} cmc ON cm.id = cmc.coursemoduleid + WHERE cm.course = :courseid + AND cmc.userid = :userid + AND (cmc.completionstate = " . COMPLETION_COMPLETE . " + OR cmc.completionstate = " . COMPLETION_COMPLETE_PASS . ")"; + $params = ['courseid' => $this->course_id, 'userid' => $userid]; + + return $DB->count_records_sql($sql, $params); + } } /** diff --git a/lib/tests/completionlib_test.php b/lib/tests/completionlib_test.php index 0991e6f0cfe..47bd8331616 100644 --- a/lib/tests/completionlib_test.php +++ b/lib/tests/completionlib_test.php @@ -2086,6 +2086,155 @@ final class completionlib_test extends advanced_testcase { ['course' => $this->course->id] )); } + + /** + * Data provider for test_count_modules_completed(). + * + * @return array[] + */ + public static function count_modules_completed_provider(): array { + return [ + 'Multiple users with two different modules but only one completed' => [ + 'existinguser' => true, + 'totalusers' => 3, + 'modules' => [ + [ + 'name' => 'assign', + 'completionstate' => COMPLETION_COMPLETE, + ], + [ + 'name' => 'choice', + 'completionstate' => COMPLETION_INCOMPLETE, + ], + ], + 'expectedcount' => 1, + ], + 'Multiple users with three different modules but only two completed' => [ + 'existinguser' => true, + 'totalusers' => 4, + 'modules' => [ + [ + 'name' => 'assign', + 'completionstate' => COMPLETION_COMPLETE, + ], + [ + 'name' => 'choice', + 'completionstate' => COMPLETION_INCOMPLETE, + ], + [ + 'name' => 'workshop', + 'completionstate' => COMPLETION_COMPLETE, + ], + ], + 'expectedcount' => 2, + ], + 'Multiple users with one completion each' => [ + 'existinguser' => true, + 'totalusers' => 5, + 'modules' => [ + [ + 'name' => 'assign', + 'completionstate' => COMPLETION_COMPLETE, + ], + ], + 'expectedcount' => 1, + ], + 'One user with one completion' => [ + 'existinguser' => true, + 'totalusers' => 1, + 'modules' => [ + [ + 'name' => 'assign', + 'completionstate' => COMPLETION_COMPLETE, + ], + ], + 'expectedcount' => 1, + ], + 'Multiple users without completion' => [ + 'existinguser' => true, + 'totalusers' => 3, + 'modules' => [ + [ + 'name' => 'assign', + 'completionstate' => COMPLETION_INCOMPLETE, + ], + ], + 'expectedcount' => 0, + ], + 'Non-existing user' => [ + 'existinguser' => false, + 'totalusers' => 1, + 'modules' => [ + [ + 'name' => 'assign', + 'completionstate' => COMPLETION_INCOMPLETE, + ], + ], + 'expectedcount' => 0, + ], + ]; + } + + /** + * Test for count_modules_completed(). + * + * @dataProvider count_modules_completed_provider + * @param bool $existinguser Whether the given user exists or not. + * @param int $totalusers The amount of users to check completion. + * @param array $modules The course modules with its completion state. + * @param int $expectedcount Expected total of modules completed. + * @covers ::count_modules_completed + */ + public function test_count_modules_completed(bool $existinguser, int $totalusers, array $modules, + int $expectedcount): void { + global $DB; + + $this->setAdminUser(); + $this->setup_data(); + + // Loop through the provided modules array and set the id key based on the generated module. + $modules = array_map(function(array $module): array { + $generator = $this->getDataGenerator()->get_plugin_generator('mod_' . $module['name']); + $modinstance = $generator->create_instance([ + 'course' => $this->course->id, + 'completion' => COMPLETION_TRACKING_AUTOMATIC, + 'completionsubmit' => true, + ]); + $cminstance = get_coursemodule_from_instance($module['name'], $modinstance->id); + + $module['id'] = $cminstance->id; + return $module; + }, $modules); + + $completion = new completion_info($this->course); + + if ($existinguser) { + // Create users, assign them to a course and define the completion record. + for ($i = 0; $i < $totalusers; $i++) { + $user = $this->getDataGenerator()->create_user(); + $this->getDataGenerator()->enrol_user($user->id, $this->course->id); + $users[] = $user; + + foreach ($modules as $module) { + $cmcompletionrecords[] = (object)[ + 'coursemoduleid' => $module['id'], + 'userid' => $user->id, + 'completionstate' => $module['completionstate'], + 'timemodified' => 0, + ]; + } + } + + $DB->insert_records('course_modules_completion', $cmcompletionrecords); + + foreach ($users as $user) { + $this->assertEquals($expectedcount, $completion->count_modules_completed($user->id)); + } + } else { + $nonexistinguserid = 123; + $this->assertEquals($expectedcount, $completion->count_modules_completed($nonexistinguserid)); + } + } } class core_completionlib_fake_recordset implements Iterator {