From 2bd34edc2d7363954cc7e1e508bec0424482929d Mon Sep 17 00:00:00 2001 From: Mahmoud Kassaei Date: Tue, 21 Sep 2021 07:30:44 +0100 Subject: [PATCH] MDL-72612 Custom user field support: Quiz report grading --- .../grading/classes/privacy/provider.php | 9 +- mod/quiz/report/grading/db/access.php | 8 +- .../report/grading/gradingsettings_form.php | 43 ++++++--- .../report/grading/lang/en/deprecated.txt | 7 ++ .../report/grading/lang/en/quiz_grading.php | 21 ++-- mod/quiz/report/grading/report.php | 96 ++++++++++++------- .../grading/tests/behat/grading.feature | 40 +++++--- .../grading/tests/privacy_provider_test.php | 27 +++++- 8 files changed, 174 insertions(+), 77 deletions(-) create mode 100644 mod/quiz/report/grading/lang/en/deprecated.txt diff --git a/mod/quiz/report/grading/classes/privacy/provider.php b/mod/quiz/report/grading/classes/privacy/provider.php index 96594c0e44d..de4cd0e8864 100644 --- a/mod/quiz/report/grading/classes/privacy/provider.php +++ b/mod/quiz/report/grading/classes/privacy/provider.php @@ -68,10 +68,10 @@ class provider implements if ($order !== null) { switch ($order) { case 'random': - $order = get_string('randomly', 'quiz_grading'); + $order = get_string('random', 'quiz_grading'); break; case 'date': - $order = get_string('bydate', 'quiz_grading'); + $order = get_string('date'); break; case 'studentfirstname': $order = get_string('studentfirstname', 'quiz_grading'); @@ -79,9 +79,8 @@ class provider implements case 'studentlastname': $order = get_string('studentlastname', 'quiz_grading'); break; - case 'idnumber': - $order = get_string('bystudentidnumber', 'quiz_grading'); - break; + default: + $order = \core_user\fields::get_display_name($order); } writer::export_user_preference('quiz_grading', 'order', $order, get_string('privacy:preference:order', 'quiz_grading')); diff --git a/mod/quiz/report/grading/db/access.php b/mod/quiz/report/grading/db/access.php index 26658c3ca58..9f50665e6a2 100644 --- a/mod/quiz/report/grading/db/access.php +++ b/mod/quiz/report/grading/db/access.php @@ -33,10 +33,12 @@ $capabilities = array( 'teacher' => CAP_ALLOW, 'editingteacher' => CAP_ALLOW ), - 'clonepermissionsfrom' => 'mod/quiz:viewreports' + 'clonepermissionsfrom' => 'mod/quiz:viewreports' ), - // Is the user allowed to see the student's idnumber while grading? + // Is the user allowed to see the student's identity fields while grading? + // Note that the name of this capability is now out-of-date, but to preserve + // backwards compatibility, the name was not changed when the functionality was updated. 'quiz/grading:viewidnumber' => array( 'captype' => 'read', 'contextlevel' => CONTEXT_MODULE, @@ -44,6 +46,6 @@ $capabilities = array( 'teacher' => CAP_ALLOW, 'editingteacher' => CAP_ALLOW ), - 'clonepermissionsfrom' => 'mod/quiz:viewreports' + 'clonepermissionsfrom' => 'mod/quiz:viewreports' ) ); diff --git a/mod/quiz/report/grading/gradingsettings_form.php b/mod/quiz/report/grading/gradingsettings_form.php index 89400e22a2a..f4b8a8afe24 100644 --- a/mod/quiz/report/grading/gradingsettings_form.php +++ b/mod/quiz/report/grading/gradingsettings_form.php @@ -39,15 +39,30 @@ class quiz_grading_settings_form extends moodleform { protected $hidden = array(); protected $counts; protected $shownames; - protected $showidnumbers; - public function __construct($hidden, $counts, $shownames, $showidnumbers) { + /** @var bool $showcustomfields whether custom field values should be shown. */ + protected $showcustomfields; + + /** @var stdClass $context the quiz context. */ + protected $context; + + /** + * quiz_grading_settings_form constructor. + * + * @param array $hidden Array of options form. + * @param stdClass $counts object that stores the number of each type of attempt. + * @param bool $shownames whether student names should be shown. + * @param bool $showcustomfields whether custom field values should be shown. + * @param stdClass $context context object. + */ + public function __construct(array $hidden, stdClass $counts, bool $shownames, bool $showcustomfields, stdClass $context) { global $CFG; $this->includeauto = !empty($hidden['includeauto']); $this->hidden = $hidden; $this->counts = $counts; $this->shownames = $shownames; - $this->showidnumbers = $showidnumbers; + $this->showcustomfields = $showcustomfields; + $this->context = $context; parent::__construct($CFG->wwwroot . '/mod/quiz/report.php'); } @@ -75,18 +90,22 @@ class quiz_grading_settings_form extends moodleform { $mform->addRule('pagesize', null, 'positiveint', null, 'client'); $mform->setType('pagesize', PARAM_INT); - $orderoptions = array( - 'random' => get_string('randomly', 'quiz_grading'), - 'date' => get_string('bydate', 'quiz_grading'), - ); + $orderoptions = [ + 'random' => get_string('random', 'quiz_grading'), + 'date' => get_string('date') + ]; if ($this->shownames) { - $orderoptions['studentfirstname'] = get_string('bystudentfirstname', 'quiz_grading'); - $orderoptions['studentlastname'] = get_string('bystudentlastname', 'quiz_grading'); + $orderoptions['studentfirstname'] = get_string('firstname'); + $orderoptions['studentlastname'] = get_string('lastname'); } - if ($this->showidnumbers) { - $orderoptions['idnumber'] = get_string('bystudentidnumber', 'quiz_grading'); + // If the current user can see custom user fields, add the custom user fields to the select menu. + if ($this->showcustomfields) { + $userfieldsapi = \core_user\fields::for_identity($this->context); + foreach ($userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]) as $field) { + $orderoptions[s($field)] = \core_user\fields::get_display_name(s($field)); + } } - $mform->addElement('select', 'order', get_string('orderattempts', 'quiz_grading'), + $mform->addElement('select', 'order', get_string('orderattemptsby', 'quiz_grading'), $orderoptions); foreach ($this->hidden as $name => $value) { diff --git a/mod/quiz/report/grading/lang/en/deprecated.txt b/mod/quiz/report/grading/lang/en/deprecated.txt new file mode 100644 index 00000000000..72c89427244 --- /dev/null +++ b/mod/quiz/report/grading/lang/en/deprecated.txt @@ -0,0 +1,7 @@ +bydate,quiz_grading +bystudentidnumber,quiz_grading +bystudentfirstname,quiz_grading +bystudentlastname,quiz_grading +gradingattemptwithidnumber,quiz_grading +orderattempts,quiz_grading +randomly,quiz_grading diff --git a/mod/quiz/report/grading/lang/en/quiz_grading.php b/mod/quiz/report/grading/lang/en/quiz_grading.php index aa1224f51bb..08c65530fb4 100644 --- a/mod/quiz/report/grading/lang/en/quiz_grading.php +++ b/mod/quiz/report/grading/lang/en/quiz_grading.php @@ -28,10 +28,6 @@ $string['alsoshowautomaticallygraded'] = 'Also show questions that have been gra $string['attemptstograde'] = 'Attempts to grade'; $string['automaticallygraded'] = 'Automatically graded'; $string['backtothelistofquestions'] = 'Back to the list of questions'; -$string['bydate'] = 'By date'; -$string['bystudentidnumber'] = 'By student ID number'; -$string['bystudentfirstname'] = 'By student first name'; -$string['bystudentlastname'] = 'By student last name'; $string['cannotloadquestioninfo'] = 'Unable to load questiontype specific question information'; $string['cannotgradethisattempt'] = 'Cannot grade this attempt.'; $string['changeoptions'] = 'Change options'; @@ -48,11 +44,11 @@ $string['graded'] = '(graded)'; $string['gradenextungraded'] = 'Grade next {$a} ungraded attempts'; $string['gradeungraded'] = 'Grade all {$a} ungraded attempts'; $string['grading'] = 'Manual grading'; -$string['grading:viewidnumber'] = 'See student ID numbers while grading'; +$string['grading:viewidnumber'] = 'See student identity fields while grading'; $string['grading:viewstudentnames'] = 'See student names while grading'; $string['gradingall'] = 'All {$a} attempts on this question.'; $string['gradingattempt'] = 'Attempt number {$a->attempt} for {$a->fullname}'; -$string['gradingattemptwithidnumber'] = 'Attempt number {$a->attempt} for {$a->fullname} ({$a->idnumber})'; +$string['gradingattemptwithcustomfields'] = 'Attempt number {$a->attempt} for {$a->fullname} ({$a->customfields})'; $string['gradingattemptsxtoyofz'] = 'Grading attempts {$a->from} to {$a->to} of {$a->of}'; $string['gradingnextungraded'] = 'Next {$a} ungraded attempts'; $string['gradingnotallowed'] = 'You do not have permission to manually grade responses in this quiz'; @@ -65,7 +61,7 @@ $string['inprogress'] = 'In progress'; $string['noquestionsfound'] = 'No manually graded questions found'; $string['nothingfound'] = 'Nothing to display'; $string['options'] = 'Options'; -$string['orderattempts'] = 'Order attempts'; +$string['orderattemptsby'] = 'Order attempts by'; $string['pluginname'] = 'Manual grading'; $string['privacy:preference:order'] = 'What order to show the attempts that need grading.'; $string['privacy:preference:pagesize'] = 'How many attempts to show on each page of the grading interface.'; @@ -74,10 +70,19 @@ $string['questionname'] = 'Question name'; $string['questionsperpage'] = 'Questions per page'; $string['questionsthatneedgrading'] = 'Questions that need grading'; $string['questiontitle'] = 'Question {$a->number} : "{$a->name}" ({$a->openspan}{$a->gradedattempts}{$a->closespan} / {$a->totalattempts} attempts {$a->openspan}graded{$a->closespan}).'; -$string['randomly'] = 'Randomly'; +$string['random'] = 'Random'; $string['saveandnext'] = 'Save and go to next page'; $string['showstudentnames'] = 'Show student names'; $string['tograde'] = 'To grade'; $string['total'] = 'Total'; $string['unknownquestion'] = 'Unknown question'; $string['updategrade'] = 'update grades'; + +// Deprecated since Moodle 4.0. +$string['bydate'] = 'By date'; +$string['bystudentidnumber'] = 'By student ID number'; +$string['bystudentfirstname'] = 'By student first name'; +$string['bystudentlastname'] = 'By student last name'; +$string['gradingattemptwithidnumber'] = 'Attempt number {$a->attempt} for {$a->fullname} ({$a->idnumber})'; +$string['orderattempts'] = 'Order attempts'; +$string['randomly'] = 'Randomly'; diff --git a/mod/quiz/report/grading/report.php b/mod/quiz/report/grading/report.php index f58c735c43a..2a3294aae77 100644 --- a/mod/quiz/report/grading/report.php +++ b/mod/quiz/report/grading/report.php @@ -72,6 +72,9 @@ class quiz_grading_report extends quiz_default_report { /** @var string fragment of SQL code to restrict to the relevant users. */ protected $userssql; + /** @var array extra user fields. */ + protected $extrauserfields = []; + public function display($quiz, $cm, $course) { $this->quiz = $quiz; @@ -93,7 +96,7 @@ class quiz_grading_report extends quiz_default_report { $page = optional_param('page', 0, PARAM_INT); $order = optional_param('order', get_user_preferences('quiz_grading_order', self::DEFAULT_ORDER), - PARAM_ALPHA); + PARAM_ALPHAEXT); // Assemble the options required to reload this page. $optparams = array('includeauto', 'page'); @@ -117,14 +120,20 @@ class quiz_grading_report extends quiz_default_report { $this->context = context_module::instance($this->cm->id); require_capability('mod/quiz:grade', $this->context); $shownames = has_capability('quiz/grading:viewstudentnames', $this->context); - $showidnumbers = has_capability('quiz/grading:viewidnumber', $this->context); - + // Whether the current user can see custom user fields. + $showcustomfields = has_capability('quiz/grading:viewidnumber', $this->context); + $userfieldsapi = \core_user\fields::for_identity($this->context)->with_name(); + $customfields = []; + foreach ($userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]) as $field) { + $customfields[] = $field; + } // Validate order. - if (!in_array($order, array('random', 'date', 'studentfirstname', 'studentlastname', 'idnumber'))) { + $orderoptions = array_merge(['random', 'date', 'studentfirstname', 'studentlastname'], $customfields); + if (!in_array($order, $orderoptions)) { $order = self::DEFAULT_ORDER; } else if (!$shownames && ($order == 'studentfirstname' || $order == 'studentlastname')) { $order = self::DEFAULT_ORDER; - } else if (!$showidnumbers && $order == 'idnumber') { + } else if (!$showcustomfields && in_array($order, $customfields)) { $order = self::DEFAULT_ORDER; } if ($order == 'random') { @@ -183,7 +192,7 @@ class quiz_grading_report extends quiz_default_report { } $this->display_grading_interface($slot, $questionid, $grade, - $pagesize, $page, $shownames, $showidnumbers, $order, $counts); + $pagesize, $page, $shownames, $showcustomfields, $order, $counts); return true; } @@ -230,13 +239,19 @@ class quiz_grading_report extends quiz_default_report { $params[] = quiz_attempt::FINISHED; $params[] = $this->quiz->id; - $fields = 'quiza.*, u.idnumber, '; - $userfieldsapi = \core_user\fields::for_name(); - $fields .= $userfieldsapi->get_sql('u', false, '', '', false)->selects; + $fields = 'quiza.*, '; + $userfieldsapi = \core_user\fields::for_identity($this->context)->with_name(); + $userfieldssql = $userfieldsapi->get_sql('u', false, '', 'userid', false); + $fields .= $userfieldssql->selects; + foreach ($userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]) as $userfield) { + $this->extrauserfields[] = s($userfield); + } + $params = array_merge($userfieldssql->params, $params); $attemptsbyid = $DB->get_records_sql(" SELECT $fields FROM {quiz_attempts} quiza JOIN {user} u ON u.id = quiza.userid + {$userfieldssql->joins} WHERE quiza.uniqueid $asql AND quiza.state = ? AND quiza.quiz = ?", $params); @@ -391,12 +406,12 @@ class quiz_grading_report extends quiz_default_report { * @param int $pagesize number of questions to show per page. * @param int $page current page number. * @param bool $shownames whether student names should be shown. - * @param bool $showidnumbers wither student idnumbers should be shown. + * @param bool $showcustomfields whether custom field values should be shown. * @param string $order preferred order of attempts. * @param stdClass $counts object that stores the number of each type of attempt. */ protected function display_grading_interface($slot, $questionid, $grade, - $pagesize, $page, $shownames, $showidnumbers, $order, $counts) { + $pagesize, $page, $shownames, $showcustomfields, $order, $counts) { if ($pagesize * $page >= $counts->$grade) { $page = 0; @@ -413,7 +428,7 @@ class quiz_grading_report extends quiz_default_report { if (array_key_exists('includeauto', $this->viewoptions)) { $hidden['includeauto'] = $this->viewoptions['includeauto']; } - $mform = new quiz_grading_settings_form($hidden, $counts, $shownames, $showidnumbers); + $mform = new quiz_grading_settings_form($hidden, $counts, $shownames, $showcustomfields, $this->context); // Tell the form the current settings. $settings = new stdClass(); @@ -468,7 +483,7 @@ class quiz_grading_report extends quiz_default_report { $slot, $displayoptions, $this->questions[$slot]->number, - $this->get_question_heading($attempt, $shownames, $showidnumbers) + $this->get_question_heading($attempt, $shownames, $showcustomfields) ); } @@ -621,6 +636,13 @@ class quiz_grading_report extends quiz_default_report { $qubaids = $this->get_qubaids_condition(); $params = []; + $userfieldsapi = \core_user\fields::for_identity($this->context)->with_name(); + $userfieldssql = $userfieldsapi->get_sql('u', true, '', 'userid', true); + $params = array_merge($params, $userfieldssql->params); + $customfields = []; + foreach ($userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]) as $field) { + $customfields[] = $field; + } if ($orderby == 'date') { list($statetest, $params) = $dm->in_summary_state_test( 'manuallygraded', false, 'mangrstate'); @@ -630,20 +652,16 @@ class quiz_grading_report extends quiz_default_report { WHERE sortqas.questionattemptid = qa.id AND sortqas.state $statetest )"; - } else if ($orderby == 'studentfirstname' || $orderby == 'studentlastname' || $orderby == 'idnumber') { - $qubaids->from .= " JOIN {user} u ON quiza.userid = u.id "; + } else if ($orderby == 'studentfirstname' || $orderby == 'studentlastname' || in_array($orderby, $customfields)) { + $qubaids->from .= " JOIN {user} u ON quiza.userid = u.id {$userfieldssql->joins}"; // For name sorting, map orderby form value to // actual column names; 'idnumber' maps naturally. - switch ($orderby) { - case "studentlastname": - $orderby = "u.lastname, u.firstname"; - break; - case "studentfirstname": - $orderby = "u.firstname, u.lastname"; - break; - case "idnumber": - $orderby = "u.idnumber"; - break; + if ($orderby === "studentlastname") { + $orderby = "u.lastname, u.firstname"; + } else if ($orderby === "studentfirstname") { + $orderby = "u.firstname, u.lastname"; + } else if (in_array($orderby, $customfields)) { // Sort order by current custom user field. + $orderby = $userfieldssql->mappings[$orderby]; } } @@ -668,26 +686,30 @@ class quiz_grading_report extends quiz_default_report { /** * Get question heading. * - * @param object $attempt an instance of quiz_attempt. - * @param bool $shownames True to show the question name. - * @param bool $showidnumbers True to show the question id number. + * @param stdClass $attempt An instance of quiz_attempt. + * @param bool $shownames True to show the student first/lastnames. + * @param bool $showcustomfields Whether custom field values should be shown. * @return string The string text for the question heading. - * @throws coding_exception */ - protected function get_question_heading($attempt, $shownames, $showidnumbers) { + protected function get_question_heading(stdClass $attempt, bool $shownames, bool $showcustomfields): string { + global $DB; $a = new stdClass(); $a->attempt = $attempt->attempt; $a->fullname = fullname($attempt); - $a->idnumber = s($attempt->idnumber); + $customfields = []; + foreach ($this->extrauserfields as $field) { + if ($attempt->{s($field)}) { + $customfields[] = $attempt->{s($field)}; + } + } + $a->customfields = trim(implode(', ', (array)$customfields), ' ,'); - $showidnumbers = $showidnumbers && !empty($attempt->idnumber); - - if ($shownames && $showidnumbers) { - return get_string('gradingattemptwithidnumber', 'quiz_grading', $a); + if ($shownames && $showcustomfields) { + return get_string('gradingattemptwithcustomfields', 'quiz_grading', $a); } else if ($shownames) { return get_string('gradingattempt', 'quiz_grading', $a); - } else if ($showidnumbers) { - $a->fullname = $attempt->idnumber; + } else if ($showcustomfields) { + $a->fullname = $a->customfields; return get_string('gradingattempt', 'quiz_grading', $a); } else { return ''; diff --git a/mod/quiz/report/grading/tests/behat/grading.feature b/mod/quiz/report/grading/tests/behat/grading.feature index fd0aee8a6df..eb9b54a4140 100644 --- a/mod/quiz/report/grading/tests/behat/grading.feature +++ b/mod/quiz/report/grading/tests/behat/grading.feature @@ -5,10 +5,19 @@ Feature: Basic use of the Manual grading report I need to use the manual grading report Background: - Given the following "users" exist: - | username | firstname | lastname | email | idnumber | - | teacher1 | T1 | Teacher1 | teacher1@example.com | T1000 | - | student1 | S1 | Student1 | student1@example.com | S1000 | + Given the following "custom profile fields" exist: + | datatype | shortname | name | + | text | username | Username | + | text | email | Email address | + | text | idnumber | ID number | + | text | frog | Favourite frog | + And the following config values are set as admin: + | showuseridentity | username,idnumber,email,profile_field_frog | + + And the following "users" exist: + | username | firstname | lastname | email | idnumber | profile_field_frog | + | teacher1 | T1 | Teacher1 | teacher1@example.com | T1000 | | + | student1 | S1 | Student1 | student1@example.com | S1000 | little yellow frog | And the following "courses" exist: | fullname | shortname | category | | Course 1 | C1 | 0 | @@ -52,9 +61,8 @@ Feature: Basic use of the Manual grading report # Go to the grading page. And I click on "update grades" "link" in the "Short answer 001" "table_row" And I should see "Grading attempts 1 to 1 of 1" - # Test the display options. - And I set the field "Order attempts" to "By student ID number" + And I set the field "Order attempts by" to "ID number" And I press "Change options" # General feedback for Short answer 001 displays. @@ -77,16 +85,16 @@ Feature: Basic use of the Manual grading report And I follow "Also show questions that have been graded automatically" And I click on "update grades" "link" in the "Short answer 001" "table_row" And I set the following fields to these values: - | Questions per page | 42 | - | Order attempts | By date | + | Questions per page | 42 | + | Order attempts by | Date | And I press "Change options" And I log out And I am on the "Quiz 1" "mod_quiz > Manual grading report" page logged in as "teacher1" And I follow "Also show questions that have been graded automatically" And I click on "update grades" "link" in the "Short answer 001" "table_row" Then the following fields match these values: - | Questions per page | 42 | - | Order attempts | By date | + | Questions per page | 42 | + | Order attempts by | Date | @javascript Scenario: Manual grading settings are validated @@ -111,4 +119,14 @@ Feature: Basic use of the Manual grading report And I set the following fields to these values: | Questions per page | 1 | And I press "Change options" - And I should not see "You must enter a number that is greater than 0." + + @javascript + Scenario: Teacher can see user custom filed columns as additional user identity + Given user "student1" has attempted "Quiz 1" with responses: + | slot | response | + | 1 | Paris | + When I am on the "Quiz 1" "mod_quiz > View" page logged in as "teacher1" + And I navigate to "Results > Manual grading" in current page administration + And I follow "Also show questions that have been graded automatically" + And I click on "update grades" "link" in the "Short answer 001" "table_row" + Then I should see "Attempt number 1 for S1 Student1 (student1, S1000, student1@example.com, little yellow frog)" diff --git a/mod/quiz/report/grading/tests/privacy_provider_test.php b/mod/quiz/report/grading/tests/privacy_provider_test.php index 96c3f5ee4ef..67cc6b79e5f 100644 --- a/mod/quiz/report/grading/tests/privacy_provider_test.php +++ b/mod/quiz/report/grading/tests/privacy_provider_test.php @@ -73,6 +73,31 @@ class quiz_grading_privacy_provider_testcase extends \core_privacy\tests\provide $this->assertEquals(42, $preferences->pagesize->value); $this->assertNotEmpty($preferences->order); - $this->assertEquals('Randomly', $preferences->order->value); + $this->assertEquals('Random', $preferences->order->value); + } + + /** + * Preference does exist using user custom fields. + */ + public function test_preference_bool_true_for_user_customfields() { + global $USER; + + $this->resetAfterTest(); + $this->setAdminUser(); + + $customfields = ['username', 'idnumber', 'email', 'profile_field_frog']; + foreach ($customfields as $customfield) { + set_user_preference('quiz_grading_order', $customfield); + + provider::export_user_preferences($USER->id); + + $writer = writer::with_context(\context_system::instance()); + $this->assertTrue($writer->has_any_data()); + + $preferences = $writer->get_user_preferences('quiz_grading'); + + $this->assertNotEmpty($preferences->order); + $this->assertEquals(\core_user\fields::get_display_name($customfield), $preferences->order->value); + } } }