diff --git a/mod/feedback/classes/external.php b/mod/feedback/classes/external.php index 9642de3afc2..72dd776bfa2 100644 --- a/mod/feedback/classes/external.php +++ b/mod/feedback/classes/external.php @@ -452,19 +452,48 @@ class mod_feedback_external extends external_api { $params = array('feedbackid' => $feedbackid, 'courseid' => $courseid); $params = self::validate_parameters(self::get_items_parameters(), $params); $warnings = array(); + $returneditems = array(); list($feedback, $course, $cm, $context, $completioncourse) = self::validate_feedback($params['feedbackid'], $params['courseid']); - $feedbackstructure = new mod_feedback_structure($feedback, $cm, $completioncourse->id); - $returneditems = array(); - if ($items = $feedbackstructure->get_items()) { - foreach ($items as $item) { - $itemnumber = empty($item->itemnr) ? null : $item->itemnr; - unset($item->itemnr); // Added by the function, not part of the record. - $exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber)); - $returneditems[] = $exporter->export($PAGE->get_renderer('core')); + $userhasaccess = true; + try { + // Check the user has access to the feedback. + self::validate_feedback_access($feedback, $completioncourse, $cm, $context, true); + } catch (moodle_exception $e) { + $userhasaccess = false; + $warnings[] = [ + 'item' => $feedback->id, + 'warningcode' => clean_param($e->errorcode, PARAM_ALPHANUM), + 'message' => $e->getMessage(), + ]; + } + + // For consistency with the web behaviour, the items should be returned only when the user can edit or view reports (to + // include non-editing teachers too). + $capabilities = [ + 'mod/feedback:edititems', + 'mod/feedback:viewreports', + ]; + if ($userhasaccess || has_any_capability($capabilities, $context)) { + // Remove previous warnings because, although the user might not have access, they have the proper capability. + $warnings = []; + $feedbackstructure = new mod_feedback_structure($feedback, $cm, $completioncourse->id); + if ($items = $feedbackstructure->get_items()) { + foreach ($items as $item) { + $itemnumber = empty($item->itemnr) ? null : $item->itemnr; + unset($item->itemnr); // Added by the function, not part of the record. + $exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber)); + $returneditems[] = $exporter->export($PAGE->get_renderer('core')); + } } + } else if ($userhasaccess) { + $warnings[] = [ + 'item' => $feedback->id, + 'warningcode' => 'nopermission', + 'message' => 'nopermission', + ]; } $result = array( @@ -586,24 +615,59 @@ class mod_feedback_external extends external_api { $params = array('feedbackid' => $feedbackid, 'page' => $page, 'courseid' => $courseid); $params = self::validate_parameters(self::get_page_items_parameters(), $params); $warnings = array(); + $returneditems = array(); + $hasprevpage = false; + $hasnextpage = false; list($feedback, $course, $cm, $context, $completioncourse) = self::validate_feedback($params['feedbackid'], $params['courseid']); - $feedbackcompletion = new mod_feedback_completion($feedback, $cm, $completioncourse->id); + $userhasaccess = true; + $feedbackcompletion = null; + try { + // Check the user has access to the feedback. + $feedbackcompletion = self::validate_feedback_access($feedback, $completioncourse, $cm, $context, true); + } catch (moodle_exception $e) { + $userhasaccess = false; + $warnings[] = [ + 'item' => $feedback->id, + 'warningcode' => str_replace('_', '', $e->errorcode), + 'message' => $e->getMessage(), + ]; + } - $page = $params['page']; - $pages = $feedbackcompletion->get_pages(); - $pageitems = $pages[$page]; - $hasnextpage = $page < count($pages) - 1; // Until we complete this page we can not trust get_next_page(). - $hasprevpage = $page && ($feedbackcompletion->get_previous_page($page, false) !== null); + // For consistency with the web behaviour, the items should be returned only when the user can edit or view reports (to + // include non-editing teachers too). + $capabilities = [ + 'mod/feedback:edititems', + 'mod/feedback:viewreports', + ]; + if ($userhasaccess || has_any_capability($capabilities, $context)) { + // Remove previous warnings because, although the user might not have access, they have the proper capability. + $warnings = []; - $returneditems = array(); - foreach ($pageitems as $item) { - $itemnumber = empty($item->itemnr) ? null : $item->itemnr; - unset($item->itemnr); // Added by the function, not part of the record. - $exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber)); - $returneditems[] = $exporter->export($PAGE->get_renderer('core')); + if ($feedbackcompletion == null) { + $feedbackcompletion = new mod_feedback_completion($feedback, $cm, $completioncourse->id); + } + + $page = $params['page']; + $pages = $feedbackcompletion->get_pages(); + $pageitems = $pages[$page]; + $hasnextpage = $page < count($pages) - 1; // Until we complete this page we can not trust get_next_page(). + $hasprevpage = $page && ($feedbackcompletion->get_previous_page($page, false) !== null); + + foreach ($pageitems as $item) { + $itemnumber = empty($item->itemnr) ? null : $item->itemnr; + unset($item->itemnr); // Added by the function, not part of the record. + $exporter = new feedback_item_exporter($item, array('context' => $context, 'itemnumber' => $itemnumber)); + $returneditems[] = $exporter->export($PAGE->get_renderer('core')); + } + } else if ($userhasaccess) { + $warnings[] = [ + 'item' => $feedback->id, + 'warningcode' => 'nopermission', + 'message' => get_string('nopermission', 'mod_feedback'), + ]; } $result = array( diff --git a/mod/feedback/tests/external/external_test.php b/mod/feedback/tests/external/external_test.php index df8e12b5dbd..6c1b1be1f69 100644 --- a/mod/feedback/tests/external/external_test.php +++ b/mod/feedback/tests/external/external_test.php @@ -29,6 +29,8 @@ namespace mod_feedback\external; use externallib_advanced_testcase; use feedback_item_multichoice; use mod_feedback_external; +use moodle_exception; +use required_capability_exception; defined('MOODLE_INTERNAL') || die(); @@ -45,6 +47,7 @@ require_once($CFG->dirroot . '/mod/feedback/lib.php'); * @copyright 2017 Juan Leyva * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @since Moodle 3.3 + * @covers \mod_feedback_external */ class external_test extends externallib_advanced_testcase { @@ -374,6 +377,130 @@ class external_test extends externallib_advanced_testcase { } } + /** + * Test get_items, to confirm validation is done too. + * + * @dataProvider items_provider + * @param string $role Whether the current user should be a student or a teacher. + * @param array $info Settings to create the feedback. + * @param string|null $warning The warning message to display or null if warnings result is empty. + */ + public function test_get_items_validation(string $role, array $info, ?string $warning): void { + global $DB; + + // Test user with full capabilities. + if ($role === 'teacher') { + $this->setUser($this->teacher); + } else { + $this->setUser($this->student); + } + + // Create the feedback. + $data = ['course' => $this->course->id]; + if (array_key_exists('closed', $info) && $info['closed']) { + $data['timeopen'] = time() + DAYSECS; + } + $feedback = $this->getDataGenerator()->create_module('feedback', $data); + + $empty = true; + if (!array_key_exists('empty', $info) || !$info['empty']) { + $empty = false; + /** @var \mod_feedback_generator $feedbackgenerator */ + $feedbackgenerator = $this->getDataGenerator()->get_plugin_generator('mod_feedback'); + // Add,at least, one item to the feedback. + $feedbackgenerator->create_item_label($feedback); + } + + if (array_key_exists('complete', $info) && !$info['complete']) { + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $coursecontext = \context_course::instance($this->course->id); + assign_capability('mod/feedback:complete', CAP_PROHIBIT, $studentrole->id, $coursecontext->id); + // Empty all the caches that may be affected by this change. + accesslib_clear_all_caches_for_unit_testing(); + \course_modinfo::clear_instance_cache(); + } + + $result = mod_feedback_external::get_items($feedback->id); + $result = \external_api::clean_returnvalue(mod_feedback_external::get_items_returns(), $result); + if ($warning) { + $this->assertEmpty($result['items']); + $this->assertCount(1, $result['warnings']); + $resultwarning = reset($result['warnings']); + if ($warning == 'required_capability_exception') { + $this->assertStringContainsString('error/nopermission', $resultwarning['message']); + } else { + $this->assertEquals($warning, $resultwarning['message']); + } + } else { + if ($empty) { + $this->assertEmpty($result['items']); + } else { + $this->assertCount(1, $result['items']); + } + $this->assertEmpty($result['warnings']); + } + } + + /** + * Data provider for test_get_items_validation() and test_get_page_items_validation(). + * + * @return array + */ + public function items_provider(): array { + return [ + 'Valid feedback (as student)' => [ + 'role' => 'student', + 'info' => [], + 'warning' => null, + ], + 'Closed feedback (as student)' => [ + 'role' => 'student', + 'info' => ['closed' => true], + 'warning' => get_string('feedback_is_not_open', 'feedback'), + ], + 'Empty feedback (as student)' => [ + 'role' => 'student', + 'info' => ['empty' => true], + 'warning' => get_string('no_items_available_yet', 'feedback'), + ], + 'Closed feedback (as student)' => [ + 'role' => 'student', + 'info' => ['closed' => true], + 'warning' => get_string('feedback_is_not_open', 'feedback'), + ], + 'Cannot complete feedback (as student)' => [ + 'role' => 'student', + 'info' => ['complete' => false], + 'warning' => 'required_capability_exception', + ], + 'Valid feedback (as teacher)' => [ + 'role' => 'teacher', + 'info' => [], + 'warning' => null, + ], + 'Closed feedback (as teacher)' => [ + 'role' => 'teacher', + 'info' => ['closed' => true], + 'warning' => null, + ], + 'Empty feedback (as teacher)' => [ + 'role' => 'teacher', + 'info' => ['empty' => true], + 'warning' => null, + ], + 'Closed feedback (as teacher)' => [ + 'role' => 'teacher', + 'info' => ['closed' => true], + 'warning' => null, + ], + 'Cannot complete feedback (as teacher)' => [ + 'role' => 'teacher', + 'info' => ['complete' => false], + 'warning' => null, + ], + ]; + } + /** * Test launch_feedback. */ @@ -454,6 +581,70 @@ class external_test extends externallib_advanced_testcase { $this->assertTrue($result['hasprevpage']); } + /** + * Test get_page_items, to confirm validation is done too. + * + * @dataProvider items_provider + * @param string $role Whether the current user should be a student or a teacher. + * @param array $info Settings to create the feedback. + * @param string|null $warning The warning message to display or null if warnings result is empty. + */ + public function test_get_page_items_validation(string $role, array $info, ?string $warning): void { + global $DB; + + // Test user with full capabilities. + if ($role === 'teacher') { + $this->setUser($this->teacher); + } else { + $this->setUser($this->student); + } + + // Create the feedback. + $data = ['course' => $this->course->id]; + if (array_key_exists('closed', $info) && $info['closed']) { + $data['timeopen'] = time() + DAYSECS; + } + $feedback = $this->getDataGenerator()->create_module('feedback', $data); + + $empty = true; + if (!array_key_exists('empty', $info) || !$info['empty']) { + $empty = false; + /** @var \mod_feedback_generator $feedbackgenerator */ + $feedbackgenerator = $this->getDataGenerator()->get_plugin_generator('mod_feedback'); + // Add,at least, one item to the feedback. + $feedbackgenerator->create_item_label($feedback); + } + + if (array_key_exists('complete', $info) && !$info['complete']) { + $studentrole = $DB->get_record('role', array('shortname' => 'student')); + $coursecontext = \context_course::instance($this->course->id); + assign_capability('mod/feedback:complete', CAP_PROHIBIT, $studentrole->id, $coursecontext->id); + // Empty all the caches that may be affected by this change. + accesslib_clear_all_caches_for_unit_testing(); + \course_modinfo::clear_instance_cache(); + } + + $result = mod_feedback_external::get_page_items($feedback->id, 0); + $result = \external_api::clean_returnvalue(mod_feedback_external::get_items_returns(), $result); + if ($warning) { + $this->assertEmpty($result['items']); + $this->assertCount(1, $result['warnings']); + $resultwarning = reset($result['warnings']); + if ($warning == 'required_capability_exception') { + $this->assertStringContainsString('error/nopermission', $resultwarning['message']); + } else { + $this->assertEquals($warning, $resultwarning['message']); + } + } else { + if ($empty) { + $this->assertEmpty($result['items']); + } else { + $this->assertCount(1, $result['items']); + } + $this->assertEmpty($result['warnings']); + } + } + /** * Test process_page. */ diff --git a/mod/feedback/upgrade.txt b/mod/feedback/upgrade.txt index b6eb5f8effa..0d96c18bf2f 100644 --- a/mod/feedback/upgrade.txt +++ b/mod/feedback/upgrade.txt @@ -1,3 +1,7 @@ +=== 4.1.2 === +* The external methods get_items() and get_page_items() now only return the items when the user can access the feedback; if the +user can't see them, a warning message will be returned, with the reasons. + === 4.0 === * The following files and classes within them have been deprecated in favour of dynamic forms: * edit_form.php