mirror of
https://github.com/moodle/moodle.git
synced 2025-04-05 08:23:01 +02:00
Merge branch 'MDL-77230-401' of https://github.com/sarjona/moodle into MOODLE_401_STABLE
This commit is contained in:
commit
506604c911
@ -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(
|
||||
|
191
mod/feedback/tests/external/external_test.php
vendored
191
mod/feedback/tests/external/external_test.php
vendored
@ -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 <juan@moodle.com>
|
||||
* @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.
|
||||
*/
|
||||
|
@ -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
|
||||
|
Loading…
x
Reference in New Issue
Block a user