MDL-50650 core_grades: Validate minimum grade when importing grades

Also add behat test for max and min grade validation.

grade/import/csv/classes/load_data.php

@@ -149,24 +149,31 @@ protected function raise_limits() {
     /**
      * Inserts a record into the grade_import_values table. This also adds common record information.
      *
-     * @param object $record The grade record being inserted into the database.
+     * @param stdClass $record The grade record being inserted into the database.
      * @param int $studentid The student ID.
-     * @return bool|int true or insert id on success. Null if the grade value is too high.
+     * @param grade_item $gradeitem Grade item.
+     * @return bool|int true or insert id on success. Null if the grade value is too high or too low or grade item not exist.
      */
-    protected function insert_grade_record($record, $studentid) {
+    protected function insert_grade_record(stdClass $record, int $studentid, grade_item $gradeitem) {
         global $DB, $USER, $CFG;
         $record->importcode = $this->importcode;
         $record->userid     = $studentid;
         $record->importer   = $USER->id;
-        // By default the maximum grade is 100.
-        // If the grade limit has been increased then use the gradepointmax setting.
-        // Unlimitedgrades allows for scores over 100%.
         // If the record final grade is set then check that the grade value isn't too high.
         // Final grade will not be set if we are inserting feedback.
-        if (!isset($record->finalgrade) || $record->finalgrade <= $CFG->gradepointmax || $CFG->unlimitedgrades) {
+        $gradepointmaximum = $gradeitem->grademax;
+        $gradepointminimum = $gradeitem->grademin;
+
+        $finalgradeinrange =
+            isset($record->finalgrade) && $record->finalgrade <= $gradepointmaximum && $record->finalgrade >= $gradepointminimum;
+        if (!isset($record->finalgrade) || $finalgradeinrange || $CFG->unlimitedgrades) {
             return $DB->insert_record('grade_import_values', $record);
         } else {
-            $this->cleanup_import(get_string('gradevaluetoobig', 'grades', $CFG->gradepointmax));
+            if ($record->finalgrade > $gradepointmaximum) {
+                $this->cleanup_import(get_string('gradevaluetoobig', 'grades', format_float($gradepointmaximum)));
+            } else {
+                $this->cleanup_import(get_string('gradevaluetoosmall', 'grades', format_float($gradepointminimum)));
+            }
             return null;
         }
     }
@@ -572,7 +579,12 @@ public function prepare_import_grade_data($header, $formdata, $csvimport, $cours
                             }
                         }
                     }
-                    $insertid = self::insert_grade_record($newgrade, $this->studentid);
+                    if (isset($newgrade->itemid)) {
+                        $gradeitem = new grade_item(['id' => $newgrade->itemid]);
+                    } else if (isset($newgrade->newgradeitem)) {
+                        $gradeitem = new grade_item(['id' => $newgrade->newgradeitem]);
+                    }
+                    $insertid = isset($gradeitem) ? self::insert_grade_record($newgrade, $this->studentid, $gradeitem) : null;
                     // Check to see if the insert was successful.
                     if (empty($insertid)) {
                         return null;
@@ -594,7 +606,7 @@ public function prepare_import_grade_data($header, $formdata, $csvimport, $cours
                     } else {
                         // The grade item for this is not updated.
                         $newfeedback->importonlyfeedback = true;
-                        $insertid = self::insert_grade_record($newfeedback, $this->studentid);
+                        $insertid = self::insert_grade_record($newfeedback, $this->studentid, new grade_item(['id' => $newfeedback->itemid]));
                         // Check to see if the insert was successful.
                         if (empty($insertid)) {
                             return null;

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

@@ -27,10 +27,11 @@ class phpunit_gradeimport_csv_load_data extends gradeimport_csv_load_data {
      *
      * @param object $record
      * @param int $studentid
+     * @param grade_item $gradeitem
      */
-    public function test_insert_grade_record($record, $studentid) {
+    public function test_insert_grade_record($record, $studentid, grade_item $gradeitem) {
         $this->importcode = 00001;
-        $this->insert_grade_record($record, $studentid);
+        $this->insert_grade_record($record, $studentid, $gradeitem);
     }
 
     /**

grade/import/csv/tests/load_data_test.php

@@ -187,7 +187,8 @@ public function test_insert_grade_record() {
         $record->feedback = 'Some test feedback';
 
         $testobject = new \phpunit_gradeimport_csv_load_data();
-        $testobject->test_insert_grade_record($record, $user->id);
+
+        $testobject->test_insert_grade_record($record, $user->id, new \grade_item());
 
         $gradeimportvalues = $DB->get_records('grade_import_values');
         // Get the insert id.

grade/tests/behat/grade_import.feature

@@ -0,0 +1,68 @@
+@core @core_grades @javascript @_file_upload
+Feature: An admin can import grades into gradebook using a CSV file
+  In order to import grades using a CSV file
+  As a teacher
+  I need to be able to upload a CSV file and see uploaded grades in gradebook
+
+  Background:
+    Given the following "courses" exist:
+      | fullname | shortname | format |
+      | Course 1 | C1        | topics |
+    And the following "users" exist:
+      | username | firstname | lastname | email                |
+      | teacher1 | Teacher   | 1        | teacher1@example.com |
+      | student1 | Student   | 1        | student1@example.com |
+      | student2 | Student   | 2        | student2@example.com |
+      | student3 | Student   | 3        | student3@example.com |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | teacher1 | C1     | editingteacher |
+      | student1 | C1     | student        |
+      | student2 | C1     | student        |
+      | student3 | C1     | student        |
+    And the following "grade item" exists:
+      | course         | C1            |
+      | itemname       | Manual item 1 |
+      | grademin       | 10            |
+      | grademax       | 500           |
+    And the following "grade grades" exist:
+      | gradeitem     | user     | grade |
+      | Manual item 1 | student1 | 50.00 |
+      | Manual item 1 | student2 | 50.00 |
+      | Manual item 1 | student3 | 50.00 |
+
+  @javascript
+  Scenario: Max grade of grade item is respected when importing grades
+    Given I am on the "Course 1" "Course" page logged in as "teacher1"
+    And I navigate to "More > Import" in the course gradebook
+    And I upload "grade/tests/fixtures/grade_import_grademax.csv" file to "File" filemanager
+    And I click on "Upload grades" "button"
+    And I set the field "Map from" to "Email address"
+    And I set the field "Map to" to "Email address"
+    And I set the field "Manual item 1" to "Manual item 1"
+    And I click on "Upload grades" "button"
+    And I should see "One of the grade values is larger than the allowed grade maximum of 500.0"
+    And I should see "Import failed. No data was imported."
+    And I click on "Continue" "button"
+    And I upload "grade/tests/fixtures/grade_import_grademin.csv" file to "File" filemanager
+    And I click on "Upload grades" "button"
+    And I set the field "Map from" to "Email address"
+    And I set the field "Map to" to "Email address"
+    And I set the field "Manual item 1" to "Manual item 1"
+    And I click on "Upload grades" "button"
+    And I should see "One of the grade values is smaller than the allowed grade minimum of 10.0"
+    And I should see "Import failed. No data was imported."
+    And I click on "Continue" "button"
+    When I upload "grade/tests/fixtures/grade_import.csv" file to "File" filemanager
+    And I click on "Upload grades" "button"
+    And I set the field "Map from" to "Email address"
+    And I set the field "Map to" to "Email address"
+    And I set the field "Manual item 1" to "Manual item 1"
+    And I click on "Upload grades" "button"
+    And I should see "Grade import success"
+    And I click on "Continue" "button"
+    Then the following should exist in the "user-grades" table:
+      | -1-                | -1-                  | -4-       | -5-       |
+      | Student 1          | student1@example.com | 400.00    | 400.00    |
+      | Student 2          | student2@example.com |  50.00    |  50.00    |
+      | Student 3          | student3@example.com |  50.00    |  50.00    |

grade/tests/fixtures/grade_import.csv

@@ -0,0 +1,2 @@
+First name,Last name,Email address,Manual item 1
+Student,1,student1@example.com,400

grade/tests/fixtures/grade_import_grademax.csv

@@ -0,0 +1,2 @@
+First name,Last name,Email address,Manual item 1
+Student,1,student1@example.com,800

grade/tests/fixtures/grade_import_grademin.csv

@@ -0,0 +1,2 @@
+First name,Last name,Email address,Manual item 1
+Student,1,student1@example.com,5

grade/upgrade.txt

@@ -1,6 +1,10 @@
 This file describes API changes in /grade/* ;
 Information provided here is intended especially for developers.
 
+=== 4.1.7 ===
+* The function gradeimport_csv_load_data::insert_grade_record() now has extra parameter $gradeitem to carry over the grade item related info
+  for validation.
+
 === 4.1.6 ===
 * The grade `itemname` property contained in the return structure of the following external methods is now PARAM_CLEANHTML:
   - `gradereport_user_get_grade_items`

lang/en/grades.php

@@ -362,6 +362,7 @@
 
 Only value and scale grade types may be aggregated. The grade type for an activity-based grade item is set on the activity settings page.';
 $string['gradevaluetoobig'] = 'One of the grade values is larger than the allowed grade maximum of {$a}';
+$string['gradevaluetoosmall'] = 'One of the grade values is smaller than the allowed grade minimum of {$a}';
 $string['gradeview'] = 'View grade';
 $string['gradewasmodifiedduringediting'] = 'The grade entered for {$a->itemname} for {$a->username} was ignored because it was more recently updated by someone else.';
 $string['gradeweighthelp'] = 'Grade weight help';