From 8352aa50f34be352148df668b4e3361af0e765c1 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Wed, 22 Apr 2015 10:08:53 +0200 Subject: [PATCH] MDL-49937 data: Handle admins/managers in get_databases_by_courses --- mod/data/classes/external.php | 42 ++++++++++++----------------- mod/data/tests/externallib_test.php | 25 ++++++++++++----- 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/mod/data/classes/external.php b/mod/data/classes/external.php index 50c608094d8..b0634173475 100644 --- a/mod/data/classes/external.php +++ b/mod/data/classes/external.php @@ -70,23 +70,11 @@ class mod_data_external extends external_api { $params = self::validate_parameters(self::get_databases_by_courses_parameters(), array('courseids' => $courseids)); $warnings = array(); - $courses = enrol_get_my_courses(); - // Used to test for ids that have been requested but can't be returned. - if (count($params['courseids']) > 0) { - $courseids = array(); - foreach ($params['courseids'] as $courseid) { - if (!in_array($courseid, array_keys($courses))) { - $warnings[] = array( - 'item' => 'course', - 'itemid' => $courseid, - 'warningcode' => '2', - 'message' => 'User is not enrolled or does not have requested capability' - ); - } else { - $courseids[] = $courseid; - } - } + if (!empty($params['courseids'])) { + $courses = array(); + $courseids = $params['courseids']; } else { + $courses = enrol_get_my_courses(); $courseids = array_keys($courses); } @@ -100,11 +88,15 @@ class mod_data_external extends external_api { // Go through the courseids. foreach ($courseids as $cid) { - $context = context_course::instance($cid); // Check the user can function in this context. try { + $context = context_course::instance($cid); self::validate_context($context); + // Check if this course was already loaded (by enrol_get_my_courses). + if (!isset($courses[$cid])) { + $courses[$cid] = get_course($cid); + } $dbcourses[$cid] = $courses[$cid]; } catch (Exception $e) { @@ -112,7 +104,7 @@ class mod_data_external extends external_api { 'item' => 'course', 'itemid' => $cid, 'warningcode' => '1', - 'message' => 'No access rights in course context '.$e->getMessage().$e->getTraceAsString() + 'message' => 'No access rights in course context '.$e->getMessage() ); } } @@ -200,13 +192,13 @@ class mod_data_external extends external_api { 'name' => new external_value(PARAM_TEXT, 'Database name'), 'intro' => new external_value(PARAM_RAW, 'The Database intro'), 'introformat' => new external_format_value('intro'), - 'comments' => new external_value(PARAM_BOOL, 'comments enabled'), - 'timeavailablefrom' => new external_value(PARAM_INT, 'timeavailablefrom field'), - 'timeavailableto' => new external_value(PARAM_INT, 'timeavailableto field'), - 'timeviewfrom' => new external_value(PARAM_INT, 'timeviewfrom field'), - 'timeviewto' => new external_value(PARAM_INT, 'timeviewto field'), - 'requiredentries' => new external_value(PARAM_INT, 'requiredentries field'), - 'requiredentriestoview' => new external_value(PARAM_INT, 'requiredentriestoview field'), + 'comments' => new external_value(PARAM_BOOL, 'comments enabled', VALUE_OPTIONAL), + 'timeavailablefrom' => new external_value(PARAM_INT, 'timeavailablefrom field', VALUE_OPTIONAL), + 'timeavailableto' => new external_value(PARAM_INT, 'timeavailableto field', VALUE_OPTIONAL), + 'timeviewfrom' => new external_value(PARAM_INT, 'timeviewfrom field', VALUE_OPTIONAL), + 'timeviewto' => new external_value(PARAM_INT, 'timeviewto field', VALUE_OPTIONAL), + 'requiredentries' => new external_value(PARAM_INT, 'requiredentries field', VALUE_OPTIONAL), + 'requiredentriestoview' => new external_value(PARAM_INT, 'requiredentriestoview field', VALUE_OPTIONAL), 'maxentries' => new external_value(PARAM_INT, 'maxentries field', VALUE_OPTIONAL), 'rssarticles' => new external_value(PARAM_INT, 'rssarticles field', VALUE_OPTIONAL), 'singletemplate' => new external_value(PARAM_RAW, 'singletemplate field', VALUE_OPTIONAL), diff --git a/mod/data/tests/externallib_test.php b/mod/data/tests/externallib_test.php index 49d8fccd5d9..1cc34c209f6 100644 --- a/mod/data/tests/externallib_test.php +++ b/mod/data/tests/externallib_test.php @@ -106,6 +106,8 @@ class mod_data_external_testcase extends externallib_advanced_testcase { $expected1[$field] = $database1->{$field}; $expected2[$field] = $database2->{$field}; } + $expected1['comments'] = (bool) $expected1['comments']; + $expected2['comments'] = (bool) $expected2['comments']; $expecteddatabases = array(); $expecteddatabases[] = $expected2; @@ -113,12 +115,12 @@ class mod_data_external_testcase extends externallib_advanced_testcase { // Call the external function passing course ids. $result = mod_data_external::get_databases_by_courses(array($course2->id, $course1->id)); - external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); + $result = external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); $this->assertEquals($expecteddatabases, $result['databases']); // Call the external function without passing course id. $result = mod_data_external::get_databases_by_courses(); - external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); + $result = external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); $this->assertEquals($expecteddatabases, $result['databases']); // Unenrol user from second course and alter expected databases. @@ -127,13 +129,13 @@ class mod_data_external_testcase extends externallib_advanced_testcase { // Call the external function without passing course id. $result = mod_data_external::get_databases_by_courses(); - external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); + $result = external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); $this->assertEquals($expecteddatabases, $result['databases']); // Call for the second course we unenrolled the user from, expected warning. $result = mod_data_external::get_databases_by_courses(array($course2->id)); $this->assertCount(1, $result['warnings']); - $this->assertEquals('2', $result['warnings'][0]['warningcode']); + $this->assertEquals('1', $result['warnings'][0]['warningcode']); $this->assertEquals($course2->id, $result['warnings'][0]['itemid']); // Now, try as a teacher for getting all the additional fields. @@ -145,10 +147,21 @@ class mod_data_external_testcase extends externallib_advanced_testcase { 'assesstimefinish', 'defaultsort', 'defaultsortdir', 'editany', 'notification'); foreach ($additionalfields as $field) { - $expecteddatabases[0][$field] = $database1->{$field}; + if ($field == 'approval' or $field == 'editany') { + $expecteddatabases[0][$field] = (bool) $database1->{$field}; + } else { + $expecteddatabases[0][$field] = $database1->{$field}; + } } $result = mod_data_external::get_databases_by_courses(); - external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); + $result = external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); + $this->assertEquals($expecteddatabases, $result['databases']); + + // Admin should get all the information. + self::setAdminUser(); + + $result = mod_data_external::get_databases_by_courses(array($course1->id)); + $result = external_api::clean_returnvalue(mod_data_external::get_databases_by_courses_returns(), $result); $this->assertEquals($expecteddatabases, $result['databases']); } }