From 6813fd409477f91511071830029ebb3331f2beeb Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Mon, 25 Jul 2022 18:21:13 +0100
Subject: [PATCH] MDL-75300 mod_h5pactivity: check whether reviewer join
 matches rows.

Return original capability join if so, to prevent generating invalid
SQL.
---
 mod/h5pactivity/classes/local/manager.php    |  3 ++
 mod/h5pactivity/tests/local/manager_test.php | 35 ++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/mod/h5pactivity/classes/local/manager.php b/mod/h5pactivity/classes/local/manager.php
index 7522024ed22..ee8e50e0cfd 100644
--- a/mod/h5pactivity/classes/local/manager.php
+++ b/mod/h5pactivity/classes/local/manager.php
@@ -357,6 +357,9 @@ class manager {
 
         // But excluding all reviewattempts users converting a capabilities join into left join.
         $reviewersjoin = get_with_capability_join($context, 'mod/h5pactivity:reviewattempts', 'u.id');
+        if ($reviewersjoin->cannotmatchanyrows) {
+            return $capjoin;
+        }
 
         $capjoin = new sql_join(
             $capjoin->joins . "\n LEFT " . str_replace('ra', 'reviewer', $reviewersjoin->joins),
diff --git a/mod/h5pactivity/tests/local/manager_test.php b/mod/h5pactivity/tests/local/manager_test.php
index 86841329a36..ba7f8453d4d 100644
--- a/mod/h5pactivity/tests/local/manager_test.php
+++ b/mod/h5pactivity/tests/local/manager_test.php
@@ -737,6 +737,41 @@ class manager_test extends \advanced_testcase {
         $this->assertEqualsCanonicalizing([$teacher->username, $userone->username], $users);
     }
 
+    /**
+     * Test getting active users join where there are no roles with 'mod/h5pactivity:reviewattempts' capability
+     */
+    public function test_get_active_users_join_no_reviewers(): void {
+        global $DB;
+
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $course = $this->getDataGenerator()->create_course();
+        $activity = $this->getDataGenerator()->create_module('h5pactivity', ['course' => $course]);
+        $user = $this->getDataGenerator()->create_and_enrol($course, 'student');
+
+        $manager = manager::create_from_instance($activity);
+
+        // By default manager and editingteacher can review attempts, prohibit both.
+        $rolemanager = $DB->get_field('role', 'id', ['shortname' => 'manager']);
+        role_change_permission($rolemanager, $manager->get_context(), 'mod/h5pactivity:reviewattempts', CAP_PROHIBIT);
+
+        $roleeditingteacher = $DB->get_field('role', 'id', ['shortname' => 'editingteacher']);
+        role_change_permission($roleeditingteacher, $manager->get_context(), 'mod/h5pactivity:reviewattempts', CAP_PROHIBIT);
+
+        // Generate users join SQL to find matching users.
+        $usersjoin = $manager->get_active_users_join(true);
+        $usernames = $DB->get_fieldset_sql(
+            "SELECT u.username
+               FROM {user} u
+                    {$usersjoin->joins}
+              WHERE {$usersjoin->wheres}",
+            $usersjoin->params
+        );
+
+        $this->assertEquals([$user->username], $usernames);
+    }
+
     /**
      * Test static count_attempts.
      */