MDL-48778 assign: Use proper latest attempt for quickgrading prechecks
The code that stored lastmodified in gradingtable used a different method to produce it than the processor used to generate the check value. Fix that and also check that the attempt number has not changed.
This commit is contained in:
committed by
Simey Lameze
parent
80b09028d1
commit
6ca893ecb2
@@ -759,6 +759,9 @@ class assign_grading_table extends table_sql implements renderable {
|
||||
$selectcol .= '<input type="hidden"
|
||||
name="grademodified_' . $row->userid . '"
|
||||
value="' . $row->timemarked . '"/>';
|
||||
$selectcol .= '<input type="hidden"
|
||||
name="gradeattempt_' . $row->userid . '"
|
||||
value="' . $row->attemptnumber . '"/>';
|
||||
return $selectcol;
|
||||
}
|
||||
|
||||
|
||||
+18
-13
@@ -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 {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -2329,5 +2329,75 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
|
||||
$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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user