MDL-46171 Assign: Always require a submission record if there is a grade
Also - update unit tests to match the new expectations.
This commit is contained in:
@@ -237,7 +237,10 @@ class restore_assign_activity_structure_step extends restore_activity_structure_
|
||||
* @return void
|
||||
*/
|
||||
protected function set_latest_submission_field() {
|
||||
global $DB;
|
||||
global $DB, $CFG;
|
||||
|
||||
// Required for constants.
|
||||
require_once($CFG->dirroot . '/mod/assign/locallib.php');
|
||||
|
||||
$assignmentid = $this->get_new_parentid('assign');
|
||||
// This code could be rewritten as a monster SQL - but the point of adding this "latest" field
|
||||
@@ -277,6 +280,29 @@ class restore_assign_activity_structure_step extends restore_activity_structure_
|
||||
$DB->update_record('assign_submission', $submission);
|
||||
}
|
||||
}
|
||||
|
||||
// Now check for records with a grade, but no submission record.
|
||||
$records = $DB->get_records_sql('SELECT g.id, g.userid
|
||||
FROM {assign_grades} g
|
||||
LEFT JOIN {assign_submission} s
|
||||
ON s.assignment = g.assignment
|
||||
AND s.userid = g.userid
|
||||
WHERE s.id IS NULL AND g.assignment = ?', array($assignmentid));
|
||||
|
||||
$submissions = array();
|
||||
foreach ($records as $record) {
|
||||
$submission = new stdClass();
|
||||
$submission->assignment = $assignmentid;
|
||||
$submission->userid = $record->userid;
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_NEW;
|
||||
$submission->groupid = 0;
|
||||
$submission->latest = 1;
|
||||
$submission->timecreated = time();
|
||||
$submission->timemodified = time();
|
||||
array_push($submissions, $submission);
|
||||
}
|
||||
|
||||
$DB->insert_records('assign_submission', $submissions);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<?xml version="1.0" encoding="UTF-8" ?>
|
||||
<XMLDB PATH="mod/assign/db" VERSION="20131220" COMMENT="XMLDB file for Moodle mod/assign"
|
||||
<XMLDB PATH="mod/assign/db" VERSION="20140724" COMMENT="XMLDB file for Moodle mod/assign"
|
||||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
|
||||
xsi:noNamespaceSchemaLocation="../../../lib/xmldb/xmldb.xsd"
|
||||
>
|
||||
@@ -52,6 +52,7 @@
|
||||
<FIELD NAME="status" TYPE="char" LENGTH="10" NOTNULL="false" SEQUENCE="false" COMMENT="The status of this assignment submission. The current statuses are DRAFT and SUBMITTED."/>
|
||||
<FIELD NAME="groupid" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="The group id for team submissions"/>
|
||||
<FIELD NAME="attemptnumber" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Used to track attempts for an assignment"/>
|
||||
<FIELD NAME="latest" TYPE="int" LENGTH="2" NOTNULL="true" DEFAULT="0" SEQUENCE="false" COMMENT="Greatly simplifies queries wanting to know information about only the latest attempt."/>
|
||||
</FIELDS>
|
||||
<KEYS>
|
||||
<KEY NAME="primary" TYPE="primary" FIELDS="id" COMMENT="The unique id for this assignment submission."/>
|
||||
@@ -61,6 +62,7 @@
|
||||
<INDEX NAME="userid" UNIQUE="false" FIELDS="userid"/>
|
||||
<INDEX NAME="attemptnumber" UNIQUE="false" FIELDS="attemptnumber"/>
|
||||
<INDEX NAME="uniqueattemptsubmission" UNIQUE="true" FIELDS="assignment, userid, groupid, attemptnumber"/>
|
||||
<INDEX NAME="latestattempt" UNIQUE="false" FIELDS="assignment, userid, groupid, latest" COMMENT="Speed up queries for the latest attempt."/>
|
||||
</INDEXES>
|
||||
</TABLE>
|
||||
<TABLE NAME="assign_grades" COMMENT="Grading information about a single assignment submission.">
|
||||
|
||||
@@ -523,14 +523,14 @@ function xmldb_assign_upgrade($oldversion) {
|
||||
// Assign savepoint reached.
|
||||
upgrade_mod_savepoint(true, 2014072401, 'assign');
|
||||
}
|
||||
if ($oldversion < 2014072403) {
|
||||
if ($oldversion < 2014072405) {
|
||||
|
||||
// Prevent running this multiple times.
|
||||
|
||||
$countsql = 'SELECT COUNT(id) FROM {assign_submission} WHERE latest = ?;';
|
||||
|
||||
$count = $DB->count_records_sql($countsql, array(1));
|
||||
if ($count == 0) {
|
||||
if ($count != 342234) {
|
||||
|
||||
// Mark the latest attempt for every submission in mod_assign.
|
||||
$maxattemptsql = 'SELECT assignment, userid, groupid, max(attemptnumber) AS maxattempt
|
||||
@@ -547,10 +547,31 @@ function xmldb_assign_upgrade($oldversion) {
|
||||
$select = 'id IN(' . $maxattemptidssql . ')';
|
||||
$DB->set_field_select('assign_submission', 'latest', 1, $select);
|
||||
|
||||
// Look for grade records with no submission record.
|
||||
$records = $DB->get_records_sql('SELECT g.id, g.assignment, g.userid
|
||||
FROM {assign_grades} g
|
||||
LEFT JOIN {assign_submission} s
|
||||
ON s.assignment = g.assignment
|
||||
AND s.userid = g.userid
|
||||
WHERE s.id IS NULL');
|
||||
$submissions = array();
|
||||
foreach ($records as $record) {
|
||||
$submission = new stdClass();
|
||||
$submission->assignment = $record->assignment;
|
||||
$submission->userid = $record->userid;
|
||||
$submission->status = 'new';
|
||||
$submission->groupid = 0;
|
||||
$submission->latest = 1;
|
||||
$submission->timecreated = time();
|
||||
$submission->timemodified = time();
|
||||
array_push($submissions, $submission);
|
||||
}
|
||||
|
||||
$DB->insert_records('assign_submission', $submissions);
|
||||
}
|
||||
|
||||
// Assign savepoint reached.
|
||||
upgrade_mod_savepoint(true, 2014072403, 'assign');
|
||||
upgrade_mod_savepoint(true, 2014072405, 'assign');
|
||||
}
|
||||
|
||||
return true;
|
||||
|
||||
@@ -138,13 +138,13 @@ class mod_assign_external extends external_api {
|
||||
ag.grader,
|
||||
ag.grade,
|
||||
ag.attemptnumber
|
||||
FROM {assign_grades} ag, {assign_submission} as
|
||||
WHERE as.assignment $inorequalsql
|
||||
AND as.userid = ag.userid
|
||||
AND as.latest = 1
|
||||
AND as.attemptnumber = ag.attemptnumber
|
||||
FROM {assign_grades} ag, {assign_submission} s
|
||||
WHERE s.assignment $inorequalsql
|
||||
AND s.userid = ag.userid
|
||||
AND s.latest = 1
|
||||
AND s.attemptnumber = ag.attemptnumber
|
||||
AND ag.timemodified >= :since
|
||||
AND ag.assignment = as.assignment
|
||||
AND ag.assignment = s.assignment
|
||||
ORDER BY ag.assignment, ag.id";
|
||||
|
||||
$placeholders['since'] = $params['since'];
|
||||
|
||||
+6
-7
@@ -390,7 +390,7 @@ function assign_print_overview($courses, &$htmlarray) {
|
||||
if (!isset($unmarkedsubmissions)) {
|
||||
// Build up and array of unmarked submissions indexed by assignment id/ userid
|
||||
// for use where the user has grading rights on assignment.
|
||||
$dbparams = array_merge($assignmentidparams, array(ASSIGN_SUBMISSION_STATUS_SUBMITTED), $assignmentidparams);
|
||||
$dbparams = array_merge(array(ASSIGN_SUBMISSION_STATUS_SUBMITTED), $assignmentidparams);
|
||||
$rs = $DB->get_recordset_sql('SELECT
|
||||
s.assignment as assignment,
|
||||
s.userid as userid,
|
||||
@@ -440,7 +440,7 @@ function assign_print_overview($courses, &$htmlarray) {
|
||||
if (!isset($mysubmissions)) {
|
||||
|
||||
// Get all user submissions, indexed by assignment id.
|
||||
$dbparams = array_merge(array($USER->id, $USER->id), $assignmentidparams);
|
||||
$dbparams = array_merge(array($USER->id), $assignmentidparams, array($USER->id));
|
||||
$mysubmissions = $DB->get_records_sql('SELECT
|
||||
a.id AS assignment,
|
||||
a.nosubmissions AS nosubmissions,
|
||||
@@ -448,16 +448,15 @@ function assign_print_overview($courses, &$htmlarray) {
|
||||
g.grader AS grader,
|
||||
g.grade AS grade,
|
||||
s.status AS status
|
||||
FROM {assign} a
|
||||
FROM {assign} a, {assign_submission} s
|
||||
LEFT JOIN {assign_grades} g ON
|
||||
g.assignment = a.id AND
|
||||
g.assignment = s.assignment AND
|
||||
g.userid = ? AND
|
||||
g.attemptnumber = s.attemptnumber
|
||||
LEFT JOIN {assign_submission} s ON
|
||||
WHERE a.id ' . $sqlassignmentids . ' AND
|
||||
s.latest = 1 AND
|
||||
s.assignment = a.id AND
|
||||
s.userid = ?
|
||||
WHERE a.id ' . $sqlassignmentids, $dbparams);
|
||||
s.userid = ?', $dbparams);
|
||||
}
|
||||
|
||||
$str .= '<div class="details">';
|
||||
|
||||
+9
-10
@@ -27,6 +27,7 @@
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
// Assignment submission statuses.
|
||||
define('ASSIGN_SUBMISSION_STATUS_NEW', 'new');
|
||||
define('ASSIGN_SUBMISSION_STATUS_REOPENED', 'reopened');
|
||||
define('ASSIGN_SUBMISSION_STATUS_DRAFT', 'draft');
|
||||
define('ASSIGN_SUBMISSION_STATUS_SUBMITTED', 'submitted');
|
||||
@@ -1562,10 +1563,6 @@ class assign {
|
||||
s.status = :submissionstatus';
|
||||
$params['groupuserid'] = 0;
|
||||
} else {
|
||||
$maxattemptsql = 'SELECT mxs.userid, MAX(mxs.attemptnumber) AS maxattempt
|
||||
FROM {assign_submission} mxs
|
||||
WHERE mxs.assignment = :assignid2 GROUP BY mxs.userid';
|
||||
|
||||
$sql = 'SELECT COUNT(s.userid)
|
||||
FROM {assign_submission} s
|
||||
JOIN(' . $esql . ') e ON e.id = s.userid
|
||||
@@ -2024,6 +2021,7 @@ class assign {
|
||||
* @param int $groupid The id of the group for this user - may be 0 in which
|
||||
* case it is determined from the userid.
|
||||
* @param bool $create If set to true a new submission object will be created in the database
|
||||
* with the status set to "new".
|
||||
* @param int $attemptnumber - -1 means the latest attempt
|
||||
* @return stdClass The submission
|
||||
*/
|
||||
@@ -2085,7 +2083,7 @@ class assign {
|
||||
// This is the case when we need to set latest to 0 for all the other attempts.
|
||||
$DB->set_field('assign_submission', 'latest', 0, $params);
|
||||
}
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_DRAFT;
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_NEW;
|
||||
$sid = $DB->insert_record('assign_submission', $submission);
|
||||
return $DB->get_record('assign_submission', array('id' => $sid));
|
||||
}
|
||||
@@ -2654,7 +2652,7 @@ class assign {
|
||||
*
|
||||
* @param int $userid The id of the user whose submission we want or 0 in which case USER->id is used
|
||||
* @param bool $create optional - defaults to false. If set to true a new submission object
|
||||
* will be created in the database.
|
||||
* will be created in the database with the status set to "new".
|
||||
* @param int $attemptnumber - -1 means the latest attempt
|
||||
* @return stdClass The submission
|
||||
*/
|
||||
@@ -2686,7 +2684,7 @@ class assign {
|
||||
$submission->userid = $userid;
|
||||
$submission->timecreated = time();
|
||||
$submission->timemodified = $submission->timecreated;
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_DRAFT;
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_NEW;
|
||||
if ($attemptnumber >= 0) {
|
||||
$submission->attemptnumber = $attemptnumber;
|
||||
} else {
|
||||
@@ -2701,10 +2699,11 @@ class assign {
|
||||
} else {
|
||||
// We need to work this out.
|
||||
$result = $DB->get_records('assign_submission', $params, 'attemptnumber DESC', 'attemptnumber', 0, 1);
|
||||
$latestsubmission = null;
|
||||
if ($result) {
|
||||
$latestsubmission = reset($result);
|
||||
}
|
||||
if (!$latestsubmission || ($attemptnumber == $latestsubmission->attemptnumber)) {
|
||||
if (empty($latestsubmission) || ($attemptnumber > $latestsubmission->attemptnumber)) {
|
||||
$submission->latest = 1;
|
||||
}
|
||||
}
|
||||
@@ -2794,9 +2793,9 @@ class assign {
|
||||
if ($attemptnumber < 0) {
|
||||
// Make sure this grade matches the latest submission attempt.
|
||||
if ($this->get_instance()->teamsubmission) {
|
||||
$submission = $this->get_group_submission($userid, 0, false);
|
||||
$submission = $this->get_group_submission($userid, 0, true);
|
||||
} else {
|
||||
$submission = $this->get_user_submission($userid, false);
|
||||
$submission = $this->get_user_submission($userid, true);
|
||||
}
|
||||
if ($submission) {
|
||||
$attemptnumber = $submission->attemptnumber;
|
||||
|
||||
@@ -483,7 +483,7 @@ class mod_assign_renderer extends plugin_renderer_base {
|
||||
$row = new html_table_row();
|
||||
$cell1 = new html_table_cell(get_string('submissionstatus', 'assign'));
|
||||
if (!$status->teamsubmissionenabled) {
|
||||
if ($status->submission) {
|
||||
if ($status->submission && $status->submission->status != ASSIGN_SUBMISSION_STATUS_NEW) {
|
||||
$statusstr = get_string('submissionstatus_' . $status->submission->status, 'assign');
|
||||
$cell2 = new html_table_cell($statusstr);
|
||||
$cell2->attributes = array('class'=>'submissionstatus' . $status->submission->status);
|
||||
@@ -499,7 +499,7 @@ class mod_assign_renderer extends plugin_renderer_base {
|
||||
} else {
|
||||
$row = new html_table_row();
|
||||
$cell1 = new html_table_cell(get_string('submissionstatus', 'assign'));
|
||||
if ($status->teamsubmission) {
|
||||
if ($status->teamsubmission && $status->teamsubmission->status != ASSIGN_SUBMISSION_STATUS_NEW) {
|
||||
$teamstatus = $status->teamsubmission->status;
|
||||
$submissionsummary = get_string('submissionstatus_' . $teamstatus, 'assign');
|
||||
$groupid = 0;
|
||||
@@ -690,7 +690,7 @@ class mod_assign_renderer extends plugin_renderer_base {
|
||||
// Links.
|
||||
if ($status->view == assign_submission_status::STUDENT_VIEW) {
|
||||
if ($status->canedit) {
|
||||
if (!$submission) {
|
||||
if (!$submission || $submission->status == ASSIGN_SUBMISSION_STATUS_NEW) {
|
||||
$o .= $this->output->box_start('generalbox submissionaction');
|
||||
$urlparams = array('id' => $status->coursemoduleid, 'action' => 'editsubmission');
|
||||
$o .= $this->output->single_button(new moodle_url('/mod/assign/view.php', $urlparams),
|
||||
|
||||
@@ -77,6 +77,18 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
|
||||
|
||||
// Create a student and give them 2 grades (for 2 attempts).
|
||||
$student = self::getDataGenerator()->create_user();
|
||||
|
||||
$submission = new stdClass();
|
||||
$submission->assignment = $assign->id;
|
||||
$submission->userid = $student->id;
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_NEW;
|
||||
$submission->latest = 0;
|
||||
$submission->attemptnumber = 0;
|
||||
$submission->groupid = 0;
|
||||
$submission->timecreated = time();
|
||||
$submission->timemodified = time();
|
||||
$DB->insert_record('assign_submission', $submission);
|
||||
|
||||
$grade = new stdClass();
|
||||
$grade->assignment = $assign->id;
|
||||
$grade->userid = $student->id;
|
||||
@@ -87,6 +99,17 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
|
||||
$grade->attemptnumber = 0;
|
||||
$DB->insert_record('assign_grades', $grade);
|
||||
|
||||
$submission = new stdClass();
|
||||
$submission->assignment = $assign->id;
|
||||
$submission->userid = $student->id;
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_NEW;
|
||||
$submission->latest = 1;
|
||||
$submission->attemptnumber = 1;
|
||||
$submission->groupid = 0;
|
||||
$submission->timecreated = time();
|
||||
$submission->timemodified = time();
|
||||
$DB->insert_record('assign_submission', $submission);
|
||||
|
||||
$grade = new stdClass();
|
||||
$grade->assignment = $assign->id;
|
||||
$grade->userid = $student->id;
|
||||
@@ -962,6 +985,8 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
|
||||
// No warnings.
|
||||
$this->assertEquals(0, count($result));
|
||||
|
||||
$records = $DB->get_records('assign_submission');
|
||||
$records = $DB->get_records('assign_grades');
|
||||
$result = mod_assign_external::get_grades(array($instance->id));
|
||||
|
||||
$this->assertEquals($result['assignments'][0]['grades'][0]['grade'], '50.0');
|
||||
|
||||
@@ -169,13 +169,15 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase {
|
||||
}
|
||||
|
||||
public function test_assign_user_complete() {
|
||||
global $PAGE;
|
||||
global $PAGE, $DB;
|
||||
|
||||
$this->setUser($this->editingteachers[0]);
|
||||
$assign = $this->create_instance(array('submissiondrafts' => 1));
|
||||
$PAGE->set_url(new moodle_url('/mod/assign/view.php', array('id' => $assign->get_course_module()->id)));
|
||||
|
||||
$submission = $assign->get_user_submission($this->students[0]->id, true);
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_DRAFT;
|
||||
$DB->update_record('assign_submission', $submission);
|
||||
|
||||
$this->expectOutputRegex('/Draft/');
|
||||
assign_user_complete($this->course, $this->students[0], $assign->get_course_module(), $assign->get_instance());
|
||||
|
||||
@@ -661,6 +661,8 @@ class mod_assign_locallib_testcase extends mod_assign_base_testcase {
|
||||
// Simulate a submission.
|
||||
$this->setUser($this->extrastudents[0]);
|
||||
$submission = $assign->get_user_submission($this->extrastudents[0]->id, true);
|
||||
$submission->status = ASSIGN_SUBMISSION_STATUS_DRAFT;
|
||||
$assign->testable_update_submission($submission, $this->extrastudents[0]->id, true, false);
|
||||
// Leave this one as DRAFT.
|
||||
$data = new stdClass();
|
||||
$data->onlinetext_editor = array('itemid'=>file_get_unused_draft_itemid(),
|
||||
@@ -1373,12 +1375,22 @@ class mod_assign_locallib_testcase extends mod_assign_base_testcase {
|
||||
$formdata->instance = $formdata->id;
|
||||
$assign->update_instance($formdata);
|
||||
|
||||
// Check we can repopen still.
|
||||
// Mark the submission again.
|
||||
$data = new stdClass();
|
||||
$data->grade = '60.0';
|
||||
$assign->testable_apply_grade_to_user($data, $this->students[0]->id, 1);
|
||||
|
||||
// Check the grade exists.
|
||||
$grades = $assign->get_user_grades_for_gradebook($this->students[0]->id);
|
||||
$this->assertEquals(60, (int)$grades[$this->students[0]->id]->rawgrade);
|
||||
|
||||
// Check we can reopen still.
|
||||
$result = $assign->testable_process_add_attempt($this->students[0]->id);
|
||||
$this->assertEquals(true, $result);
|
||||
|
||||
// Should no longer have a grade because there is no grade for the latest attempt.
|
||||
$grades = $assign->get_user_grades_for_gradebook($this->students[0]->id);
|
||||
$this->assertEquals(50, (int)$grades[$this->students[0]->id]->rawgrade);
|
||||
$this->assertEmpty($grades);
|
||||
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,12 @@
|
||||
This files describes API changes in the assign code.
|
||||
=== 2.8 ===
|
||||
* Some DB changes were made to simplify the SQL required to query the latest attempt.
|
||||
- The assign_submission table now has a column "latest" which is set to 1 for the latest submission attempt.
|
||||
- There will always be a submission row if there is a grade (so the latest grade can be found by joining with the submission)
|
||||
- There is a new submission status "new" for a submission that has never been attempted by a student (but the record exists purely
|
||||
to mark the latest attempt number as 0). The function get_user_submission will create a record with the status set to "new"
|
||||
by default (the previous default was "draft").
|
||||
|
||||
=== 2.7 ===
|
||||
|
||||
* Added setting sendstudentnotifications to assign DB table with admin defaults. This sets the default value for the
|
||||
|
||||
@@ -25,7 +25,7 @@
|
||||
defined('MOODLE_INTERNAL') || die();
|
||||
|
||||
$plugin->component = 'mod_assign'; // Full name of the plugin (used for diagnostics).
|
||||
$plugin->version = 2014072403; // The current module version (Date: YYYYMMDDXX).
|
||||
$plugin->version = 2014072405; // The current module version (Date: YYYYMMDDXX).
|
||||
$plugin->requires = 2014050800; // Requires this Moodle version.
|
||||
$plugin->cron = 60;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user