MDL-76318 gradebook: Improve csv import error message

This commit is contained in:
Julien Boulen
2025-08-22 09:57:01 +02:00
parent c1311feeb1
commit ead32c158e
4 changed files with 162 additions and 40 deletions
+43 -9
View File
@@ -300,9 +300,10 @@ class gradeimport_csv_load_data {
* @param int $key The line that we are currently working on.
* @param bool $verbosescales Form setting for grading with scales.
* @param string $value The grade value.
* @param int $linenumber The line number that we are currently working on.
* @return array grades to be updated.
*/
protected function update_grade_item($courseid, $map, $key, $verbosescales, $value) {
protected function update_grade_item($courseid, $map, $key, $verbosescales, $value, int $linenumber) {
// Case of an id, only maps id of a grade_item.
// This was idnumber.
if (!$gradeitem = new grade_item(array('id' => $map[$key], 'courseid' => $courseid))) {
@@ -330,7 +331,10 @@ class gradeimport_csv_load_data {
array_unshift($scales, '-'); // Scales start at key 1.
$key = array_search($value, $scales);
if ($key === false) {
$this->cleanup_import(get_string('badgrade', 'grades'));
$this->cleanup_import(get_string('badgrade', 'gradeimport_csv', [
'badgrade' => $value,
'linenumber' => $linenumber,
]));
return null;
}
$value = $key;
@@ -346,7 +350,10 @@ class gradeimport_csv_load_data {
$value = $validvalue;
} else {
// Non numeric grade value supplied, possibly mapped wrong column.
$this->cleanup_import(get_string('badgrade', 'grades'));
$this->cleanup_import(get_string('badgrade', 'gradeimport_csv', [
'badgrade' => $value,
'linenumber' => $linenumber,
]));
return null;
}
}
@@ -378,9 +385,19 @@ class gradeimport_csv_load_data {
* @param int $courseid The course ID.
* @param int $feedbackgradeid The ID of the grade item that the feedback relates to.
* @param bool $verbosescales Form setting for grading with scales.
* @param int $linenumber The line number that we are currently working on.
*/
protected function map_user_data_with_value($mappingidentifier, $value, $header, $map, $key, $courseid, $feedbackgradeid,
$verbosescales) {
protected function map_user_data_with_value(
$mappingidentifier,
$value,
$header,
$map,
$key,
$courseid,
$feedbackgradeid,
$verbosescales,
int $linenumber
) {
// Fields that the user can be mapped from.
$userfields = array(
@@ -423,8 +440,14 @@ class gradeimport_csv_load_data {
default:
// Existing grade items.
if (!empty($map[$key])) {
$this->newgrades = $this->update_grade_item($courseid, $map, $key, $verbosescales, $value,
$mappingidentifier);
$this->newgrades = $this->update_grade_item(
$courseid,
$map,
$key,
$verbosescales,
$value,
$linenumber
);
}
// Otherwise, we ignore this column altogether because user has chosen
// to ignore them (e.g. institution, address etc).
@@ -493,11 +516,13 @@ class gradeimport_csv_load_data {
$csvimport->init();
$linenumber = 1;
while ($line = $csvimport->next()) {
if (count($line) <= 1) {
// There is no data on this line, move on.
continue;
}
$linenumber++;
// Array to hold all grades to be inserted.
$this->newgrades = array();
@@ -527,8 +552,17 @@ class gradeimport_csv_load_data {
$feedbackgradeid = '';
}
$this->map_user_data_with_value($mappingidentifier, $value, $header, $map, $key, $courseid, $feedbackgradeid,
$verbosescales);
$this->map_user_data_with_value(
$mappingidentifier,
$value,
$header,
$map,
$key,
$courseid,
$feedbackgradeid,
$verbosescales,
$linenumber
);
if ($this->status === false) {
return $this->status;
}
@@ -23,6 +23,7 @@
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
$string['badgrade'] = 'Supplied grade (\'{$a->badgrade}\') is invalid on line {$a->linenumber}.';
$string['csv:view'] = 'Import grades from CSV';
$string['gradepenalties'] = 'Grade penalties will not be applied to imported grades.';
$string['pluginname'] = 'CSV file';
@@ -78,13 +78,6 @@ class phpunit_gradeimport_csv_load_data extends gradeimport_csv_load_data {
return $this->create_feedback($courseid, $itemid, $value);
}
/**
* Method to open up the appropriate method for unit testing.
*/
public function test_update_grade_item($courseid, $map, $key, $verbosescales, $value) {
return $this->update_grade_item($courseid, $map, $key, $verbosescales, $value);
}
/**
* Method to open up the appropriate method for unit testing.
*
@@ -92,15 +85,66 @@ class phpunit_gradeimport_csv_load_data extends gradeimport_csv_load_data {
* @param array $map Mapping information provided by the user.
* @param int $key The line that we are currently working on.
* @param bool $verbosescales Form setting for grading with scales.
* @param string $value The grade value .
* @param string $value The grade value.
* @param int $linenumber The line number that we are currently working on.
* @return array grades to be updated.
*/
public function test_map_user_data_with_value($mappingidentifier, $value, $header, $map, $key, $courseid, $feedbackgradeid,
$verbosescales) {
public function test_update_grade_item(
int $courseid,
array $map,
int $key,
bool $verbosescales,
string $value,
int $linenumber
) {
return $this->update_grade_item(
$courseid,
$map,
$key,
$verbosescales,
$value,
$linenumber
);
}
/**
* Method to open up the appropriate method for unit testing.
*
* @param string $mappingidentifier The user field that we are matching together.
* @param string $value The value we are checking / importing.
* @param array $header The column headers of the csv file.
* @param array $map Mapping information provided by the user.
* @param int $key Current row identifier.
* @param int $courseid The course ID.
* @param int $feedbackgradeid The ID of the grade item that the feedback relates to.
* @param bool $verbosescales Form setting for grading with scales.
* @param int $linenumber The line number that we are currently working on.
* @return mixed Returns an integer representing a user ID, an array of feedbacks or an array of grades.
*/
public function test_map_user_data_with_value(
string $mappingidentifier,
string $value,
array $header,
array $map,
int $key,
int $courseid,
int $feedbackgradeid,
bool $verbosescales,
int $linenumber
) {
// Set an import code.
$this->importcode = 00001;
$this->map_user_data_with_value($mappingidentifier, $value, $header, $map, $key, $courseid, $feedbackgradeid,
$verbosescales);
$this->map_user_data_with_value(
$mappingidentifier,
$value,
$header,
$map,
$key,
$courseid,
$feedbackgradeid,
$verbosescales,
$linenumber
);
switch ($mappingidentifier) {
case 'userid':
@@ -108,16 +152,11 @@ class phpunit_gradeimport_csv_load_data extends gradeimport_csv_load_data {
case 'useremail':
case 'username':
return $this->studentid;
break;
case 'new':
return $this->newgrades;
break;
case 'feedback':
return $this->newfeedbacks;
break;
case 'new':
default:
return $this->newgrades;
break;
}
}
}
@@ -382,11 +382,13 @@ Bobby,Bunce,,"Moodle HQ","Rock on!",[email protected],75.00,,75.00,{exportdat
// We're not using scales so no to this option.
$verbosescales = 0;
$linenumber = 2;
// Map and key are to retrieve the grade_item that we are updating.
$map = array(1);
$key = 0;
// We return the new grade array for saving.
$newgrades = $testobject->test_update_grade_item($this->courseid, $map, $key, $verbosescales, $testarray[0][6]);
$grade = $testarray[0][6];
$newgrades = $testobject->test_update_grade_item($this->courseid, $map, $key, $verbosescales, $grade, $linenumber);
$expectedresult = array();
$expectedresult[0] = new \stdClass();
@@ -396,10 +398,10 @@ Bobby,Bunce,,"Moodle HQ","Rock on!",[email protected],75.00,,75.00,{exportdat
$this->assertEquals($newgrades, $expectedresult);
// Try sending a bad grade value (A letter instead of a float / int).
$newgrades = $testobject->test_update_grade_item($this->courseid, $map, $key, $verbosescales, 'A');
$newgrades = $testobject->test_update_grade_item($this->courseid, $map, $key, $verbosescales, 'A', $linenumber);
// The $newgrades variable should be null.
$this->assertNull($newgrades);
$expectederrormessage = get_string('badgrade', 'grades');
$expectederrormessage = get_string('badgrade', 'gradeimport_csv', ['badgrade' => 'A', 'linenumber' => $linenumber]);
// Check that the error message is what we expect.
$gradebookerrors = $testobject->get_gradebookerrors();
$this->assertEquals($expectederrormessage, $gradebookerrors[0]);
@@ -421,30 +423,67 @@ Bobby,Bunce,,"Moodle HQ","Rock on!",[email protected],75.00,,75.00,{exportdat
// We're not using scales so no to this option.
$verbosescales = 0;
$linenumber = 2;
// Map and key are to retrieve the grade_item that we are updating.
$map = array(1);
$key = 0;
// Test new user mapping. This should return the user id if there were no problems.
$userid = $testobject->test_map_user_data_with_value('useremail', $testarray[0][5], $this->columns, $map, $key,
$this->courseid, $map[$key], $verbosescales);
$userid = $testobject->test_map_user_data_with_value(
'useremail',
$testarray[0][5],
$this->columns,
$map,
$key,
$this->courseid,
$map[$key],
$verbosescales,
$linenumber
);
$this->assertEquals($userid, $userdetail->id);
$newgrades = $testobject->test_map_user_data_with_value('new', $testarray[0][6], $this->columns, $map, $key,
$this->courseid, $map[$key], $verbosescales);
$newgrades = $testobject->test_map_user_data_with_value(
'new',
$testarray[0][6],
$this->columns,
$map,
$key,
$this->courseid,
$map[$key],
$verbosescales,
$linenumber
);
// Check that the final grade is the same as the one inserted.
$this->assertEquals($testarray[0][6], $newgrades[0]->finalgrade);
$newgrades = $testobject->test_map_user_data_with_value('new', $testarray[0][8], $this->columns, $map, $key,
$this->courseid, $map[$key], $verbosescales);
$newgrades = $testobject->test_map_user_data_with_value(
'new',
$testarray[0][8],
$this->columns,
$map,
$key,
$this->courseid,
$map[$key],
$verbosescales,
$linenumber
);
// Check that the final grade is the same as the one inserted.
// The testobject should now contain 2 new grade items.
$this->assertEquals(2, count($newgrades));
// Because this grade item is empty, the value for final grade should be null.
$this->assertNull($newgrades[1]->finalgrade);
$feedback = $testobject->test_map_user_data_with_value('feedback', $testarray[0][7], $this->columns, $map, $key,
$this->courseid, $map[$key], $verbosescales);
$feedback = $testobject->test_map_user_data_with_value(
'feedback',
$testarray[0][7],
$this->columns,
$map,
$key,
$this->courseid,
$map[$key],
$verbosescales,
$linenumber
);
// Expected result.
$resultarray = array();
$resultarray[0] = new \stdClass();
@@ -453,8 +492,17 @@ Bobby,Bunce,,"Moodle HQ","Rock on!",[email protected],75.00,,75.00,{exportdat
$this->assertEquals($feedback, $resultarray);
// Default behaviour (update a grade item).
$newgrades = $testobject->test_map_user_data_with_value('default', $testarray[0][6], $this->columns, $map, $key,
$this->courseid, $map[$key], $verbosescales);
$newgrades = $testobject->test_map_user_data_with_value(
'default',
$testarray[0][6],
$this->columns,
$map,
$key,
$this->courseid,
$map[$key],
$verbosescales,
$linenumber
);
$this->assertEquals($testarray[0][6], $newgrades[0]->finalgrade);
}