diff --git a/public/mod/scorm/classes/courseformat/overview.php b/public/mod/scorm/classes/courseformat/overview.php
index ea60e0a4d39..c87c65269f9 100644
--- a/public/mod/scorm/classes/courseformat/overview.php
+++ b/public/mod/scorm/classes/courseformat/overview.php
@@ -150,14 +150,15 @@ class overview extends \core_courseformat\activityoverviewbase {
return null;
}
$groups = array_map(fn($group) => $group->id, $this->get_groups_for_filtering());
- $totalattempts = $this->manager->count_users_who_attempted($groups);
- $userswhocanattempt = $this->manager->count_participants($groups);
$maxattempts = $this->manager->get_max_attempts();
if ($maxattempts === 0) {
$maxattemptstext = get_string('unlimited');
} else {
$maxattemptstext = (string) $maxattempts;
}
+ $totalattempts = $this->manager->count_all_attempts($groups);
+ $attemptedusers = $this->manager->count_users_who_attempted($groups);
+ $averageattempts = $totalattempts ? round($totalattempts / $attemptedusers, 1) : 0;
$content = new overviewdialog(
buttoncontent: $totalattempts,
@@ -171,10 +172,6 @@ class overview extends \core_courseformat\activityoverviewbase {
);
$content->add_item(get_string('allowedattemptsstudent', 'mod_scorm'), $maxattemptstext);
- $averageattempts = 0;
- if ($userswhocanattempt > 0) {
- $averageattempts = (int) round($totalattempts / $userswhocanattempt);
- }
$content->add_item(get_string('averageattemptperstudent', 'mod_scorm'), $averageattempts);
return new overviewitem(
diff --git a/public/mod/scorm/classes/manager.php b/public/mod/scorm/classes/manager.php
index 132ec1ae849..da9f2cb99d3 100644
--- a/public/mod/scorm/classes/manager.php
+++ b/public/mod/scorm/classes/manager.php
@@ -160,6 +160,26 @@ class manager {
return $this->db->count_records_sql($query, $params);
}
+ /**
+ * Count the total number of attempts for the SCORM activity.
+ *
+ * @param array $groupids optional group id array, empty array means no group filtering.
+ * @return int the total number of attempts for the SCORM activity
+ */
+ public function count_all_attempts(array $groupids = []): int {
+ $params = ['scormid' => $this->instance->id];
+ $joins = '';
+ $where = "WHERE a.scormid = :scormid";
+ if ($groupids) {
+ $sqljoin = groups_get_members_join($groupids, 'a.userid', $this->context);
+ $joins = $sqljoin->joins;
+ $where .= " AND $sqljoin->wheres";
+ $params += $sqljoin->params;
+ }
+ $sql = "SELECT COUNT(DISTINCT a.id) FROM {scorm_attempt} a $joins $where";
+ return $this->db->count_records_sql($sql, $params);
+ }
+
/**
* Get the max attempt setting.
*
diff --git a/public/mod/scorm/tests/courseformat/overview_test.php b/public/mod/scorm/tests/courseformat/overview_test.php
index f3057c92c90..1fc1da77b0e 100644
--- a/public/mod/scorm/tests/courseformat/overview_test.php
+++ b/public/mod/scorm/tests/courseformat/overview_test.php
@@ -261,7 +261,7 @@ final class overview_test extends \advanced_testcase {
'content' => '2 of 4',
],
'totalattempts' => [
- 'value' => 2,
+ 'value' => 3,
'items' => [
[
'label' => 'Grading method',
@@ -273,12 +273,10 @@ final class overview_test extends \advanced_testcase {
],
[
'label' => 'Average attempts per student',
- 'value' => '1',
+ 'value' => '1.5',
],
],
-
],
-
],
];
yield 'teacher 1 - no groups without attempts' => [
@@ -320,7 +318,7 @@ final class overview_test extends \advanced_testcase {
'content' => '2 of 4',
],
'totalattempts' => [
- 'value' => 2,
+ 'value' => 3,
'items' => [
[
'label' => 'Grading method',
@@ -332,7 +330,7 @@ final class overview_test extends \advanced_testcase {
],
[
'label' => 'Average attempts per student',
- 'value' => '1',
+ 'value' => '1.5',
],
],
],
@@ -348,7 +346,7 @@ final class overview_test extends \advanced_testcase {
'content' => '1 of 2', // Teacher can also attempt, so s1 and t1 are counted.
],
'totalattempts' => [
- 'value' => 1, // Attempt from s1 only.
+ 'value' => 2, // Attempt from s1 only.
'items' => [
[
'label' => 'Grading method',
@@ -360,7 +358,7 @@ final class overview_test extends \advanced_testcase {
],
[
'label' => 'Average attempts per student',
- 'value' => '1',
+ 'value' => '2', // Only student 1 in this group attempted twice.
],
],
],
@@ -378,7 +376,7 @@ final class overview_test extends \advanced_testcase {
'content' => '2 of 4',
],
'totalattempts' => [
- 'value' => 2,
+ 'value' => 3,
'items' => [
[
'label' => 'Grading method',
@@ -390,7 +388,7 @@ final class overview_test extends \advanced_testcase {
],
[
'label' => 'Average attempts per student',
- 'value' => '1',
+ 'value' => '1.5',
],
],
],
@@ -406,7 +404,7 @@ final class overview_test extends \advanced_testcase {
'content' => '2 of 4',
],
'totalattempts' => [
- 'value' => 2,
+ 'value' => 3,
'items' => [
[
'label' => 'Grading method',
@@ -418,7 +416,7 @@ final class overview_test extends \advanced_testcase {
],
[
'label' => 'Average attempts per student',
- 'value' => '1',
+ 'value' => '1.5',
],
],
],
@@ -489,8 +487,7 @@ final class overview_test extends \advanced_testcase {
// attempt for testing purposes.
if ($createattempt && $createusers) {
$scormgenerator = $this->getDataGenerator()->get_plugin_generator('mod_scorm');
- // Create attempts for the students.
- // Two attempts for the first student, one for the second.
+ // Create an attempt for each student, and two attempts for the student s1.
foreach ($data as $username => $userinfo) {
$record = [
'userid' => $users[$username]->id,
@@ -500,8 +497,11 @@ final class overview_test extends \advanced_testcase {
$record['element'] = 'cmi.core.score.raw';
$record['value'] = $grades[$username];
}
- // Create an attempt for each student.
+ // Create two attempts for the first student.
if ($userinfo['role'] === 'student') {
+ if ($username === 's1') {
+ $scormgenerator->create_attempt($record);
+ }
$scormgenerator->create_attempt($record);
}
}
diff --git a/public/mod/scorm/tests/manager_test.php b/public/mod/scorm/tests/manager_test.php
index da33a2151c8..c6f01fdb782 100644
--- a/public/mod/scorm/tests/manager_test.php
+++ b/public/mod/scorm/tests/manager_test.php
@@ -98,7 +98,7 @@ final class manager_test extends \advanced_testcase {
*/
public function test_can_view_reports(): void {
$this->resetAfterTest();
- ['users' => $users, 'course' => $course, 'instances' => $instances] = $this->setup_users_and_activity();
+ ['users' => $users, 'instances' => $instances] = $this->setup_users_and_activity();
$manager = \mod_scorm\manager::create_from_instance($instances['withattempts']);
// Create an attempt for the current user.
$this->assertTrue($manager->can_view_reports($users['t1']));
@@ -132,8 +132,7 @@ final class manager_test extends \advanced_testcase {
*/
public function test_count_users_who_attempted(int $groupmode, string $activity, array $expected): void {
$this->resetAfterTest();
- ['users' => $users, 'course' => $course, 'instances' => $instances] =
- $this->setup_users_and_activity(groupmode: $groupmode);
+ ['users' => $users, 'instances' => $instances] = $this->setup_users_and_activity(groupmode: $groupmode);
$manager = \mod_scorm\manager::create_from_instance($instances[$activity]);
// Check the count of users who attempted.
@@ -193,6 +192,80 @@ final class manager_test extends \advanced_testcase {
];
}
+ /**
+ * Count the number of attempts for the SCORM activity.
+ *
+ * @param int $groupmode the group mode to use for the course.
+ * @param string $activity the activity name to test.
+ * @param array $currentgroups
+ * @param int $expectedcount
+ * @throws \moodle_exception
+ */
+ #[\PHPUnit\Framework\Attributes\DataProvider('get_count_all_attempts_data')]
+ public function test_count_all_attempts(int $groupmode, string $activity, array $currentgroups, int $expectedcount): void {
+ $this->resetAfterTest();
+ ['instances' => $instances, 'groups' => $groups] = $this->setup_users_and_activity(groupmode: $groupmode);
+ $manager = \mod_scorm\manager::create_from_instance($instances[$activity]);
+ $groupids = array_map(
+ fn($gname) => $groups[$gname]->id,
+ $currentgroups
+ );
+ $this->assertEquals(
+ $expectedcount,
+ $manager->count_all_attempts($groupids),
+ );
+ }
+
+ /**
+ * Data provider for participant count tests.
+ *
+ * @return \Generator
+ */
+ public static function get_count_all_attempts_data(): \Generator {
+ yield 'No groups' => [
+ 'groupmode' => NOGROUPS,
+ 'activity' => 'withattempts',
+ 'currentgroups' => [],
+ 'expectedcount' => 3,
+ ];
+ yield 'Separate groups, g1 selected' => [
+ 'groupmode' => SEPARATEGROUPS,
+ 'activity' => 'withattempts',
+ 'currentgroups' => ['g1'],
+ 'expectedcount' => 2,
+ ];
+ yield 'Separate groups, g2 selected' => [
+ 'groupmode' => SEPARATEGROUPS,
+ 'activity' => 'withattempts',
+ 'currentgroups' => ['g2'],
+ 'expectedcount' => 1,
+ ];
+ yield 'Separate groups, g1 and g2 selected' => [
+ 'groupmode' => SEPARATEGROUPS,
+ 'activity' => 'withattempts',
+ 'currentgroups' => ['g1', 'g2'],
+ 'expectedcount' => 3,
+ ];
+ yield 'Separate groups, no group selected' => [
+ 'groupmode' => SEPARATEGROUPS,
+ 'activity' => 'withattempts',
+ 'currentgroups' => [],
+ 'expectedcount' => 3, // Seems counter-intuitive but with no group selected we count all attempts.
+ ];
+ yield 'Visible groups' => [
+ 'groupmode' => VISIBLEGROUPS,
+ 'activity' => 'withattempts',
+ 'currentgroups' => ['g1', 'g2'],
+ 'expectedcount' => 3,
+ ];
+ yield 'Visible groups g1 selected' => [
+ 'groupmode' => VISIBLEGROUPS,
+ 'activity' => 'withattempts',
+ 'currentgroups' => ['g1'],
+ 'expectedcount' => 2,
+ ];
+ }
+
/**
* Test if the manager can view reports for a user.
*
@@ -392,6 +465,7 @@ final class manager_test extends \advanced_testcase {
'users' => $users,
'course' => $course,
'instances' => $instances,
+ 'groups' => $groups,
];
}
}