From fa6928250e9dbc36eae05ed5464edffeae135635 Mon Sep 17 00:00:00 2001
From: Jake Dallimore <jake@moodle.com>
Date: Wed, 20 Sep 2023 15:38:07 +0800
Subject: [PATCH] MDL-79423 mod_lti: add tool state filtering to LTI tools
 report

The report should only include those tools which are fully configured,
per the LTI_TOOL_STATE_CONFIGURED constant.
---
 .../systemreports/course_external_tools_list.php | 16 +++++++++++++---
 mod/lti/tests/behat/managecoursetools.feature    | 11 +++++++----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/mod/lti/classes/reportbuilder/local/systemreports/course_external_tools_list.php b/mod/lti/classes/reportbuilder/local/systemreports/course_external_tools_list.php
index 43fca299cd4ae..99feda7af1659 100644
--- a/mod/lti/classes/reportbuilder/local/systemreports/course_external_tools_list.php
+++ b/mod/lti/classes/reportbuilder/local/systemreports/course_external_tools_list.php
@@ -75,10 +75,20 @@ protected function initialise(): void {
         $paramprefix = database::generate_param_name();
         $coursevisibleparam = database::generate_param_name();
         $categoryparam = database::generate_param_name();
+        $toolstateparam = database::generate_param_name();
         [$insql, $params] = $DB->get_in_or_equal([get_site()->id, $this->course->id], SQL_PARAMS_NAMED, "{$paramprefix}_");
-        $wheresql = "{$entitymainalias}.course {$insql} AND {$entitymainalias}.coursevisible NOT IN (:{$coursevisibleparam}) ".
-        "AND ({$cattablealias}.id IS NULL OR {$cattablealias}.categoryid = :{$categoryparam})";
-        $params = array_merge($params, [$coursevisibleparam => LTI_COURSEVISIBLE_NO, $categoryparam => $this->course->category]);
+        $wheresql = "{$entitymainalias}.course {$insql} ".
+            "AND {$entitymainalias}.coursevisible NOT IN (:{$coursevisibleparam}) ".
+            "AND ({$cattablealias}.id IS NULL OR {$cattablealias}.categoryid = :{$categoryparam}) ".
+            "AND {$entitymainalias}.state = :{$toolstateparam}";
+        $params = array_merge(
+            $params,
+            [
+                $coursevisibleparam => LTI_COURSEVISIBLE_NO,
+                $categoryparam => $this->course->category,
+                $toolstateparam => LTI_TOOL_STATE_CONFIGURED
+            ]
+        );
         $this->add_base_condition_sql($wheresql, $params);
 
         $this->set_downloadable(false, get_string('pluginname', 'mod_lti'));
diff --git a/mod/lti/tests/behat/managecoursetools.feature b/mod/lti/tests/behat/managecoursetools.feature
index c51cf74e212e1..2931be9b9dee7 100644
--- a/mod/lti/tests/behat/managecoursetools.feature
+++ b/mod/lti/tests/behat/managecoursetools.feature
@@ -34,11 +34,13 @@ Feature: Manage course tools
     And I should see "A short description of the tool" in the "Teaching Tool 1" "table_row"
 
   Scenario: Viewing a site level tool in the course tools table
+    # The first tool isn't visible in courses, the next two are, and the last tool is in a pending state and is not visible.
     Given the following "mod_lti > tool types" exist:
-      | name         | description         | baseurl                   | coursevisible |
-      | Example tool | Another description | https://example.com/tool1 | 0             |
-      | Test tool 2  | Tool2 description   | https://example.com/tool2 | 1             |
-      | Test tool 3  | Tool3 description   | https://example.com/tool3 | 2             |
+      | name         | description         | baseurl                   | coursevisible | state |
+      | Example tool | Another description | https://example.com/tool1 | 0             | 1     |
+      | Test tool 2  | Tool2 description   | https://example.com/tool2 | 1             | 1     |
+      | Test tool 3  | Tool3 description   | https://example.com/tool3 | 2             | 1     |
+      | Test tool 4  | Tool4 description   | https://example.com/tool4 | 2             | 2     |
     And I am on the "Course 1" course page logged in as teacher1
     When I navigate to "LTI External tools" in current page administration
     Then I should see "Test tool 2" in the "reportbuilder-table" "table"
@@ -46,6 +48,7 @@ Feature: Manage course tools
     And I should see "Test tool 3" in the "reportbuilder-table" "table"
     And "You don't have permission to edit this tool" "icon" should exist in the "Test tool 3" "table_row"
     And I should not see "Example tool" in the "reportbuilder-table" "table"
+    And I should not see "Test tool 4" in the "reportbuilder-table" "table"
 
   Scenario: Viewing course tools without the capability to add/edit but having the capability to use
     Given the following "role capability" exists: