MDL-76318 gradebook: improve csv import error message

grade/import/csv/classes/load_data.php

@@ -300,9 +300,10 @@ protected function create_feedback($courseid, $itemid, $value) {
      * @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, $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 @@ protected function update_grade_item($courseid, $map, $key, $verbosescales, $val
                 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 @@ protected function update_grade_item($courseid, $map, $key, $verbosescales, $val
                     $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,10 @@ protected function cleanup_import($notification) {
      * @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) {
+            $verbosescales, $linenumber) {
 
         // Fields that the user can be mapped from.
         $userfields = array(
@@ -424,7 +432,7 @@ protected function map_user_data_with_value($mappingidentifier, $value, $header,
                 // Existing grade items.
                 if (!empty($map[$key])) {
                     $this->newgrades = $this->update_grade_item($courseid, $map, $key, $verbosescales, $value,
-                            $mappingidentifier);
+                            $linenumber);
                 }
                 // Otherwise, we ignore this column altogether because user has chosen
                 // to ignore them (e.g. institution, address etc).
@@ -493,11 +501,13 @@ public function prepare_import_grade_data($header, $formdata, $csvimport, $cours
 
         $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();
@@ -528,7 +538,7 @@ public function prepare_import_grade_data($header, $formdata, $csvimport, $cours
                 }
 
                 $this->map_user_data_with_value($mappingidentifier, $value, $header, $map, $key, $courseid, $feedbackgradeid,
-                        $verbosescales);
+                        $verbosescales, $linenumber);
                 if ($this->status === false) {
                     return $this->status;
                 }

grade/import/csv/lang/en/gradeimport_csv.php

@@ -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['pluginname'] = 'CSV file';
 $string['privacy:metadata'] = 'The import grades from CSV plugin does not store any personal data.';

grade/import/csv/tests/fixtures/phpunit_gradeimport_csv_load_data.php

@@ -81,8 +81,8 @@ public function test_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);
+    public function test_update_grade_item($courseid, $map, $key, $verbosescales, $value, $linenumber) {
+        return $this->update_grade_item($courseid, $map, $key, $verbosescales, $value, $linenumber);
     }
 
     /**
@@ -96,11 +96,11 @@ public function test_update_grade_item($courseid, $map, $key, $verbosescales, $v
      * @return array grades to be updated.
      */
     public function test_map_user_data_with_value($mappingidentifier, $value, $header, $map, $key, $courseid, $feedbackgradeid,
-            $verbosescales) {
+            $verbosescales, $linenumber) {
         // Set an import code.
         $this->importcode = 00001;
         $this->map_user_data_with_value($mappingidentifier, $value, $header, $map, $key, $courseid, $feedbackgradeid,
-                $verbosescales);
+                $verbosescales, $linenumber);
 
         switch ($mappingidentifier) {
             case 'userid':

grade/import/csv/tests/load_data_test.php

@@ -382,11 +382,13 @@ public function test_update_grade_item(): void {
 
         // 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 @@ public function test_update_grade_item(): void {
         $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,31 @@ public function test_map_user_data_with_value(): void {
 
         // 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);
+                $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);
+                $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);
+                $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);
+                $this->courseid, $map[$key], $verbosescales, $linenumber);
         // Expected result.
         $resultarray = array();
         $resultarray[0] = new \stdClass();
@@ -454,7 +457,7 @@ public function test_map_user_data_with_value(): void {
 
         // 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);
+                $this->courseid, $map[$key], $verbosescales, $linenumber);
         $this->assertEquals($testarray[0][6], $newgrades[0]->finalgrade);
     }