From 8e11464fd6c6627e1e55ac37e8989d1ff5468c8f Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 28 Mar 2017 09:58:47 +0200 Subject: [PATCH 1/5] MDL-58412 mod_feedback: Remove access control in get_items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function it is used for printing the list of questions. The feedback preview doesn’t have any access restriction, you can see the list of questions at any time. --- mod/feedback/classes/external.php | 1 - 1 file changed, 1 deletion(-) diff --git a/mod/feedback/classes/external.php b/mod/feedback/classes/external.php index 17ed2513525..785591ff13e 100644 --- a/mod/feedback/classes/external.php +++ b/mod/feedback/classes/external.php @@ -422,7 +422,6 @@ class mod_feedback_external extends external_api { $warnings = array(); list($feedback, $course, $cm, $context) = self::validate_feedback($params['feedbackid']); - self::validate_feedback_access($feedback, $course, $cm, $context); $feedbackstructure = new mod_feedback_structure($feedback, $cm, $course->id); $returneditems = array(); From 1b0b4ab25f5cb1685f4d14980df2927c72662ecd Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 28 Mar 2017 10:01:25 +0200 Subject: [PATCH 2/5] MDL-58412 mod_feedback: Add default value for responses In some cases we will be processing pages without responses, like a page introduction with a label. --- mod/feedback/classes/external.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mod/feedback/classes/external.php b/mod/feedback/classes/external.php index 785591ff13e..5bf5a4320ec 100644 --- a/mod/feedback/classes/external.php +++ b/mod/feedback/classes/external.php @@ -610,7 +610,7 @@ class mod_feedback_external extends external_api { 'name' => new external_value(PARAM_NOTAGS, 'The response name (usually type[index]_id).'), 'value' => new external_value(PARAM_RAW, 'The response value.'), ) - ), 'The data to be processed.' + ), 'The data to be processed.', VALUE_DEFAULT, array() ), 'goprevious' => new external_value(PARAM_BOOL, 'Whether we want to jump to previous page.', VALUE_DEFAULT, false), ) @@ -627,7 +627,7 @@ class mod_feedback_external extends external_api { * @return array of warnings and launch information * @since Moodle 3.3 */ - public static function process_page($feedbackid, $page, $responses, $goprevious = false) { + public static function process_page($feedbackid, $page, $responses = [], $goprevious = false) { global $USER, $SESSION; $params = array('feedbackid' => $feedbackid, 'page' => $page, 'responses' => $responses, 'goprevious' => $goprevious); From 592306c60b8a2c361fcb37c5d2247eadb6817f01 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 28 Mar 2017 10:05:56 +0200 Subject: [PATCH 3/5] MDL-58412 mod_feedback: Always set gonextpage when moving forward In some cases the last page will be omitted in a feedback (for example when using dependent questions). Because of that the save process will be launched in a page that is not the last. --- mod/feedback/classes/external.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mod/feedback/classes/external.php b/mod/feedback/classes/external.php index 5bf5a4320ec..7dbd20b67e6 100644 --- a/mod/feedback/classes/external.php +++ b/mod/feedback/classes/external.php @@ -649,11 +649,12 @@ class mod_feedback_external extends external_api { $_POST['courseid'] = $course->id; $_POST['gopage'] = $params['page']; $_POST['_qf__mod_feedback_complete_form'] = 1; + + // Determine where to go, backwards or forward. if (!$params['goprevious']) { + $_POST['gonextpage'] = 1; // Even if we are saving values we need this set. if ($feedbackcompletion->get_next_page($params['page'], false) === null) { $_POST['savevalues'] = 1; // If there is no next page, it means we are finishing the feedback. - } else { - $_POST['gonextpage'] = 1; // If we are not going to previous page or finishing we are going forward. } } From 2b2a0319a0d0a0070b37811b613a3561f02926c5 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Tue, 28 Mar 2017 10:13:26 +0200 Subject: [PATCH 4/5] MDL-58412 mod_feedback: Handle array parameters We should handle array parameters for multi choice (multi select) . --- mod/feedback/classes/external.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mod/feedback/classes/external.php b/mod/feedback/classes/external.php index 7dbd20b67e6..17c98b6bc42 100644 --- a/mod/feedback/classes/external.php +++ b/mod/feedback/classes/external.php @@ -642,7 +642,12 @@ class mod_feedback_external extends external_api { // Create the $_POST object required by the feedback question engine. $_POST = array(); foreach ($responses as $response) { - $_POST[$response['name']] = $response['value']; + // First check if we are handling array parameters. + if (preg_match('/(.+)\[(.+)\]$/', $response['name'], $matches)) { + $_POST[$matches[1]][$matches[2]] = $response['value']; + } else { + $_POST[$response['name']] = $response['value']; + } } // Force fields. $_POST['id'] = $cm->id; From cadae8dce6633dec19afb4bbcc9bd93e5055c40b Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 19 Apr 2017 11:37:15 +0200 Subject: [PATCH 5/5] MDL-58412 mod_feedback: Use constant for anonymous feedback We were using hardcoded values instead the correct constant FEEDBACK_ANONYMOUS_NO --- mod/feedback/tests/external_test.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/mod/feedback/tests/external_test.php b/mod/feedback/tests/external_test.php index b1e9fddb818..92bc8a36afc 100644 --- a/mod/feedback/tests/external_test.php +++ b/mod/feedback/tests/external_test.php @@ -29,6 +29,7 @@ defined('MOODLE_INTERNAL') || die(); global $CFG; require_once($CFG->dirroot . '/webservice/tests/helpers.php'); +require_once($CFG->dirroot . '/mod/feedback/lib.php'); use mod_feedback\external\feedback_summary_exporter; @@ -315,7 +316,7 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { global $DB; // Force non anonymous. - $DB->set_field('feedback', 'anonymous', 0, array('id' => $this->feedback->id)); + $DB->set_field('feedback', 'anonymous', FEEDBACK_ANONYMOUS_NO, array('id' => $this->feedback->id)); // Add a completed_tmp record. $record = [ 'feedback' => $this->feedback->id, @@ -323,7 +324,7 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { 'guestid' => '', 'timemodified' => time() - DAYSECS, 'random_response' => 0, - 'anonymous_response' => 2, + 'anonymous_response' => FEEDBACK_ANONYMOUS_NO, 'courseid' => $this->course->id, ]; $record['id'] = $DB->insert_record('feedback_completedtmp', (object) $record); @@ -386,7 +387,7 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { // Now, try a feedback that we attempted. // Force non anonymous. - $DB->set_field('feedback', 'anonymous', 0, array('id' => $this->feedback->id)); + $DB->set_field('feedback', 'anonymous', FEEDBACK_ANONYMOUS_NO, array('id' => $this->feedback->id)); // Add a completed_tmp record. $record = [ 'feedback' => $this->feedback->id, @@ -394,7 +395,7 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { 'guestid' => '', 'timemodified' => time() - DAYSECS, 'random_response' => 0, - 'anonymous_response' => 2, + 'anonymous_response' => FEEDBACK_ANONYMOUS_NO, 'courseid' => $this->course->id, ]; $record['id'] = $DB->insert_record('feedback_completedtmp', (object) $record);