diff --git a/mod/assign/gradingtable.php b/mod/assign/gradingtable.php index 90f5aa78691..daef667d855 100644 --- a/mod/assign/gradingtable.php +++ b/mod/assign/gradingtable.php @@ -759,6 +759,9 @@ class assign_grading_table extends table_sql implements renderable { $selectcol .= ''; + $selectcol .= ''; return $selectcol; } diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 310ea852425..85947f703b8 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -5332,6 +5332,7 @@ class assign { // Gets a list of possible users and look for values based upon that. foreach ($participants as $userid => $unused) { $modified = optional_param('grademodified_' . $userid, -1, PARAM_INT); + $attemptnumber = optional_param('gradeattempt_' . $userid, -1, PARAM_INT); // Gather the userid, updated grade and last modified value. $record = new stdClass(); $record->userid = $userid; @@ -5343,6 +5344,7 @@ class assign { // This user was not in the grading table. continue; } + $record->attemptnumber = $attemptnumber; $record->lastmodified = $modified; $record->gradinginfo = grade_get_grades($this->get_course()->id, 'mod', @@ -5361,19 +5363,20 @@ class assign { $params['assignid2'] = $this->get_instance()->id; // Check them all for currency. - $grademaxattempt = 'SELECT mxg.userid, MAX(mxg.attemptnumber) AS maxattempt - FROM {assign_grades} mxg - WHERE mxg.assignment = :assignid1 GROUP BY mxg.userid'; + $grademaxattempt = 'SELECT s.userid, s.attemptnumber AS maxattempt + FROM {assign_submission} s + WHERE s.assignment = :assignid1 AND s.latest = 1'; - $sql = 'SELECT u.id as userid, g.grade as grade, g.timemodified as lastmodified, uf.workflowstate, uf.allocatedmarker - FROM {user} u - LEFT JOIN ( ' . $grademaxattempt . ' ) gmx ON u.id = gmx.userid - LEFT JOIN {assign_grades} g ON - u.id = g.userid AND - g.assignment = :assignid2 AND - g.attemptnumber = gmx.maxattempt - LEFT JOIN {assign_user_flags} uf ON uf.assignment = g.assignment AND uf.userid = g.userid - WHERE u.id ' . $userids; + $sql = 'SELECT u.id AS userid, g.grade AS grade, g.timemodified AS lastmodified, + uf.workflowstate, uf.allocatedmarker, gmx.maxattempt AS attemptnumber + FROM {user} u + LEFT JOIN ( ' . $grademaxattempt . ' ) gmx ON u.id = gmx.userid + LEFT JOIN {assign_grades} g ON + u.id = g.userid AND + g.assignment = :assignid2 AND + g.attemptnumber = gmx.maxattempt + LEFT JOIN {assign_user_flags} uf ON uf.assignment = g.assignment AND uf.userid = g.userid + WHERE u.id ' . $userids; $currentgrades = $DB->get_recordset_sql($sql, $params); $modifiedusers = array(); @@ -5437,7 +5440,9 @@ class assign { if ($this->grading_disabled($modified->userid)) { continue; } - if ((int)$current->lastmodified > (int)$modified->lastmodified) { + $badmodified = (int)$current->lastmodified > (int)$modified->lastmodified; + $badattempt = (int)$current->attemptnumber != (int)$modified->attemptnumber; + if ($badmodified || $badattempt) { // Error - record has been modified since viewing the page. return get_string('errorrecordmodified', 'assign'); } else { diff --git a/mod/assign/tests/events_test.php b/mod/assign/tests/events_test.php index 14d68ecd6e0..787350b941c 100644 --- a/mod/assign/tests/events_test.php +++ b/mod/assign/tests/events_test.php @@ -436,6 +436,7 @@ class assign_events_testcase extends mod_assign_base_testcase { $data = array( 'grademodified_' . $this->students[0]->id => time(), + 'gradeattempt_' . $this->students[0]->id => '', 'quickgrade_' . $this->students[0]->id => '60.0', 'quickgrade_' . $this->students[0]->id . '_workflowstate' => 'inmarking' ); @@ -565,8 +566,10 @@ class assign_events_testcase extends mod_assign_base_testcase { // Test process_save_quick_grades. $sink = $this->redirectEvents(); + $grade = $assign->get_user_grade($this->students[0]->id, false); $data = array( 'grademodified_' . $this->students[0]->id => time(), + 'gradeattempt_' . $this->students[0]->id => $grade->attemptnumber, 'quickgrade_' . $this->students[0]->id => '60.0' ); $assign->testable_process_save_quick_grades($data); diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php index ad92370e3b5..75a03c71603 100644 --- a/mod/assign/tests/locallib_test.php +++ b/mod/assign/tests/locallib_test.php @@ -2329,5 +2329,75 @@ Anchor link 2:Link text $this->assertTrue(in_array($this->extrastudents[0]->id, $allgroupmembers)); $this->assertTrue(in_array($this->extrastudents[1]->id , $allgroupmembers)); } -} + /** + * Test the quicksave grades processor + */ + public function test_process_save_quick_grades() { + $this->editingteachers[0]->ignoresesskey = true; + $this->setUser($this->editingteachers[0]); + + $assign = $this->create_instance(array('attemptreopenmethod' => ASSIGN_ATTEMPT_REOPEN_METHOD_MANUAL)); + + // Initially grade the user. + $grade = $assign->get_user_grade($this->students[0]->id, false); + if (!$grade) { + $grade = new stdClass(); + $grade->attemptnumber = ''; + $grade->timemodified = ''; + } + $data = array( + 'grademodified_' . $this->students[0]->id => $grade->timemodified, + 'gradeattempt_' . $this->students[0]->id => $grade->attemptnumber, + 'quickgrade_' . $this->students[0]->id => '60.0' + ); + $result = $assign->testable_process_save_quick_grades($data); + $this->assertContains(get_string('quickgradingchangessaved', 'assign'), $result); + $grade = $assign->get_user_grade($this->students[0]->id, false); + $this->assertEquals('60.0', $grade->grade); + + // Attempt to grade with a past attempts grade info. + $assign->testable_process_add_attempt($this->students[0]->id); + $data = array( + 'grademodified_' . $this->students[0]->id => $grade->timemodified, + 'gradeattempt_' . $this->students[0]->id => $grade->attemptnumber, + 'quickgrade_' . $this->students[0]->id => '50.0' + ); + $result = $assign->testable_process_save_quick_grades($data); + $this->assertContains(get_string('errorrecordmodified', 'assign'), $result); + $grade = $assign->get_user_grade($this->students[0]->id, false); + $this->assertFalse($grade); + + // Attempt to grade a the attempt. + $submission = $assign->get_user_submission($this->students[0]->id, false); + $data = array( + 'grademodified_' . $this->students[0]->id => '', + 'gradeattempt_' . $this->students[0]->id => $submission->attemptnumber, + 'quickgrade_' . $this->students[0]->id => '40.0' + ); + $result = $assign->testable_process_save_quick_grades($data); + $this->assertContains(get_string('quickgradingchangessaved', 'assign'), $result); + $grade = $assign->get_user_grade($this->students[0]->id, false); + $this->assertEquals('40.0', $grade->grade); + + // Catch grade update conflicts. + // Save old data for later. + $pastdata = $data; + // Update the grade the 'good' way. + $data = array( + 'grademodified_' . $this->students[0]->id => $grade->timemodified, + 'gradeattempt_' . $this->students[0]->id => $grade->attemptnumber, + 'quickgrade_' . $this->students[0]->id => '30.0' + ); + $result = $assign->testable_process_save_quick_grades($data); + $this->assertContains(get_string('quickgradingchangessaved', 'assign'), $result); + $grade = $assign->get_user_grade($this->students[0]->id, false); + $this->assertEquals('30.0', $grade->grade); + + // Now update using 'old' data. Should fail. + $result = $assign->testable_process_save_quick_grades($pastdata); + $this->assertContains(get_string('errorrecordmodified', 'assign'), $result); + $grade = $assign->get_user_grade($this->students[0]->id, false); + $this->assertEquals('30.0', $grade->grade); + } +}