From 3ffd13827eb68fdce514cfdf0345512fafba8ed2 Mon Sep 17 00:00:00 2001 From: Andres Melo Date: Wed, 17 May 2017 16:07:29 -0500 Subject: [PATCH] MDL-46322 mod_assign: Only list enrolled graders as potential markers --- mod/assign/gradingtable.php | 3 +- mod/assign/locallib.php | 15 ++- mod/assign/tests/markerallocation_test.php | 149 +++++++++++++++++++++ 3 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 mod/assign/tests/markerallocation_test.php diff --git a/mod/assign/gradingtable.php b/mod/assign/gradingtable.php index 33caa81004e..2eba8355061 100644 --- a/mod/assign/gradingtable.php +++ b/mod/assign/gradingtable.php @@ -647,7 +647,8 @@ class assign_grading_table extends table_sql implements renderable { static $markerlist = array(); if ($markers === null) { list($sort, $params) = users_order_by_sql(); - $markers = get_users_by_capability($this->assignment->get_context(), 'mod/assign:grade', '', $sort); + // Only enrolled users could be assigned as potential markers. + $markers = get_enrolled_users($this->assignment->get_context(), 'mod/assign:grade', 0, 'u.*', $sort); $markerlist[0] = get_string('choosemarker', 'assign'); $viewfullnames = has_capability('moodle/site:viewfullnames', $this->assignment->get_context()); foreach ($markers as $marker) { diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index f32def9a70c..0982deec3e6 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -4018,7 +4018,8 @@ class assign { $markingallocationoptions = array(); if ($markingallocation) { list($sort, $params) = users_order_by_sql(); - $markers = get_users_by_capability($this->context, 'mod/assign:grade', '', $sort); + // Only enrolled users could be assigned as potential markers. + $markers = get_enrolled_users($this->context, 'mod/assign:grade', 0, 'u.*', $sort); $markingallocationoptions[''] = get_string('filternone', 'assign'); $markingallocationoptions[ASSIGN_MARKER_FILTER_NO_MARKER] = get_string('markerfilternomarker', 'assign'); $viewfullnames = has_capability('moodle/site:viewfullnames', $this->context); @@ -4645,7 +4646,8 @@ class assign { ); list($sort, $params) = users_order_by_sql(); - $markers = get_users_by_capability($this->get_context(), 'mod/assign:grade', '', $sort); + // Only enrolled users could be assigned as potential markers. + $markers = get_enrolled_users($this->get_context(), 'mod/assign:grade', 0, 'u.*', $sort); $markerlist = array(); foreach ($markers as $marker) { $markerlist[$marker->id] = fullname($marker); @@ -6531,7 +6533,8 @@ class assign { $markingallocationoptions[''] = get_string('filternone', 'assign'); $markingallocationoptions[ASSIGN_MARKER_FILTER_NO_MARKER] = get_string('markerfilternomarker', 'assign'); list($sort, $params) = users_order_by_sql(); - $markers = get_users_by_capability($this->context, 'mod/assign:grade', '', $sort); + // Only enrolled users could be assigned as potential markers. + $markers = get_enrolled_users($this->context, 'mod/assign:grade', 0, 'u.*', $sort); foreach ($markers as $marker) { $markingallocationoptions[$marker->id] = fullname($marker); } @@ -7166,7 +7169,8 @@ class assign { has_capability('mod/assign:manageallocations', $this->context)) { list($sort, $params) = users_order_by_sql(); - $markers = get_users_by_capability($this->context, 'mod/assign:grade', '', $sort); + // Only enrolled users could be assigned as potential markers. + $markers = get_enrolled_users($this->context, 'mod/assign:grade', 0, 'u.*', $sort); $markerlist = array('' => get_string('choosemarker', 'assign')); $viewfullnames = has_capability('moodle/site:viewfullnames', $this->context); foreach ($markers as $marker) { @@ -7586,7 +7590,8 @@ class assign { ); list($sort, $params) = users_order_by_sql(); - $markers = get_users_by_capability($this->get_context(), 'mod/assign:grade', '', $sort); + // Only enrolled users could be assigned as potential markers. + $markers = get_enrolled_users($this->get_context(), 'mod/assign:grade', 0, 'u.*', $sort); $markerlist = array(); foreach ($markers as $marker) { $markerlist[$marker->id] = fullname($marker); diff --git a/mod/assign/tests/markerallocation_test.php b/mod/assign/tests/markerallocation_test.php new file mode 100644 index 00000000000..5bf9420a313 --- /dev/null +++ b/mod/assign/tests/markerallocation_test.php @@ -0,0 +1,149 @@ +. + +/** + * Unit tests for (some of) mod/assign/markerallocaion_test.php. + * + * @package mod_assign + * @category test + * @copyright 2017 Andrés Melo + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/lib/accesslib.php'); +require_once($CFG->dirroot . '/course/lib.php'); + +/** + * This class tests some of marker allocation functionality. + * + * @package mod_assign + * @copyright 2017 Andrés Melo + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +class mod_assign_markerallocation_testcase extends advanced_testcase { + + /** + * Create all the needed elements to test the difference between both functions. + */ + + public function test_markerusers() { + $this->resetAfterTest(); + global $DB; + // Create a course, by default it is created with 5 sections. + $this->course = $this->getDataGenerator()->create_course(); + + // Setting assing module, markingworkflow and markingallocation set to 1 to enable marker allocation. + $record = new stdClass(); + $record->course = $this->course; + + $modulesettings = array( + 'alwaysshowdescription' => 1, + 'submissiondrafts' => 1, + 'requiresubmissionstatement' => 0, + 'sendnotifications' => 0, + 'sendstudentnotifications' => 1, + 'sendlatenotifications' => 0, + 'duedate' => 0, + 'allowsubmissionsfromdate' => 0, + 'grade' => 100, + 'cutoffdate' => 0, + 'teamsubmission' => 0, + 'requireallteammemberssubmit' => 0, + 'teamsubmissiongroupingid' => 0, + 'blindmarking' => 0, + 'attemptreopenmethod' => 'none', + 'maxattempts' => -1, + 'markingworkflow' => 1, + 'markingallocation' => 1, + ); + + $assignelement = $this->getDataGenerator()->create_module('assign', $record, $modulesettings); + + $coursesectionid = course_add_cm_to_section($this->course->id, $assignelement->id, 1); + + // Adding users to the course. + $userdata = array(); + $userdata['firstname'] = 'teacher1'; + $userdata['lasttname'] = 'lastname_teacher1'; + + $user1 = $this->getDataGenerator()->create_user($userdata); + + $this->getDataGenerator()->enrol_user($user1->id, $this->course->id, 'teacher'); + + $userdata = array(); + $userdata['firstname'] = 'teacher2'; + $userdata['lasttname'] = 'lastname_teacher2'; + + $user2 = $this->getDataGenerator()->create_user($userdata); + + $this->getDataGenerator()->enrol_user($user2->id, $this->course->id, 'teacher'); + + $userdata = array(); + $userdata['firstname'] = 'student'; + $userdata['lasttname'] = 'lastname_student'; + + $user3 = $this->getDataGenerator()->create_user($userdata); + + $this->getDataGenerator()->enrol_user($user3->id, $this->course->id, 'student'); + + // Adding manager to the system. + $userdata = array(); + $userdata['firstname'] = 'Manager'; + $userdata['lasttname'] = 'lastname_Manager'; + + $user4 = $this->getDataGenerator()->create_user($userdata); + + // Getting id of manager role. + $managerrole = $DB->get_record('role', array('shortname' => 'manager')); + if (!empty($managerrole)) { + // By default the context of the system is assigned. + $idassignment = $this->getDataGenerator()->role_assign($managerrole->id, $user4->id); + } + + $oldusers = array($user1, $user2, $user4); + $newusers = array($user1, $user2); + + list($sort, $params) = users_order_by_sql(); + + // Old code, it must return 3 users: teacher1, teacher2 and Manger. + $oldmarkers = get_users_by_capability(context_course::instance($this->course->id), 'mod/assign:grade', '', $sort); + // New code, it must return 2 users: teacher1 and teacher2. + $newmarkers = get_enrolled_users(context_course::instance($this->course->id), 'mod/assign:grade', 0, 'u.*', $sort); + + // Test result quantity. + $this->assertEquals(count($oldusers), count($oldmarkers)); + $this->assertEquals(count($newusers), count($newmarkers)); + $this->assertEquals(count($oldmarkers) > count($newmarkers), true); + + // Elements expected with new code. + foreach ($newmarkers as $key => $nm) { + $this->assertEquals($nm, $newusers[array_search($nm, $newusers)]); + } + + // Elements expected with old code. + foreach ($oldusers as $key => $os) { + $this->assertEquals($os->id, $oldmarkers[$os->id]->id); + unset($oldmarkers[$os->id]); + } + + $this->assertEquals(count($oldmarkers), 0); + + } +}