From 5fdb6183e55fa844c5a82a638329147e41045a7e Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 15 Nov 2018 12:03:46 +0100 Subject: [PATCH] MDL-63627 enrol: Return progress field correctly - Check permissions and - Return the progress for the given user --- enrol/externallib.php | 11 ++++-- enrol/tests/externallib_test.php | 63 ++++++++++++++++++++++++++------ 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/enrol/externallib.php b/enrol/externallib.php index 55b074c4164..7c1b131ea36 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -299,6 +299,8 @@ class core_enrol_external extends external_api { // Do basic automatic PARAM checks on incoming data, using params description // If any problems are found then exceptions are thrown with helpful error messages $params = self::validate_parameters(self::get_users_courses_parameters(), array('userid'=>$userid)); + $userid = $params['userid']; + $sameuser = $USER->id == $userid; $courses = enrol_get_users_courses($params['userid'], true, 'id, shortname, fullname, idnumber, visible, summary, summaryformat, format, showgrades, lang, enablecompletion, category, startdate, enddate'); @@ -313,7 +315,7 @@ class core_enrol_external extends external_api { continue; } - if ($userid != $USER->id and !course_can_view_participants($context)) { + if (!$sameuser and !course_can_view_participants($context)) { // we need capability to view participants continue; } @@ -328,8 +330,11 @@ class core_enrol_external extends external_api { $course->shortname = external_format_string($course->shortname, $context->id); $progress = null; - if ($course->enablecompletion) { - $progress = \core_completion\progress::get_course_progress_percentage($course); + // Return only private information if the user should be able to see it. + if ($course->enablecompletion && + ($sameuser || completion_can_view_data($userid, $course))) { + + $progress = \core_completion\progress::get_course_progress_percentage($course, $userid); } $result[] = array( diff --git a/enrol/tests/externallib_test.php b/enrol/tests/externallib_test.php index 22c3c5c5e16..390219b110e 100644 --- a/enrol/tests/externallib_test.php +++ b/enrol/tests/externallib_test.php @@ -359,9 +359,11 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { * Test get_users_courses */ public function test_get_users_courses() { - global $USER; + global $CFG, $DB; + require_once($CFG->dirroot . '/completion/criteria/completion_criteria_self.php'); $this->resetAfterTest(true); + $CFG->enablecompletion = 1; $timenow = time(); $coursedata1 = array( @@ -383,21 +385,31 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { $course1 = self::getDataGenerator()->create_course($coursedata1); $course2 = self::getDataGenerator()->create_course($coursedata2); $courses = array($course1, $course2); + $contexts = array ($course1->id => context_course::instance($course1->id), + $course2->id => context_course::instance($course2->id)); - // Enrol $USER in the courses. - // We use the manual plugin. - $roleid = null; - $contexts = array(); - foreach ($courses as $course) { - $contexts[$course->id] = context_course::instance($course->id); - $roleid = $this->assignUserCapability('moodle/course:viewparticipants', - $contexts[$course->id]->id, $roleid); + $student = $this->getDataGenerator()->create_user(); + $otherstudent = $this->getDataGenerator()->create_user(); + $studentroleid = $DB->get_field('role', 'id', array('shortname' => 'student')); + $this->getDataGenerator()->enrol_user($student->id, $course1->id, $studentroleid); + $this->getDataGenerator()->enrol_user($otherstudent->id, $course1->id, $studentroleid); + $this->getDataGenerator()->enrol_user($student->id, $course2->id, $studentroleid); - $this->getDataGenerator()->enrol_user($USER->id, $course->id, $roleid, 'manual'); - } + // Force completion, setting at least one criteria. + $criteriadata = new stdClass(); + $criteriadata->id = $course1->id; + // Self completion. + $criteriadata->criteria_self = 1; + $criterion = new completion_criteria_self(); + $criterion->update_config($criteriadata); + + $ccompletion = new completion_completion(array('course' => $course1->id, 'userid' => $student->id)); + $ccompletion->mark_complete(); + + $this->setUser($student); // Call the external function. - $enrolledincourses = core_enrol_external::get_users_courses($USER->id); + $enrolledincourses = core_enrol_external::get_users_courses($student->id); // We need to execute the return values cleaning process to simulate the web service server. $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); @@ -418,11 +430,38 @@ class core_enrol_externallib_testcase extends externallib_advanced_testcase { foreach ($coursedata1 as $fieldname => $value) { $this->assertEquals($courseenrol[$fieldname], $course1->$fieldname); } + // Check progress. + $this->assertEquals(100.0, $courseenrol['progress']); } else { // Check language pack. Should be empty since an incorrect one was used when creating the course. $this->assertEmpty($courseenrol['lang']); + // Check progress. + $this->assertEquals(0, $courseenrol['progress']); } } + + // Now check that admin users can see all the info. + $this->setAdminUser(); + + $enrolledincourses = core_enrol_external::get_users_courses($student->id); + $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); + $this->assertEquals(2, count($enrolledincourses)); + foreach ($enrolledincourses as $courseenrol) { + if ($courseenrol['id'] == $course1->id) { + $this->assertEquals(100.0, $courseenrol['progress']); + } else { + $this->assertEquals(0, $courseenrol['progress']); + } + } + + // Check other users can't see private info. + $this->setUser($otherstudent); + + $enrolledincourses = core_enrol_external::get_users_courses($student->id); + $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); + $this->assertEquals(1, count($enrolledincourses)); // I see only the course I share. + + $this->assertEquals(null, $enrolledincourses[0]['progress']); // I can't see this, private. } /**