For reports with query parameters, allow param values to be set in th…

…e URL
moodleou · May 23, 2019 · 775b76c · 775b76c
1 parent 7bc5d88
commit 775b76c
Show file tree

Hide file tree

Showing 7 changed files with 122 additions and 41 deletions.
diff --git a/edit.php b/edit.php
@@ -56,10 +56,7 @@
 }
 
 $querysql = optional_param('querysql', $reportquerysql, PARAM_RAW);
-$queryparams = array();
-foreach (report_customsql_get_query_placeholders($querysql) as $queryparam) {
-    $queryparams[substr($queryparam, 1)] = 'queryparam' . substr($queryparam, 1);
-}
+$queryparams = report_customsql_get_query_placeholders_and_field_names($querysql);
 
 $mform = new report_customsql_edit_form(report_customsql_url($relativeurl), $queryparams);
 

diff --git a/edit_form.php b/edit_form.php
@@ -157,20 +157,18 @@ public function validation($data, $files) {
             $sql = report_customsql_prepare_sql($report, time());
 
             // Check for required query parameters if there are any.
-            $queryparams = array();
-            foreach (report_customsql_get_query_placeholders($sql) as $queryparam) {
-                $queryparam = substr($queryparam, 1);
-                $formparam = 'queryparam' . $queryparam;
+            $paramvalues = [];
+            foreach (report_customsql_get_query_placeholders_and_field_names($sql) as $queryparam => $formparam) {
                 if (!isset($data[$formparam])) {
                     $errors['params'] = get_string('queryparamschanged', 'report_customsql');
                     break;
                 }
-                $queryparams[$queryparam] = $data[$formparam];
+                $paramvalues[$queryparam] = $data[$formparam];
             }
 
             if (!isset($errors['params'])) {
                 try {
-                    $rs = report_customsql_execute_query($sql, $queryparams, 2);
+                    $rs = report_customsql_execute_query($sql, $paramvalues, 2);
 
                     if (!empty($data['singlerow'])) {
                         // Count rows for Moodle 2 as all Moodle 1.9 useful and more performant

diff --git a/locallib.php b/locallib.php
@@ -60,13 +60,27 @@ function report_customsql_prepare_sql($report, $timenow) {
 /**
  * Extract all the placeholder names from the SQL.
  * @param string $sql The sql.
- * @return array placeholder names
+ * @return array placeholder names including the leading colon.
  */
 function report_customsql_get_query_placeholders($sql) {
     preg_match_all('/(?<!:):[a-z][a-z0-9_]*/', $sql, $matches);
     return $matches[0];
 }
 
+/**
+ * Extract all the placeholder names from the SQL, and work out the corresponding form field names.
+ *
+ * @param string $querysql The sql.
+ * @return string[] placeholder name => form field name.
+ */
+function report_customsql_get_query_placeholders_and_field_names(string $querysql): array {
+    $queryparams = [];
+    foreach (report_customsql_get_query_placeholders($querysql) as $queryparam) {
+        $queryparams[substr($queryparam, 1)] = 'queryparam' . substr($queryparam, 1);
+    }
+    return $queryparams;
+}
+
 /**
  * Return the type of form field to use for a placeholder, based on its name.
  * @param string $name the placeholder name.
@@ -641,7 +655,7 @@ function report_customsql_get_message($report, $csvfilename) {
 }
 
 function report_customsql_email_report($report, $csvfilename = null) {
-    global $CFG, $DB, $OUTPUT;
+    global $DB;
 
     // If there are no recipients return.
     if (!$report->emailto) {
@@ -698,8 +712,9 @@ function report_customsql_get_ready_to_run_daily_reports($timenow) {
 /**
  * Sends a notification message to the reciepients.
  *
- * @param object $recepient, the message recipient.
- * @param object $message, the message objectr.
+ * @param object $recipient the message recipient.
+ * @param object $message the message object.
+ * @return mixed result of {@link message_send()}.
  */
 function report_customsql_send_email_notification($recipient, $message) {
 
@@ -722,7 +737,9 @@ function report_customsql_send_email_notification($recipient, $message) {
 
 /**
  * Check if the report is ready to run.
+ *
  * @param object $report
+ * @param int $timenow
  * @return boolean
  */
 function report_customsql_is_daily_report_ready($report, $timenow) {
@@ -780,8 +797,9 @@ function report_customsql_copy_csv_to_customdir($report, $timenow, $csvfilename
  * Get a report name as plain text, for use in places like cron output and email subject lines.
  *
  * @param object $report report settings from the database.
+ * @return string the usable version of the name.
  */
-function report_customsql_plain_text_report_name($report) {
+function report_customsql_plain_text_report_name($report): string {
     return format_string($report->displayname, true,
-            ['context' => \context_system::instance()]);
+            ['context' => context_system::instance()]);
 }
diff --git a/tests/behat/behat_report_customsql.php b/tests/behat/behat_report_customsql.php
@@ -53,7 +53,6 @@ class behat_report_customsql extends behat_base {
      * @param TableNode $data Supplied data
      */
     public function the_following_custom_sql_report_exists(TableNode $data) {
-        global $DB;
         $report = $data->getRowsHash();
 
         // Report name.
@@ -102,7 +101,7 @@ public function the_following_custom_sql_report_exists(TableNode $data) {
             $report['runable'] = 'manual';
         }
 
-        $DB->insert_record('report_customsql_queries', (object) $report);
+        $this->save_new_report($report);
     }
 
     /**
@@ -122,7 +121,6 @@ public function the_following_custom_sql_report_exists(TableNode $data) {
      * @param PyStringNode $querysql The query SQL
      */
     public function the_custom_sql_report_x_exists(string $reportname, PyStringNode $querysql) {
-        global $DB;
         $report = [
             'displayname' => $reportname,
             'description' => '',
@@ -134,6 +132,19 @@ public function the_custom_sql_report_x_exists(string $reportname, PyStringNode
             'runable' => 'manual',
         ];
 
+        $this->save_new_report($report);
+    }
+
+    protected function save_new_report(array $report) {
+        global $CFG, $DB;
+
+        require_once($CFG->dirroot . '/report/customsql/locallib.php');
+
+        $params = report_customsql_get_query_placeholders_and_field_names($report['querysql']);
+        if ($params) {
+            $report['queryparams'] = serialize($params);
+        }
+
         $DB->insert_record('report_customsql_queries', (object) $report);
     }
 
@@ -190,7 +201,7 @@ public function i_view_the_x_custom_sql_report_with_these_url_parameters(string
             $queryparams[] = $name . '=' . urlencode($rowdata[1]);
         }
 
-        $this->getSession()->visit($this->locate_path('/report/customsql/view.php?id=' .
+        $this->getSession()->visit($this->locate_path('/report/customsql/view.php?' .
                 implode('&', $queryparams)));
     }
 

diff --git a/tests/behat/report_customsql.feature b/tests/behat/report_customsql.feature
@@ -133,3 +133,54 @@ Feature: Ad-hoc database queries report
     And "http://example.com/3" "link" should exist in the "report_customsql_results" "table"
     And I should not see "Link text link url" in the "report_customsql_results" "table"
     And I should see "This report has 1 rows."
+
+  Scenario: Create and run an Ad-hoc database query that has parameters
+    When I log in as "admin"
+    And I navigate to "Reports > Ad-hoc database queries" in site administration
+    And I press "Add a new query"
+    And I set the following fields to these values:
+      | Query name  | Find user                                       |
+      | Query SQL   | SELECT * FROM {user} WHERE username = :username |
+    And I press "Verify the Query SQL text and update the form"
+    And I set the field "username" to "frog"
+    And I press "Save changes"
+    Then I should see "Find user"
+    And I should see "Query parameters"
+    And the field "username" matches value "frog"
+    And I set the field "username" to "admin"
+    And I press "Run query"
+    And I should see "Find user"
+    And I should see "username: admin"
+    And I should see "[email protected]"
+    And I should see "This report has 1 rows."
+
+  Scenario: Link directly to an Ad-hoc database query that has parameters
+    Given the following custom sql report exists:
+      | name        | Find user                                       |
+      | querysql    | SELECT * FROM {user} WHERE username = :username |
+    When I log in as "admin"
+    And I view the "Find user" custom sql report with these URL parameters:
+      | username | frog |
+    Then I should see "This query did not return any data."
+    And I view the "Find user" custom sql report with these URL parameters:
+      | username | admin |
+    And I should see "[email protected]"
+    And I should see "This report has 1 rows."
+
+  Scenario: Link directly to an Ad-hoc database query giving some parameters
+    Given the following custom sql report exists:
+      | name        | Find user                                                                  |
+      | querysql    | SELECT * FROM {user} WHERE firstname = :firstname AND lastname = :lastname |
+    When I log in as "admin"
+    And I view the "Find user" custom sql report with these URL parameters:
+      | firstname | Admin |
+    Then I should see "Find user"
+    And I should see "Query parameters"
+    And the field "firstname" matches value "Admin"
+    And I set the field "lastname" to "User"
+    And I press "Run query"
+    And I should see "Find user"
+    And I should see "firstname: Admin"
+    And I should see "lastname: User"
+    And I should see "[email protected]"
+    And I should see "This report has 1 rows."
diff --git a/view.php b/view.php
@@ -58,29 +58,44 @@
 
     // Allow query parameters to be entered.
     if (!empty($report->queryparams)) {
-
-        $queryparams = array();
-        foreach (report_customsql_get_query_placeholders($report->querysql) as $queryparam) {
-            $queryparams[substr($queryparam, 1)] = 'queryparam'.substr($queryparam, 1);
+        $queryparams = report_customsql_get_query_placeholders_and_field_names($report->querysql);
+
+        // Get any query param values that are given in the URL.
+        $paramvalues = [];
+        foreach ($queryparams as $queryparam => $notused) {
+            $value = optional_param($queryparam, null, PARAM_RAW);
+            if ($value !== null && $value !== '') {
+                $paramvalues[$queryparam] = $value;
+            }
         }
 
         $relativeurl = 'view.php?id=' . $id;
         $mform = new report_customsql_view_form(report_customsql_url($relativeurl), $queryparams);
+        $formdefaults = [];
+        if ($report->queryparams) {
+            foreach (unserialize($report->queryparams) as $queryparam => $defaultvalue) {
+                $formdefaults[$queryparams[$queryparam]] = $defaultvalue;
+            }
+        }
+        foreach ($paramvalues as $queryparam => $value) {
+            $formdefaults[$queryparams[$queryparam]] = $value;
+        }
+        $mform->set_data($formdefaults);
 
         if ($mform->is_cancelled()) {
             redirect(report_customsql_url('index.php'));
         }
 
-        if ($newreport = $mform->get_data()) {
+        if (($newreport = $mform->get_data()) || count($paramvalues) == count($queryparams)) {
 
             // Pick up named parameters into serialised array.
-            if ($queryparams) {
+            if ($newreport) {
                 foreach ($queryparams as $queryparam => $formparam) {
-                    $queryparams[$queryparam] = $newreport->{$formparam};
-                    unset($newreport->{$formparam});
+                    $paramvalues[$queryparam] = $newreport->{$formparam};
                 }
-                $report->queryparams = serialize($queryparams);
             }
+            $report->queryparams = serialize($paramvalues);
+
         } else {
 
             admin_externalpage_setup('report_customsql', '', $urlparams,
@@ -92,13 +107,6 @@
             if (!html_is_blank($report->description)) {
                 echo html_writer::tag('p', format_text($report->description, FORMAT_HTML));
             }
-
-            $report->description = strip_tags($report->description);
-            $queryparams = unserialize($report->queryparams);
-            foreach ($queryparams as $param => $value) {
-                $report->{'queryparam'.$param} = $value;
-            }
-            $mform->set_data($report);
             $mform->display();
 
             echo $OUTPUT->footer();
@@ -135,8 +143,8 @@
     echo html_writer::tag('p', format_text($report->description, FORMAT_HTML));
 }
 
-if (!empty($queryparams)) {
-    foreach ($queryparams as $name => $value) {
+if (!empty($paramvalues)) {
+    foreach ($paramvalues as $name => $value) {
         if (report_customsql_get_element_type($name) == 'date_time_selector') {
             $value = userdate($value, '%F %T');
         }
@@ -158,7 +166,7 @@
 
         if ($report->runable != 'manual' && !$report->singlerow) {
             echo $OUTPUT->heading(get_string('reportfor', 'report_customsql',
-                                         userdate($csvtimestamp, get_string('strftimedate'))), 3);
+                    userdate($csvtimestamp, get_string('strftimedate'))), 3);
         }
 
         $table = new html_table();

diff --git a/view_form.php b/view_form.php
@@ -35,8 +35,6 @@
  */
 class report_customsql_view_form extends moodleform {
     public function definition() {
-        global $CFG;
-
         $mform = $this->_form;
 
         $mform->addElement('header', 'heading', get_string('queryparameters', 'report_customsql'));