From 53fb01aef079b732537eafb4ad9b027260c481c0 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Thu, 19 Jun 2014 11:01:03 +0800 Subject: [PATCH 1/5] MDL-46044 Assign: Update unit test to test multiple attempts on overview page --- mod/assign/tests/lib_test.php | 60 +++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/mod/assign/tests/lib_test.php b/mod/assign/tests/lib_test.php index 290191cd743..2691057673c 100644 --- a/mod/assign/tests/lib_test.php +++ b/mod/assign/tests/lib_test.php @@ -43,10 +43,64 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { global $DB; $this->setUser($this->editingteachers[0]); $this->create_instance(); - $this->create_instance(array('duedate'=>time())); + $assign = $this->create_instance(array('duedate' => time(), + 'attemptreopenmethod' => ASSIGN_ATTEMPT_REOPEN_METHOD_MANUAL, + 'maxattempts' => 3, + 'submissiondrafts' => 1, + 'assignsubmission_onlinetext_enabled' => 1)); $courses = $DB->get_records('course', array('id' => $this->course->id)); + $this->setUser($this->students[0]); + // Add a submission. + $submission = $assign->get_user_submission($this->students[0]->id, true); + $data = new stdClass(); + $data->onlinetext_editor = array('itemid' => file_get_unused_draft_itemid(), + 'text' => 'Submission text', + 'format' => FORMAT_HTML); + $plugin = $assign->get_submission_plugin_by_type('onlinetext'); + $plugin->save($submission, $data); + + // And now submit it for marking. + $submission->status = ASSIGN_SUBMISSION_STATUS_SUBMITTED; + $assign->testable_update_submission($submission, $this->students[0]->id, true, false); + + // Mark the submission. + $this->setUser($this->teachers[0]); + $data = new stdClass(); + $data->grade = '50.0'; + $assign->testable_apply_grade_to_user($data, $this->students[0]->id, 0); + + // This is required so that the submissions timemodified > the grade timemodified. + sleep(2); + + // Edit the submission again. + $this->setUser($this->students[0]); + $submission = $assign->get_user_submission($this->students[0]->id, true); + $assign->testable_update_submission($submission, $this->students[0]->id, true, false); + + // This is required so that the submissions timemodified > the grade timemodified. + sleep(2); + + // Allow the student another attempt. + $this->teachers[0]->ignoresesskey = true; + $this->setUser($this->teachers[0]); + $result = $assign->testable_process_add_attempt($this->students[0]->id); + // Add another submission. + $this->setUser($this->students[0]); + $submission = $assign->get_user_submission($this->students[0]->id, true); + $data = new stdClass(); + $data->onlinetext_editor = array('itemid' => file_get_unused_draft_itemid(), + 'text' => 'Submission text 2', + 'format' => FORMAT_HTML); + $plugin = $assign->get_submission_plugin_by_type('onlinetext'); + $plugin->save($submission, $data); + + // And now submit it for marking (again). + $submission->status = ASSIGN_SUBMISSION_STATUS_SUBMITTED; + $assign->testable_update_submission($submission, $this->students[0]->id, true, false); + + // Check the overview as the different users. $this->setUser($this->students[0]); $overview = array(); assign_print_overview($courses, $overview); @@ -113,7 +167,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { $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))); + $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); @@ -137,7 +191,7 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { public function test_assign_get_completion_state() { global $DB; - $assign = $this->create_instance(array('submissiondrafts'=>0, 'completionsubmit'=>1)); + $assign = $this->create_instance(array('submissiondrafts' => 0, 'completionsubmit' => 1)); $this->setUser($this->students[0]); $result = assign_get_completion_state($this->course, $assign->get_course_module(), $this->students[0]->id, false); From d95453d903f44dabb69f250ee93368c62e44a58e Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Wed, 18 Jun 2014 16:57:48 +0800 Subject: [PATCH 2/5] MDL-46044 Assign: Fix print_overview function when there are multiple attempts --- mod/assign/lib.php | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/mod/assign/lib.php b/mod/assign/lib.php index 6f7962c9411..29eb712cabe 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -390,6 +390,13 @@ function assign_print_overview($courses, &$htmlarray) { $context = context_module::instance($assignment->coursemodule); if (has_capability('mod/assign:grade', $context)) { if (!isset($unmarkedsubmissions)) { + $submissionmaxattempt = 'SELECT mxs.userid, MAX(mxs.attemptnumber) AS maxattempt, mxs.assignment + FROM {assign_submission} mxs + GROUP BY mxs.userid, mxs.assignment'; + $grademaxattempt = 'SELECT mxg.userid, MAX(mxg.attemptnumber) AS maxattempt, mxg.assignment + FROM {assign_grades} mxg + GROUP BY mxg.userid, mxg.assignment'; + // 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(array(ASSIGN_SUBMISSION_STATUS_SUBMITTED), $assignmentidparams); @@ -400,14 +407,22 @@ function assign_print_overview($courses, &$htmlarray) { s.status as status, g.timemodified as timegraded FROM {assign_submission} s + LEFT JOIN ( ' . $submissionmaxattempt . ' ) smx ON + smx.userid = s.userid AND + smx.assignment = s.id + LEFT JOIN ( ' . $grademaxattempt . ' ) gmx ON + gmx.userid = s.userid AND + gmx.assignment = s.id LEFT JOIN {assign_grades} g ON s.userid = g.userid AND - s.assignment = g.assignment + s.assignment = g.assignment AND + g.attemptnumber = gmx.maxattempt WHERE ( g.timemodified is NULL OR s.timemodified > g.timemodified ) AND s.timemodified IS NOT NULL AND s.status = ? AND + s.attemptnumber = smx.maxattempt AND s.assignment ' . $sqlassignmentids, $dbparams); $unmarkedsubmissions = array(); @@ -438,8 +453,17 @@ function assign_print_overview($courses, &$htmlarray) { } if (has_capability('mod/assign:submit', $context)) { if (!isset($mysubmissions)) { + + // This is nasty because we only want the last attempt. + $submissionmaxattempt = 'SELECT mxs.userid, MAX(mxs.attemptnumber) AS maxattempt, mxs.assignment + FROM {assign_submission} mxs + GROUP BY mxs.userid, mxs.assignment'; + $grademaxattempt = 'SELECT mxg.userid, MAX(mxg.attemptnumber) AS maxattempt, mxg.assignment + FROM {assign_grades} mxg + GROUP BY mxg.userid, mxg.assignment'; + // Get all user submissions, indexed by assignment id. - $dbparams = array_merge(array($USER->id, $USER->id), $assignmentidparams); + $dbparams = array_merge(array($USER->id, $USER->id, $USER->id, $USER->id), $assignmentidparams); $mysubmissions = $DB->get_records_sql('SELECT a.id AS assignment, a.nosubmissions AS nosubmissions, @@ -448,10 +472,18 @@ function assign_print_overview($courses, &$htmlarray) { g.grade AS grade, s.status AS status FROM {assign} a + LEFT JOIN ( ' . $submissionmaxattempt . ' ) smx ON + smx.userid = ? AND + smx.assignment = a.id + LEFT JOIN ( ' . $grademaxattempt . ' ) gmx ON + gmx.userid = ? AND + gmx.assignment = a.id LEFT JOIN {assign_grades} g ON g.assignment = a.id AND - g.userid = ? + g.userid = ? AND + g.attemptnumber = gmx.maxattempt LEFT JOIN {assign_submission} s ON + s.attemptnumber = smx.maxattempt AND s.assignment = a.id AND s.userid = ? WHERE a.id ' . $sqlassignmentids, $dbparams); From 99e0bb23bf21d0e7ed64732dd5739beb4a13103a Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Thu, 19 Jun 2014 12:39:08 +0800 Subject: [PATCH 3/5] MDL-46044 Assign: Move common setup to setUp method (share it for all of lib_test.php) --- mod/assign/tests/lib_test.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/mod/assign/tests/lib_test.php b/mod/assign/tests/lib_test.php index 2691057673c..f24a792218f 100644 --- a/mod/assign/tests/lib_test.php +++ b/mod/assign/tests/lib_test.php @@ -39,8 +39,10 @@ require_once($CFG->dirroot . '/mod/assign/tests/base_test.php'); */ class mod_assign_lib_testcase extends mod_assign_base_testcase { - public function test_assign_print_overview() { - global $DB; + protected function setUp() { + parent::setUp(); + + // Add additional default data (some real attempts and stuff). $this->setUser($this->editingteachers[0]); $this->create_instance(); $assign = $this->create_instance(array('duedate' => time(), @@ -49,10 +51,8 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { 'submissiondrafts' => 1, 'assignsubmission_onlinetext_enabled' => 1)); - $courses = $DB->get_records('course', array('id' => $this->course->id)); - - $this->setUser($this->students[0]); // Add a submission. + $this->setUser($this->students[0]); $submission = $assign->get_user_submission($this->students[0]->id, true); $data = new stdClass(); $data->onlinetext_editor = array('itemid' => file_get_unused_draft_itemid(), @@ -99,6 +99,12 @@ class mod_assign_lib_testcase extends mod_assign_base_testcase { // And now submit it for marking (again). $submission->status = ASSIGN_SUBMISSION_STATUS_SUBMITTED; $assign->testable_update_submission($submission, $this->students[0]->id, true, false); + } + + public function test_assign_print_overview() { + global $DB; + $courses = $DB->get_records('course', array('id' => $this->course->id)); + // Check the overview as the different users. $this->setUser($this->students[0]); From 70971d43eb6105846ecdbdf279f56bd25f290e30 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Fri, 27 Jun 2014 14:14:14 +0800 Subject: [PATCH 4/5] MDL-46044 Assign: Modify myhome query to only use the maxattempt from the submission Using mismatches from the submissions/grades tables does not make much sense, we should always only consider only the maxattempt from the submissions table. --- mod/assign/lib.php | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/mod/assign/lib.php b/mod/assign/lib.php index 29eb712cabe..abb222c579f 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -393,9 +393,6 @@ function assign_print_overview($courses, &$htmlarray) { $submissionmaxattempt = 'SELECT mxs.userid, MAX(mxs.attemptnumber) AS maxattempt, mxs.assignment FROM {assign_submission} mxs GROUP BY mxs.userid, mxs.assignment'; - $grademaxattempt = 'SELECT mxg.userid, MAX(mxg.attemptnumber) AS maxattempt, mxg.assignment - FROM {assign_grades} mxg - GROUP BY mxg.userid, mxg.assignment'; // Build up and array of unmarked submissions indexed by assignment id/ userid // for use where the user has grading rights on assignment. @@ -410,13 +407,10 @@ function assign_print_overview($courses, &$htmlarray) { LEFT JOIN ( ' . $submissionmaxattempt . ' ) smx ON smx.userid = s.userid AND smx.assignment = s.id - LEFT JOIN ( ' . $grademaxattempt . ' ) gmx ON - gmx.userid = s.userid AND - gmx.assignment = s.id LEFT JOIN {assign_grades} g ON s.userid = g.userid AND s.assignment = g.assignment AND - g.attemptnumber = gmx.maxattempt + g.attemptnumber = smx.maxattempt WHERE ( g.timemodified is NULL OR s.timemodified > g.timemodified ) AND @@ -458,9 +452,6 @@ function assign_print_overview($courses, &$htmlarray) { $submissionmaxattempt = 'SELECT mxs.userid, MAX(mxs.attemptnumber) AS maxattempt, mxs.assignment FROM {assign_submission} mxs GROUP BY mxs.userid, mxs.assignment'; - $grademaxattempt = 'SELECT mxg.userid, MAX(mxg.attemptnumber) AS maxattempt, mxg.assignment - FROM {assign_grades} mxg - GROUP BY mxg.userid, mxg.assignment'; // Get all user submissions, indexed by assignment id. $dbparams = array_merge(array($USER->id, $USER->id, $USER->id, $USER->id), $assignmentidparams); @@ -475,13 +466,10 @@ function assign_print_overview($courses, &$htmlarray) { LEFT JOIN ( ' . $submissionmaxattempt . ' ) smx ON smx.userid = ? AND smx.assignment = a.id - LEFT JOIN ( ' . $grademaxattempt . ' ) gmx ON - gmx.userid = ? AND - gmx.assignment = a.id LEFT JOIN {assign_grades} g ON g.assignment = a.id AND g.userid = ? AND - g.attemptnumber = gmx.maxattempt + g.attemptnumber = smx.maxattempt LEFT JOIN {assign_submission} s ON s.attemptnumber = smx.maxattempt AND s.assignment = a.id AND @@ -491,15 +479,18 @@ function assign_print_overview($courses, &$htmlarray) { $str .= '
'; $str .= get_string('mysubmission', 'assign'); - $submission = $mysubmissions[$assignment->id]; - if ($submission->nosubmissions) { - $str .= get_string('offline', 'assign'); - } else if (!$submission->status || $submission->status == 'draft') { + $submission = false; + if (isset($mysubmissions[$assignment->id])) { + $submission = $mysubmissions[$assignment->id]; + } + if (!$submission || !$submission->status || $submission->status == 'draft') { $str .= $strnotsubmittedyet; + } else if ($submission->nosubmissions) { + $str .= get_string('offline', 'assign'); } else { $str .= get_string('submissionstatus_' . $submission->status, 'assign'); } - if (!$submission->grade || $submission->grade < 0) { + if (!$submission || !$submission->grade || $submission->grade < 0) { $str .= ', ' . get_string('notgraded', 'assign'); } else { $str .= ', ' . get_string('graded', 'assign'); From 1affa3ea2278a57df3c58b27fb4d4526ed06b74d Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Fri, 27 Jun 2014 14:58:58 +0800 Subject: [PATCH 5/5] MDL-46044 Assign: Add conditions to the inner query for performance --- mod/assign/lib.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mod/assign/lib.php b/mod/assign/lib.php index abb222c579f..d5ff98e31c6 100644 --- a/mod/assign/lib.php +++ b/mod/assign/lib.php @@ -392,11 +392,12 @@ function assign_print_overview($courses, &$htmlarray) { if (!isset($unmarkedsubmissions)) { $submissionmaxattempt = 'SELECT mxs.userid, MAX(mxs.attemptnumber) AS maxattempt, mxs.assignment FROM {assign_submission} mxs + WHERE mxs.assignment ' . $sqlassignmentids . ' GROUP BY mxs.userid, mxs.assignment'; // 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(array(ASSIGN_SUBMISSION_STATUS_SUBMITTED), $assignmentidparams); + $dbparams = array_merge($assignmentidparams, array(ASSIGN_SUBMISSION_STATUS_SUBMITTED), $assignmentidparams); $rs = $DB->get_recordset_sql('SELECT s.assignment as assignment, s.userid as userid, @@ -451,10 +452,12 @@ function assign_print_overview($courses, &$htmlarray) { // This is nasty because we only want the last attempt. $submissionmaxattempt = 'SELECT mxs.userid, MAX(mxs.attemptnumber) AS maxattempt, mxs.assignment FROM {assign_submission} mxs + WHERE mxs.assignment ' . $sqlassignmentids . ' + AND mxs.userid = ? GROUP BY mxs.userid, mxs.assignment'; // Get all user submissions, indexed by assignment id. - $dbparams = array_merge(array($USER->id, $USER->id, $USER->id, $USER->id), $assignmentidparams); + $dbparams = array_merge($assignmentidparams, array($USER->id, $USER->id, $USER->id), $assignmentidparams); $mysubmissions = $DB->get_records_sql('SELECT a.id AS assignment, a.nosubmissions AS nosubmissions, @@ -464,7 +467,6 @@ function assign_print_overview($courses, &$htmlarray) { s.status AS status FROM {assign} a LEFT JOIN ( ' . $submissionmaxattempt . ' ) smx ON - smx.userid = ? AND smx.assignment = a.id LEFT JOIN {assign_grades} g ON g.assignment = a.id AND