diff --git a/mod/h5pactivity/classes/local/manager.php b/mod/h5pactivity/classes/local/manager.php index aaaabb58f50..412aa2c346b 100644 --- a/mod/h5pactivity/classes/local/manager.php +++ b/mod/h5pactivity/classes/local/manager.php @@ -332,15 +332,24 @@ class manager { * * @since Moodle 3.11 * @param bool $allpotentialusers if true, the join will return all active users, not only the ones with attempts. + * @param int|bool $currentgroup False if groups not used, 0 for all groups, group id (int) to filter by specific group * @return sql_join the active users attempts join */ - public function get_active_users_join(bool $allpotentialusers = false): sql_join { + public function get_active_users_join(bool $allpotentialusers = false, $currentgroup = false): sql_join { // Only valid users counts. By default, all users with submit capability are considered potential ones. $context = $this->get_context(); + $coursemodule = $this->get_coursemodule(); + + // Ensure user can view users from all groups. + if ($currentgroup === 0 && $coursemodule->effectivegroupmode == SEPARATEGROUPS + && !has_capability('moodle/site:accessallgroups', $context)) { + + return new sql_join('', '1=2', [], true); + } // We want to present all potential users. - $capjoin = get_enrolled_with_capabilities_join($context, '', 'mod/h5pactivity:view'); + $capjoin = get_enrolled_with_capabilities_join($context, '', 'mod/h5pactivity:view', $currentgroup); if ($capjoin->cannotmatchanyrows) { return $capjoin; @@ -438,9 +447,10 @@ class manager { * * @param int $userid an opional userid to show * @param int $attemptid an optional $attemptid to show + * @param int|bool $currentgroup False if groups not used, 0 for all groups, group id (int) to filter by specific group * @return report|null available report (or null if no report available) */ - public function get_report(int $userid = null, int $attemptid = null): ?report { + public function get_report(int $userid = null, int $attemptid = null, $currentgroup = false): ?report { global $USER; // If tracking is disabled, no reports are available. @@ -480,7 +490,7 @@ class manager { } else if ($user) { return new attempts($this, $user); } - return new participants($this); + return new participants($this, $currentgroup); } /** diff --git a/mod/h5pactivity/classes/local/report/participants.php b/mod/h5pactivity/classes/local/report/participants.php index a9e6f43df36..5dff01d124c 100644 --- a/mod/h5pactivity/classes/local/report/participants.php +++ b/mod/h5pactivity/classes/local/report/participants.php @@ -61,8 +61,9 @@ class participants extends table_sql implements report { * Create a new participants report. * * @param manager $manager h5pactivitymanager object + * @param int|bool $currentgroup False if groups not used, 0 for all groups, group id (int) to filter by specific group */ - public function __construct(manager $manager) { + public function __construct(manager $manager, $currentgroup = false) { parent::__construct('mod_h5pactivity-participants'); $this->manager = $manager; $this->scores = $manager->get_users_scaled_score(); @@ -83,7 +84,7 @@ class participants extends table_sql implements report { $this->no_sorting('attempts'); $this->pageable(true); - $capjoin = $this->manager->get_active_users_join(true); + $capjoin = $this->manager->get_active_users_join(true, $currentgroup); // Final SQL. $this->set_sql( diff --git a/mod/h5pactivity/report.php b/mod/h5pactivity/report.php index e1ed52a8b69..6433f0f1213 100644 --- a/mod/h5pactivity/report.php +++ b/mod/h5pactivity/report.php @@ -43,9 +43,11 @@ if (empty($id)) { require_login($course, true, $cm); +$currentgroup = groups_get_activity_group($cm, true); + $manager = manager::create_from_coursemodule($cm); -$report = $manager->get_report($userid, $attemptid); +$report = $manager->get_report($userid, $attemptid, $currentgroup); if (!$report) { print_error('permissiondenied'); } @@ -125,6 +127,8 @@ $PAGE->set_context($context); echo $OUTPUT->header(); +groups_print_activity_menu($cm, $PAGE->url); + echo $report->print(); echo $OUTPUT->footer(); diff --git a/mod/h5pactivity/tests/local/manager_test.php b/mod/h5pactivity/tests/local/manager_test.php index 4dba608b07d..86841329a36 100644 --- a/mod/h5pactivity/tests/local/manager_test.php +++ b/mod/h5pactivity/tests/local/manager_test.php @@ -14,14 +14,6 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * mod_h5pactivity manager tests - * - * @package mod_h5pactivity - * @category test - * @copyright 2020 Ferran Recio - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ namespace mod_h5pactivity\local; use context_module; @@ -31,6 +23,7 @@ use stdClass; * Manager tests class for mod_h5pactivity. * * @package mod_h5pactivity + * @covers \mod_h5pactivity\local\manager * @category test * @copyright 2020 Ferran Recio * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later @@ -627,7 +620,7 @@ class manager_test extends \advanced_testcase { } /** - * Test static count_attempts of all active participants. + * Test static test_get_active_users_join of all active participants. * * Most method scenarios are tested in test_count_attempts_all so we only * need to test the with $allpotentialusers true and false. @@ -692,6 +685,58 @@ class manager_test extends \advanced_testcase { ]; } + /** + * Test active users joins returns appropriate results for groups + */ + public function test_get_active_users_join_groupmode(): void { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $course = $this->getDataGenerator()->create_course(['groupmode' => SEPARATEGROUPS, 'groupmodeforce' => 1]); + + // Teacher/user one in group one. + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $userone = $this->getDataGenerator()->create_and_enrol($course, 'student'); + + $groupone = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->getDataGenerator()->create_group_member(['groupid' => $groupone->id, 'userid' => $teacher->id]); + $this->getDataGenerator()->create_group_member(['groupid' => $groupone->id, 'userid' => $userone->id]); + + // User two in group two. + $usertwo = $this->getDataGenerator()->create_and_enrol($course, 'student'); + + $grouptwo = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $this->getDataGenerator()->create_group_member(['groupid' => $grouptwo->id, 'userid' => $usertwo->id]); + + $activity = $this->getDataGenerator()->create_module('h5pactivity', ['course' => $course]); + $manager = manager::create_from_instance($activity); + + // Admin user can view all participants. + $usersjoin = $manager->get_active_users_join(true, 0); + $users = $DB->get_fieldset_sql("SELECT u.username FROM {user} u {$usersjoin->joins} WHERE {$usersjoin->wheres}", + $usersjoin->params); + + $this->assertEqualsCanonicalizing([$teacher->username, $userone->username, $usertwo->username], $users); + + // Switch to teacher, who cannot view all participants. + $this->setUser($teacher); + + $usersjoin = $manager->get_active_users_join(true, 0); + $users = $DB->get_fieldset_sql("SELECT u.username FROM {user} u {$usersjoin->joins} WHERE {$usersjoin->wheres}", + $usersjoin->params); + + $this->assertEmpty($users); + + // Teacher can view participants inside group. + $usersjoin = $manager->get_active_users_join(true, $groupone->id); + $users = $DB->get_fieldset_sql("SELECT u.username FROM {user} u {$usersjoin->joins} WHERE {$usersjoin->wheres}", + $usersjoin->params); + + $this->assertEqualsCanonicalizing([$teacher->username, $userone->username], $users); + } + /** * Test static count_attempts. */ diff --git a/mod/h5pactivity/upgrade.txt b/mod/h5pactivity/upgrade.txt index 70435b5d661..d21d2f8c45e 100644 --- a/mod/h5pactivity/upgrade.txt +++ b/mod/h5pactivity/upgrade.txt @@ -1,6 +1,13 @@ This files describes API changes in /mod/h5pactivity/*, information provided here is intended especially for developers. +=== 3.11.9 === + +* The following methods now accept an optional `$currentgroup` parameter in order to better support groups: + - `\mod_h5pactivity\local\manager::get_active_users_join` + - `\mod_h5pactivity\local\manager::get_report` + - `\mod_h5pactivity\local\report\participants` constructor + === 3.11 === * Added mod_h5pactivity\local\manager::get_active_users_join method to query all active