Merge branch 'MDL-43805-25' of git://github.com/damyon/moodle into MO…

…ODLE_25_STABLE

mod/assign/feedback/comments/locallib.php

@@ -94,7 +94,10 @@ public function is_quickgrading_modified($userid, $grade) {
                 $commenttext = $feedbackcomments->commenttext;
             }
         }
-        return optional_param('quickgrade_comments_' . $userid, '', PARAM_TEXT) != $commenttext;
+        // Note that this handles the difference between empty and not in the quickgrading
+        // form at all (hidden column).
+        $newvalue = optional_param('quickgrade_comments_' . $userid, false, PARAM_TEXT);
+        return ($newvalue !== false) && ($newvalue != $commenttext);
     }
 
 
@@ -173,6 +176,11 @@ public function set_editor_text($name, $value, $gradeid) {
     public function save_quickgrading_changes($userid, $grade) {
         global $DB;
         $feedbackcomment = $this->get_feedback_comments($grade->id);
+        $feedbackpresent = optional_param('quickgrade_comments_' . $userid, false, PARAM_TEXT) !== false;
+        if (!$feedbackpresent) {
+            // Nothing to save (e.g. hidden column).
+            return true;
+        }
         if ($feedbackcomment) {
             $feedbackcomment->commenttext = optional_param('quickgrade_comments_' . $userid, '', PARAM_TEXT);
             return $DB->update_record('assignfeedback_comments', $feedbackcomment);

mod/assign/gradingtable.php

@@ -605,6 +605,9 @@ public function col_select(stdClass $row) {
                               id="selectuser_' . $row->userid . '"
                               name="selectedusers"
                               value="' . $row->userid . '"/>';
+        $selectcol .= '<input type="hidden"
+                              name="grademodified_' . $row->userid . '"
+                              value="' . $row->timemarked . '"/>';
         return $selectcol;
     }
 

mod/assign/locallib.php

@@ -1181,12 +1181,8 @@ public function display_grade($grade, $editing, $userid=0, $modified=0) {
                               maxlength="10"
                               class="quickgrade"/>';
                 $o .= ' / ' . format_float($this->get_instance()->grade, 2);
-                $o .= '<input type="hidden"
-                              name="grademodified_' . $userid . '"
-                              value="' . $modified . '"/>';
                 return $o;
             } else {
-                $o .= '<input type="hidden" name="grademodified_' . $userid . '" value="' . $modified . '"/>';
                 if ($grade == -1 || $grade === null) {
                     $o .= '-';
                 } else {
@@ -1225,9 +1221,6 @@ class="quickgrade"/>';
                     $o .= '<option value="' . $optionid . '" ' . $selected . '>' . $option . '</option>';
                 }
                 $o .= '</select>';
-                $o .= '<input type="hidden" ' .
-                             'name="grademodified_' . $userid . '" ' .
-                             'value="' . $modified . '"/>';
                 return $o;
             } else {
                 $scaleid = (int)$grade;
@@ -4489,14 +4482,18 @@ protected function process_save_quick_grades() {
         foreach ($currentgrades as $current) {
             $modified = $users[(int)$current->userid];
             $grade = $this->get_user_grade($modified->userid, false);
+            // Check to see if the grade column was even visible.
+            $gradecolpresent = optional_param('quickgrade_' . $modified->userid, false, PARAM_INT) !== false;
 
             // Check to see if the outcomes were modified.
             if ($CFG->enableoutcomes) {
                 foreach ($modified->gradinginfo->outcomes as $outcomeid => $outcome) {
                     $oldoutcome = $outcome->grades[$modified->userid]->grade;
                     $paramname = 'outcome_' . $outcomeid . '_' . $modified->userid;
                     $newoutcome = optional_param($paramname, -1, PARAM_FLOAT);
-                    if ($oldoutcome != $newoutcome) {
+                    // Check to see if the outcome column was even visible.
+                    $outcomecolpresent = optional_param($paramname, false, PARAM_FLOAT) !== false;
+                    if ($outcomecolpresent && ($oldoutcome != $newoutcome)) {
                         // Can't check modified time for outcomes because it is not reported.
                         $modifiedusers[$modified->userid] = $modified;
                         continue;
@@ -4507,6 +4504,8 @@ protected function process_save_quick_grades() {
             // Let plugins participate.
             foreach ($this->feedbackplugins as $plugin) {
                 if ($plugin->is_visible() && $plugin->is_enabled() && $plugin->supports_quickgrading()) {
+                    // The plugins must handle is_quickgrading_modified correctly - ie
+                    // handle hidden columns.
                     if ($plugin->is_quickgrading_modified($modified->userid, $grade)) {
                         if ((int)$current->lastmodified > (int)$modified->lastmodified) {
                             return get_string('errorrecordmodified', 'assign');
@@ -4527,7 +4526,8 @@ protected function process_save_quick_grades() {
             if ($current->grade !== null) {
                 $current->grade = floatval($current->grade);
             }
-            if ($current->grade !== $modified->grade) {
+            $gradechanged = $gradecolpresent && $current->grade !== $modified->grade;
+            if ($gradechanged) {
                 // Grade changed.
                 if ($this->grading_disabled($modified->userid)) {
                     continue;
@@ -4551,6 +4551,7 @@ protected function process_save_quick_grades() {
             $grade = $this->get_user_grade($userid, true);
             $grade->grade= grade_floatval(unformat_float($modified->grade));
             $grade->grader= $USER->id;
+            $gradecolpresent = optional_param('quickgrade_' . $userid, false, PARAM_INT) !== false;
 
             // Save plugins data.
             foreach ($this->feedbackplugins as $plugin) {
@@ -4573,8 +4574,10 @@ protected function process_save_quick_grades() {
                 foreach ($modified->gradinginfo->outcomes as $outcomeid => $outcome) {
                     $oldoutcome = $outcome->grades[$modified->userid]->grade;
                     $paramname = 'outcome_' . $outcomeid . '_' . $modified->userid;
-                    $newoutcome = optional_param($paramname, -1, PARAM_INT);
-                    if ($oldoutcome != $newoutcome) {
+                    // This will be false if the input was not in the quickgrading
+                    // form (e.g. column hidden).
+                    $newoutcome = optional_param($paramname, false, PARAM_INT);
+                    if ($newoutcome !== false && ($oldoutcome != $newoutcome)) {
                         $data[$outcomeid] = $newoutcome;
                     }
                 }

mod/assign/tests/behat/quickgrading.feature

@@ -0,0 +1,141 @@
+@mod @mod_assign
+Feature: In an assignment, teachers grade multiple students on one page
+  In order to quickly give students grades and feedback
+  As a teacher
+  I need to grade multiple students on one page
+
+  @javascript
+  Scenario: Grade multiple students on one page
+    Given the following "courses" exists:
+      | fullname | shortname | category | groupmode |
+      | Course 1 | C1 | 0 | 1 |
+    And the following "users" exists:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@asd.com |
+      | student1 | Student | 1 | student1@asd.com |
+      | student2 | Student | 2 | student2@asd.com |
+    And the following "course enrolments" exists:
+      | user | course | role |
+      | teacher1 | C1 | editingteacher |
+      | student1 | C1 | student |
+      | student2 | C1 | student |
+    When I log in as "admin"
+    And I set the following administration settings values:
+      | Enable outcomes | 1 |
+    And I log out
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Outcomes"
+    And I follow "Edit outcomes"
+    And I press "Add a new outcome"
+    And I press "Continue"
+    And I fill the moodle form with:
+      | Name | 1337dom scale |
+      | Scale | Noob, Nub, 1337, HaXor |
+    And I press "Save changes"
+    And I follow "Course 1"
+    And I follow "Outcomes"
+    And I follow "Edit outcomes"
+    And I press "Add a new outcome"
+    And I fill the moodle form with:
+      | Full name | M8d skillZ! |
+      | Short name | skillZ! |
+      | Scale | 1337dom scale |
+    And I press "Save changes"
+    And I follow "Course 1"
+    And I turn editing mode on
+    And I add a "Assignment" to section "1" and I fill the form with:
+      | Assignment name | Test assignment name |
+      | Description | Submit your online text |
+      | assignsubmission_onlinetext_enabled | 1 |
+      | assignsubmission_file_enabled | 0 |
+      | M8d skillZ! | 1 |
+    And I log out
+    And I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I press "Add submission"
+    And I fill the moodle form with:
+      | Online text | I'm the student1 submission |
+    And I press "Save changes"
+    And I log out
+    And I log in as "student2"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    When I press "Add submission"
+    And I fill the moodle form with:
+      | Online text | I'm the student2 submission |
+    And I press "Save changes"
+    And I log out
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I follow "View/grade all submissions"
+    And I click on "Grade Student 1" "link" in the "Student 1" "table_row"
+    And I fill the moodle form with:
+      | Grade out of 100 | 50.0 |
+      | M8d skillZ! | 1337 |
+      | Feedback comments | I'm the teacher first feedback |
+    And I press "Save changes"
+    And I press "Continue"
+    Then I click on "Quick grading" "checkbox"
+    And I fill in "User grade" with "60.0"
+    And I press "Save all quick grading changes"
+    And I should see "The grade changes were saved"
+    And I press "Continue"
+    And I log out
+    And I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should see "I'm the teacher first feedback"
+    And I should see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should see "1337"
+    And I log out
+    And I log in as "student2"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should not see "I'm the teacher first feedback"
+    And I should not see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should not see "1337"
+    And I log out
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I follow "View/grade all submissions"
+    And I click on "Hide User picture" "link"
+    And I click on "Hide Full name" "link"
+    And I click on "Hide Email address" "link"
+    And I click on "Hide Status" "link"
+    And I click on "Hide Grade" "link"
+    And I click on "Hide Edit" "link"
+    And I click on "Hide Last modified (submission)" "link"
+    And I click on "Hide Online text" "link"
+    And I click on "Hide Last modified (grade)" "link"
+    And I click on "Hide Feedback comments" "link"
+    And I click on "Hide Final grade" "link"
+    And I click on "Hide Outcomes" "link"
+    And I press "Save all quick grading changes"
+    And I should see "The grade changes were saved"
+    And I press "Continue"
+    And I log out
+    And I log in as "student1"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should see "I'm the teacher first feedback"
+    And I should see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should see "1337"
+    And I log out
+    And I log in as "student2"
+    And I follow "Course 1"
+    And I follow "Test assignment name"
+    And I should not see "I'm the teacher first feedback"
+    And I should not see "60.0"
+    And I follow "Course 1"
+    And I follow "Grades"
+    And I should not see "1337"