From 7d693114d1dd6389260f5a8e0fe106bc48db8d34 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 17 May 2022 15:56:55 +0100 Subject: [PATCH] MDL-73966 grade: fetch required user name fields of unenrolled users. --- grade/import/lib.php | 6 ++- grade/tests/importlib_test.php | 91 ++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/grade/import/lib.php b/grade/import/lib.php index 37186bd9ce7..a1855420bc4 100644 --- a/grade/import/lib.php +++ b/grade/import/lib.php @@ -173,7 +173,7 @@ function grade_import_commit($courseid, $importcode, $importfeedback=true, $verb * are still stored in the database, but will not be visible in the gradebook unless * this user subsequently enrols on the course in a graded roles. * - * The returned objects have fields user firstname, lastname and useridnumber, and gradeidnumber. + * The returned objects have fields useridnumber and gradeidnumber, plus enough user name fields to pass to {@see fullname} * * @param integer $importcode import batch identifier * @param integer $courseid the course we are importing to. @@ -190,12 +190,14 @@ function get_unenrolled_users_in_import($importcode, $courseid) { // Users with a gradeable role. list($gradebookrolessql, $gradebookrolesparams) = $DB->get_in_or_equal(explode(',', $CFG->gradebookroles), SQL_PARAMS_NAMED, 'grbr'); + $usernamefields = core_user\fields::for_name()->get_sql('u', false, '', '', false); + // Enrolled users. $context = context_course::instance($courseid); list($enrolledsql, $enrolledparams) = get_enrolled_sql($context); list($sort, $sortparams) = users_order_by_sql('u'); - $sql = "SELECT giv.id, u.firstname, u.lastname, u.idnumber AS useridnumber, + $sql = "SELECT giv.id, {$usernamefields->selects}, u.idnumber AS useridnumber, COALESCE(gi.idnumber, gin.itemname) AS gradeidnumber FROM {grade_import_values} giv JOIN {user} u diff --git a/grade/tests/importlib_test.php b/grade/tests/importlib_test.php index 4850ce2b183..83d6bd8915f 100644 --- a/grade/tests/importlib_test.php +++ b/grade/tests/importlib_test.php @@ -16,10 +16,7 @@ namespace core_grades; -defined('MOODLE_INTERNAL') || die(); - -global $CFG; -require_once($CFG->dirroot . '/grade/import/lib.php'); +use grade_item; /** * Tests grade_import_lib functions. @@ -31,6 +28,14 @@ require_once($CFG->dirroot . '/grade/import/lib.php'); */ class importlib_test extends \advanced_testcase { + /** + * Load required test libraries + */ + public static function setUpBeforeClass(): void { + global $CFG; + require_once("{$CFG->dirroot}/grade/import/lib.php"); + } + /** * Import grades into 'grade_import_values' table. This is done differently in the various import plugins, * so there is no direct API to call. @@ -72,6 +77,8 @@ class importlib_test extends \advanced_testcase { /** * Tests for importing grades from an external source. + * + * @covers ::grade_import_commit */ public function test_grade_import_commit() { global $USER, $DB, $CFG; @@ -199,4 +206,80 @@ class importlib_test extends \advanced_testcase { $this->assertTrue($status); $this->assertStringContainsString("++ Grade import success ++", $output); } + + /** + * Test grade import commit for users who aren't enrolled on the target course + * + * @covers ::grade_import_commit + */ + public function test_grade_import_commit_unenrolled_user(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $assign = $this->getDataGenerator()->create_module('assign', ['course' => $course->id]); + $user = $this->getDataGenerator()->create_user(['firstname' => 'Lionel', 'lastname' => 'Doe']); + + // Enter a new grade into an existing grade item. + $gradeitem = grade_item::fetch(['courseid' => $course->id, 'itemtype' => 'mod']); + + $importcode = get_new_importcode(); + $this->import_grades([ + 'importcode' => $importcode, + 'itemid' => $gradeitem->id, + 'userid' => $user->id, + 'finalgrade' => 10, + ]); + + ob_start(); + $status = grade_import_commit($course->id, $importcode); + $output = ob_get_contents(); + ob_end_clean(); + + // Assert commit succeeded and we didn't receive debugging about lack of name fields. + $this->assertTrue($status); + $this->assertStringContainsString('This import included the following grades for users not currently' . + ' enrolled in this course', $output); + $this->assertStringContainsString('User ' . fullname($user), $output); + $this->assertDebuggingNotCalled(); + } + + /** + * Test retrieving users included in impoty who aren't enrolled on the target course + * + * @covers ::get_unenrolled_users_in_import + */ + public function test_get_unenrolled_users_in_import(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + $assign = $this->getDataGenerator()->create_module('assign', ['course' => $course->id, 'idnumber' => 'gid101']); + $user = $this->getDataGenerator()->create_user(['idnumber' => 'uid101']); + + // Enter a new grade into an existing grade item. + $gradeitem = grade_item::fetch(['courseid' => $course->id, 'itemtype' => 'mod']); + + $importcode = get_new_importcode(); + $importgradeid = $this->import_grades([ + 'importcode' => $importcode, + 'itemid' => $gradeitem->id, + 'userid' => $user->id, + 'finalgrade' => 10, + ]); + + $unenrolledusers = get_unenrolled_users_in_import($importcode, $course->id); + $this->assertCount(1, $unenrolledusers); + + $unenrolleduser = reset($unenrolledusers); + $this->assertEquals((object) [ + 'id' => $importgradeid, + 'firstnamephonetic' => $user->firstnamephonetic, + 'lastnamephonetic' => $user->lastnamephonetic, + 'middlename' => $user->middlename, + 'alternatename' => $user->alternatename, + 'firstname' => $user->firstname, + 'lastname' => $user->lastname, + 'useridnumber' => 'uid101', + 'gradeidnumber' => 'gid101', + ], $unenrolleduser); + } }