diff --git a/public/mod/lesson/classes/courseformat/overview.php b/public/mod/lesson/classes/courseformat/overview.php index d831149c6c4..3df8eab351d 100644 --- a/public/mod/lesson/classes/courseformat/overview.php +++ b/public/mod/lesson/classes/courseformat/overview.php @@ -113,8 +113,10 @@ class overview extends \core_courseformat\activityoverviewbase { return null; } - $attemptedusers = $this->lesson->count_submitted_participants(); - $totalusers = $this->lesson->count_all_participants(); + $groups = array_map(fn($group) => $group->id, $this->get_groups_for_filtering()); + + $attemptedusers = $this->lesson->count_submitted_participants($groups); + $totalusers = $this->lesson->count_all_participants($groups); return new overviewitem( name: $this->stringmanager->get_string('studentswhoattempted', 'mod_lesson'), @@ -138,10 +140,12 @@ class overview extends \core_courseformat\activityoverviewbase { return null; } - $totalattempts = $this->lesson->count_all_submissions(); + $groups = array_map(fn($group) => $group->id, $this->get_groups_for_filtering()); + + $totalattempts = $this->lesson->count_all_submissions($groups); if ($this->lesson->retake) { - $attemptedusers = $this->lesson->count_submitted_participants(); + $attemptedusers = $this->lesson->count_submitted_participants($groups); $overviewdialog = new overviewdialog( buttoncontent: $totalattempts, diff --git a/public/mod/lesson/locallib.php b/public/mod/lesson/locallib.php index 189e2e3bb2f..fc002af00cc 100644 --- a/public/mod/lesson/locallib.php +++ b/public/mod/lesson/locallib.php @@ -3036,12 +3036,17 @@ class lesson extends lesson_base { /** * Count all submissions by all users in the lesson. * + * @param array $groups the groups to filter by. * @return int the number of submissions (grades table) by all users in the lesson */ - public function count_all_submissions(): int { + public function count_all_submissions(array $groups): int { $db = \core\di::get(\moodle_database::class); - [$esql, $eparams] = get_enrolled_sql($this->get_context(), 'mod/lesson:view'); + [$esql, $eparams] = get_enrolled_sql( + context: $this->get_context(), + withcapability: 'mod/lesson:view', + groupids: $groups, + ); $sql = "SELECT COUNT(lg.id) FROM {lesson_grades} lg JOIN ($esql) e ON e.id = lg.userid @@ -3053,11 +3058,16 @@ class lesson extends lesson_base { /** * Count the number of participants that have attempted the lesson. * + * @param array $groups the groups to filter by. * @return int the number of users that have attempted the lesson */ - public function count_submitted_participants(): int { + public function count_submitted_participants(array $groups): int { $db = \core\di::get(\moodle_database::class); - [$esql, $eparams] = get_enrolled_sql($this->get_context(), 'mod/lesson:view'); + [$esql, $eparams] = get_enrolled_sql( + context: $this->get_context(), + withcapability: 'mod/lesson:view', + groupids: $groups, + ); $sql = "SELECT COUNT(DISTINCT lg.userid) FROM {lesson_grades} lg JOIN ($esql) e ON e.id = lg.userid @@ -3069,12 +3079,18 @@ class lesson extends lesson_base { /** * Count the number of participants that have access to the lesson. * + * @param array $groups the groups to filter by. * @return int the number of users that have access to view the lesson */ - public function count_all_participants(): int { + public function count_all_participants(array $groups): int { $db = \core\di::get(\moodle_database::class); - $join = get_enrolled_with_capabilities_join($this->get_context(), '', 'mod/lesson:view'); + $join = get_enrolled_with_capabilities_join( + context: $this->get_context(), + prefix: '', + capability: 'mod/lesson:view', + groupids: $groups + ); $managersjoin = get_with_capability_join($this->get_context(), 'mod/lesson:manage', 'u.id'); if (!$managersjoin->cannotmatchanyrows) { $join = new \core\dml\sql_join( diff --git a/public/mod/lesson/tests/courseformat/overview_test.php b/public/mod/lesson/tests/courseformat/overview_test.php index 0dd138429ca..53fa970fba5 100644 --- a/public/mod/lesson/tests/courseformat/overview_test.php +++ b/public/mod/lesson/tests/courseformat/overview_test.php @@ -169,6 +169,7 @@ final class overview_test extends \advanced_testcase { * @dataProvider provider_test_get_extra_totalattempts_overview * * @param string $role + * @param int $groupmode * @param bool $hasentries * @param bool $hasretakes * @param array|null $expected @@ -176,6 +177,7 @@ final class overview_test extends \advanced_testcase { */ public function test_get_extra_totalattempts_overview( string $role, + int $groupmode, bool $hasentries, bool $hasretakes, ?array $expected @@ -185,11 +187,18 @@ final class overview_test extends \advanced_testcase { $course = $this->getDataGenerator()->create_course(); $student1 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student2 = $this->getDataGenerator()->create_and_enrol($course, 'student'); $currentuser = $this->getDataGenerator()->create_and_enrol($course, $role); + if ($groupmode != NOGROUPS) { + $group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->getDataGenerator()->create_group_member(['userid' => $currentuser->id, 'groupid' => $group1->id]); + $this->getDataGenerator()->create_group_member(['userid' => $student1->id, 'groupid' => $group1->id]); + } + $lessonmodule = $this->getDataGenerator()->create_module( 'lesson', - ['course' => $course, 'retake' => $hasretakes] + ['course' => $course, 'retake' => $hasretakes, 'groupmode' => $groupmode] ); $cm = get_fast_modinfo($course)->get_cm($lessonmodule->cmid); $lesson = new lesson($lessonmodule); @@ -203,6 +212,11 @@ final class overview_test extends \advanced_testcase { 'userid' => $student1->id, 'grade' => 100, ]); + $lessongenerator->create_submission([ + 'lessonid' => $lesson->id, + 'userid' => $student2->id, + 'grade' => 100, + ]); $lessongenerator->create_submission([ 'lessonid' => $lesson->id, 'userid' => $currentuser->id, @@ -237,15 +251,37 @@ final class overview_test extends \advanced_testcase { return [ 'Teacher (with attempts)' => [ 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, 'hasentries' => true, 'hasretakes' => true, 'expected' => [ 'name' => get_string('totalattepmts', 'mod_lesson'), - 'value' => 2, + 'value' => 3, + ], + ], + 'Teacher (with attempts) (Separate Groups)' => [ + 'role' => 'editingteacher', + 'groupmode' => SEPARATEGROUPS, + 'hasentries' => true, + 'hasretakes' => true, + 'expected' => [ + 'name' => get_string('totalattepmts', 'mod_lesson'), + 'value' => 3, + ], + ], + 'Teacher (with attempts) (Visible Groups)' => [ + 'role' => 'editingteacher', + 'groupmode' => VISIBLEGROUPS, + 'hasentries' => true, + 'hasretakes' => true, + 'expected' => [ + 'name' => get_string('totalattepmts', 'mod_lesson'), + 'value' => 3, ], ], 'Teacher (with attempts without retakes)' => [ 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, 'hasentries' => true, 'hasretakes' => false, 'expected' => [ @@ -255,6 +291,7 @@ final class overview_test extends \advanced_testcase { ], 'Teacher (without attempts)' => [ 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, 'hasentries' => false, 'hasretakes' => true, 'expected' => [ @@ -262,8 +299,39 @@ final class overview_test extends \advanced_testcase { 'value' => 0, ], ], + 'Non-editing Teacher (with attempts)' => [ + 'role' => 'teacher', + 'groupmode' => NOGROUPS, + 'hasentries' => true, + 'hasretakes' => true, + 'expected' => [ + 'name' => get_string('totalattepmts', 'mod_lesson'), + 'value' => 3, + ], + ], + 'Non-editing Teacher (with attempts) (Separate Groups)' => [ + 'role' => 'teacher', + 'groupmode' => SEPARATEGROUPS, + 'hasentries' => true, + 'hasretakes' => true, + 'expected' => [ + 'name' => get_string('totalattepmts', 'mod_lesson'), + 'value' => 2, + ], + ], + 'Non-editing Teacher (with attempts) (Visible Groups)' => [ + 'role' => 'teacher', + 'groupmode' => VISIBLEGROUPS, + 'hasentries' => true, + 'hasretakes' => true, + 'expected' => [ + 'name' => get_string('totalattepmts', 'mod_lesson'), + 'value' => 3, + ], + ], 'Student' => [ 'role' => 'student', + 'groupmode' => NOGROUPS, 'hasentries' => true, 'hasretakes' => true, 'expected' => null, @@ -278,12 +346,14 @@ final class overview_test extends \advanced_testcase { * @dataProvider provider_test_get_extra_attemptedstudents_overview * * @param string $role + * @param bool $groupmode * @param bool $hasentries * @param array|null $expected * @return void */ public function test_get_extra_attemptedstudents_overview( string $role, + int $groupmode, bool $hasentries, ?array $expected ): void { @@ -295,7 +365,16 @@ final class overview_test extends \advanced_testcase { $student2 = $this->getDataGenerator()->create_and_enrol($course, 'student'); $currentuser = $this->getDataGenerator()->create_and_enrol($course, $role); - $lessonmodule = $this->getDataGenerator()->create_module('lesson', ['course' => $course]); + if ($groupmode != NOGROUPS) { + $group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->getDataGenerator()->create_group_member(['userid' => $currentuser->id, 'groupid' => $group1->id]); + $this->getDataGenerator()->create_group_member(['userid' => $student1->id, 'groupid' => $group1->id]); + } + + $lessonmodule = $this->getDataGenerator()->create_module( + 'lesson', + ['course' => $course, 'groupmode' => $groupmode] + ); $cm = get_fast_modinfo($course)->get_cm($lessonmodule->cmid); $lesson = new lesson($lessonmodule); /** @var \mod_lesson_generator $lessongenerator */ @@ -308,6 +387,11 @@ final class overview_test extends \advanced_testcase { 'userid' => $student1->id, 'grade' => 100, ]); + $lessongenerator->create_submission([ + 'lessonid' => $lesson->id, + 'userid' => $student2->id, + 'grade' => 100, + ]); $lessongenerator->create_submission([ 'lessonid' => $lesson->id, 'userid' => $currentuser->id, @@ -342,22 +426,70 @@ final class overview_test extends \advanced_testcase { return [ 'Teacher (with attempts)' => [ 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, 'hasentries' => true, 'expected' => [ 'name' => get_string('studentswhoattempted', 'mod_lesson'), - 'value' => 2, + 'value' => 3, + ], + ], + 'Teacher (with attempts) (Separate Groups)' => [ + 'role' => 'editingteacher', + 'groupmode' => SEPARATEGROUPS, + 'hasentries' => true, + 'expected' => [ + 'name' => get_string('studentswhoattempted', 'mod_lesson'), + 'value' => 3, + ], + ], + 'Teacher (with attempts) (Visible Groups)' => [ + 'role' => 'editingteacher', + 'groupmode' => VISIBLEGROUPS, + 'hasentries' => true, + 'expected' => [ + 'name' => get_string('studentswhoattempted', 'mod_lesson'), + 'value' => 3, ], ], 'Teacher (without attempts)' => [ 'role' => 'editingteacher', + 'groupmode' => NOGROUPS, 'hasentries' => false, 'expected' => [ 'name' => get_string('studentswhoattempted', 'mod_lesson'), 'value' => 0, ], ], + 'Non-editing Teacher (with attempts)' => [ + 'role' => 'teacher', + 'groupmode' => NOGROUPS, + 'hasentries' => true, + 'expected' => [ + 'name' => get_string('studentswhoattempted', 'mod_lesson'), + 'value' => 3, + ], + ], + 'Non-editing Teacher (with attempts) (Separate Groups)' => [ + 'role' => 'teacher', + 'groupmode' => SEPARATEGROUPS, + 'hasentries' => true, + 'expected' => [ + 'name' => get_string('studentswhoattempted', 'mod_lesson'), + 'value' => 2, + ], + ], + 'Non-editing Teacher (with attempts) (Visible Groups)' => [ + 'role' => 'teacher', + 'groupmode' => VISIBLEGROUPS, + 'hasentries' => true, + 'expected' => [ + 'name' => get_string('studentswhoattempted', 'mod_lesson'), + 'value' => 3, + ], + ], 'Student' => [ 'role' => 'student', + 'groupmode' => NOGROUPS, 'hasentries' => true, 'expected' => null, ], diff --git a/public/mod/lesson/tests/locallib_test.php b/public/mod/lesson/tests/locallib_test.php index 01670076ac0..29a2dc0f2b7 100644 --- a/public/mod/lesson/tests/locallib_test.php +++ b/public/mod/lesson/tests/locallib_test.php @@ -365,15 +365,15 @@ final class locallib_test extends \advanced_testcase { $this->setUser($teacher->id); - $this->assertEquals(5, $lesson->count_all_submissions()); - $this->assertEquals(3, $lesson->count_submitted_participants()); - $this->assertEquals(4, $lesson->count_all_participants()); + $this->assertEquals(5, $lesson->count_all_submissions([])); + $this->assertEquals(3, $lesson->count_submitted_participants([])); + $this->assertEquals(4, $lesson->count_all_participants([])); // Check that the lesson is not counting teachers as participants. $teacher2 = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); - $this->assertEquals(4, $lesson->count_all_participants()); + $this->assertEquals(4, $lesson->count_all_participants([])); $student5 = $this->getDataGenerator()->create_and_enrol($course, 'student'); - $this->assertEquals(5, $lesson->count_all_participants()); + $this->assertEquals(5, $lesson->count_all_participants([])); // Prohibit mod/lesson:view capability on student role to ensure it does not count students as participants/submissions. $studentrole = $DB->get_record('role', ['shortname' => 'student']); @@ -383,8 +383,64 @@ final class locallib_test extends \advanced_testcase { $studentrole->id, context_module::instance($lesson->get_cm()->id) ); - $this->assertEquals(0, $lesson->count_all_submissions()); - $this->assertEquals(0, $lesson->count_submitted_participants()); - $this->assertEquals(0, $lesson->count_all_participants()); + $this->assertEquals(0, $lesson->count_all_submissions([])); + $this->assertEquals(0, $lesson->count_submitted_participants([])); + $this->assertEquals(0, $lesson->count_all_participants([])); + } + + /** + * Test the count_all_attempts, count_attempted_participants and count_all_participants methods with groups. + * + * @covers \lesson::count_all_submissions + * @covers \lesson::count_submitted_participants + * @covers \lesson::count_all_participants + */ + public function test_count_attempts_and_participants_with_groups(): void { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $course = $this->getDataGenerator()->create_course(); + $student1 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student2 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student3 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student4 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + + $lessonrecord = $this->getDataGenerator()->create_module( + 'lesson', + ['course' => $course, 'retake' => 1, 'groupmode' => SEPARATEGROUPS] + ); + + $lesson = new lesson($lessonrecord); + $this->create_lesson_pages($lesson, 2); + $this->create_user_submissions($lesson, $student1->id, 1); + $this->create_user_submissions($lesson, $student2->id, 2); + $this->create_user_submissions($lesson, $student3->id, 2); + + $group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $group2 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->getDataGenerator()->create_group_member(['userid' => $student1->id, 'groupid' => $group1->id]); + $this->getDataGenerator()->create_group_member(['userid' => $student2->id, 'groupid' => $group1->id]); + $this->getDataGenerator()->create_group_member(['userid' => $student4->id, 'groupid' => $group2->id]); + + // The following data was created for the lesson: + // Student 1: group 1, with 1 submission. + // Student 2: group 1, with 2 submissions. + // Student 3: not in a group, with 2 submissions. + // Student 4: group 2, with no submissions. + + $this->assertEquals(3, $lesson->count_all_submissions([$group1->id])); + $this->assertEquals(2, $lesson->count_submitted_participants([$group1->id])); + $this->assertEquals(2, $lesson->count_all_participants([$group1->id])); + + $this->assertEquals(3, $lesson->count_all_submissions([$group1->id, $group2->id])); + $this->assertEquals(2, $lesson->count_submitted_participants([$group1->id, $group2->id])); + $this->assertEquals(3, $lesson->count_all_participants([$group1->id, $group2->id])); + + // Check that the lesson is not counting teachers as participants. + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $this->getDataGenerator()->create_group_member(['userid' => $teacher->id, 'groupid' => $group1->id]); + $this->assertEquals(2, $lesson->count_all_participants([$group1->id])); } }