From 0ffcd115c6c5c3566b8623c2576cd70114c71e51 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Mon, 5 Feb 2024 16:28:01 +0000 Subject: [PATCH 1/8] MDL-68806 quiz: Add new "Not started" and "Submitted" states --- .upgradenotes/MDL-68806-2024060613451616.yml | 21 ++++ .../tool/recyclebin/tests/course_bin_test.php | 3 +- .../tests/restore_stepslib_date_test.php | 3 +- .../seb/tests/test_helper_trait.php | 3 +- mod/quiz/classes/event/attempt_graded.php | 104 ++++++++++++++++++ mod/quiz/classes/quiz_attempt.php | 85 ++++++++++++-- mod/quiz/lang/en/quiz.php | 1 + mod/quiz/lib.php | 3 +- mod/quiz/locallib.php | 103 ++++++++++++----- .../report/overview/tests/report_test.php | 32 +++++- mod/quiz/tests/attempt_walkthrough_test.php | 73 ++++++++---- mod/quiz/tests/backup/restore_date_test.php | 2 +- mod/quiz/tests/behat/behat_mod_quiz.php | 3 +- .../classes/attempt_walkthrough_testcase.php | 3 +- .../classes/question_helper_test_trait.php | 3 +- mod/quiz/tests/custom_completion_test.php | 3 +- mod/quiz/tests/event/events_test.php | 36 +++++- mod/quiz/tests/external/external_test.php | 24 ++-- mod/quiz/tests/generator/lib.php | 3 +- mod/quiz/tests/lib_test.php | 12 +- mod/quiz/tests/privacy/provider_test.php | 6 +- ..._attempt_manual_grading_completed_test.php | 21 ++-- .../tests/datalib_reporting_queries_test.php | 3 +- .../statistics_bulk_loader_test.php | 3 +- 24 files changed, 456 insertions(+), 97 deletions(-) create mode 100644 .upgradenotes/MDL-68806-2024060613451616.yml create mode 100644 mod/quiz/classes/event/attempt_graded.php diff --git a/.upgradenotes/MDL-68806-2024060613451616.yml b/.upgradenotes/MDL-68806-2024060613451616.yml new file mode 100644 index 00000000000..31c3c24891e --- /dev/null +++ b/.upgradenotes/MDL-68806-2024060613451616.yml @@ -0,0 +1,21 @@ +issueNumber: MDL-68806 +notes: + mod_quiz: + - message: >- + quiz_attempt now has 2 additional state values, NOT_STARTED and + SUBMITTED. These represent attempts when an attempt has been + type: improved + - message: > + quiz_attempt_save_started now sets the IN_PROGRESS state, timestarted, + and saves the attempt, while the new quiz_attempt_save_not_started + function sets the NOT_STARTED state and saves the attempt. + type: changed + - message: > + quiz_attempt::process_finish is now deprecated, and its functionality is + split between ::process_submit, which saves the + + submission, sets the finish time and sets the SUBMITTED status, and + ::process_grade_submission which performs automated + + grading and sets the FINISHED status. + type: deprecated diff --git a/admin/tool/recyclebin/tests/course_bin_test.php b/admin/tool/recyclebin/tests/course_bin_test.php index 60cf0d5ead7..37e9f10a302 100644 --- a/admin/tool/recyclebin/tests/course_bin_test.php +++ b/admin/tool/recyclebin/tests/course_bin_test.php @@ -321,6 +321,7 @@ final class course_bin_test extends \advanced_testcase { $tosubmit = array(1 => array('answer' => '0')); $attemptobj->process_submitted_actions($timenow, false, $tosubmit); $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); } } diff --git a/backup/moodle2/tests/restore_stepslib_date_test.php b/backup/moodle2/tests/restore_stepslib_date_test.php index e048efdf1d6..6156ed50675 100644 --- a/backup/moodle2/tests/restore_stepslib_date_test.php +++ b/backup/moodle2/tests/restore_stepslib_date_test.php @@ -409,7 +409,8 @@ final class restore_stepslib_date_test extends \restore_date_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); $questionattemptstepdates = []; $originaliterator = $quba->get_attempt_iterator(); diff --git a/mod/quiz/accessrule/seb/tests/test_helper_trait.php b/mod/quiz/accessrule/seb/tests/test_helper_trait.php index 40829fcd176..c096d802861 100644 --- a/mod/quiz/accessrule/seb/tests/test_helper_trait.php +++ b/mod/quiz/accessrule/seb/tests/test_helper_trait.php @@ -217,7 +217,8 @@ trait quizaccess_seb_test_helper_trait { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($starttime, false); + $attemptobj->process_submit($starttime, false); + $attemptobj->process_grade_submission($starttime); $this->setUser(); diff --git a/mod/quiz/classes/event/attempt_graded.php b/mod/quiz/classes/event/attempt_graded.php new file mode 100644 index 00000000000..b56c720edd1 --- /dev/null +++ b/mod/quiz/classes/event/attempt_graded.php @@ -0,0 +1,104 @@ +. + +/** + * The mod_quiz attempt submitted event. + * + * @package mod_quiz + * @copyright 2024 Catalyst IT Europe Ltd. + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +namespace mod_quiz\event; + +/** + * The mod_quiz attempt graded event class. + * + * @package mod_quiz + * @since Moodle 4.5 + * @copyright 2024 + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class attempt_graded extends \core\event\base { + /** + * Init method. + */ + protected function init(): void { + $this->data['objecttable'] = 'quiz_attempts'; + $this->data['crud'] = 'u'; + $this->data['edulevel'] = self::LEVEL_PARTICIPATING; + } + + /** + * Returns description of what happened. + * + * @return string + */ + public function get_description(): string { + return "The attempt with id {$this->objectid} for for quiz with course module id {$this->contextinstanceid} by user with " . + "id {$this->relateduserid} had automatic grading performed."; + } + + /** + * Returns localised general event name. + * + * @return string + */ + public static function get_name(): string { + return get_string('eventquizattemptgraded', 'mod_quiz'); + } + + /** + * Returns relevant URL. + * + * @return \moodle_url + */ + public function get_url(): \moodle_url { + return new \moodle_url('/mod/quiz/review.php', ['attempt' => $this->objectid]); + } + + /** + * Custom validation. + * + * @throws \coding_exception + * @return void + */ + protected function validate_data(): void { + parent::validate_data(); + + if (!isset($this->relateduserid)) { + throw new \coding_exception('The \'relateduserid\' must be set.'); + } + + if (!array_key_exists('submitterid', $this->other)) { + throw new \coding_exception('The \'submitterid\' value must be set in other.'); + } + } + + #[\Override] + public static function get_objectid_mapping(): array { + return ['db' => 'quiz_attempts', 'restore' => 'quiz_attempt']; + } + + #[\Override] + public static function get_other_mapping(): array { + $othermapped = []; + $othermapped['submitterid'] = ['db' => 'user', 'restore' => 'user']; + $othermapped['quizid'] = ['db' => 'quiz', 'restore' => 'quiz']; + + return $othermapped; + } +} diff --git a/mod/quiz/classes/quiz_attempt.php b/mod/quiz/classes/quiz_attempt.php index 2ec840f72b6..978866f4d9f 100644 --- a/mod/quiz/classes/quiz_attempt.php +++ b/mod/quiz/classes/quiz_attempt.php @@ -53,10 +53,14 @@ use stdClass; */ class quiz_attempt { + /** @var string to identify the "not started" state, when an attempt has been pre-generated. */ + const NOT_STARTED = 'notstarted'; /** @var string to identify the in progress state. */ const IN_PROGRESS = 'inprogress'; /** @var string to identify the overdue state. */ const OVERDUE = 'overdue'; + /** @var string to identify the submitted state, when an attempt is awaiting grading. */ + const SUBMITTED = 'submitted'; /** @var string to identify the finished state. */ const FINISHED = 'finished'; /** @var string to identify the abandoned state. */ @@ -1648,7 +1652,8 @@ class quiz_attempt { // Transition to the appropriate state. switch ($this->quizobj->get_quiz()->overduehandling) { case 'autosubmit': - $this->process_finish($timestamp, false, $studentisonline ? $timestamp : $timeclose, $studentisonline); + $this->process_submit($timestamp, false, $studentisonline ? $timestamp : $timeclose, $studentisonline); + $this->process_grade_submission($studentisonline ? $timestamp : $timeclose); return; case 'graceperiod': @@ -1805,8 +1810,36 @@ class quiz_attempt { * @param ?int $timefinish if set, use this as the finish time for the attempt. * (otherwise use $timestamp as the finish time as well). * @param bool $studentisonline is the student currently interacting with Moodle? + * @deprecated since Moodle 5.0 MDL-68806 use process_submit() and process_grade_submission() instead + * @todo Final deprecation in Moodle 6.0 MDL-80956 */ public function process_finish($timestamp, $processsubmitted, $timefinish = null, $studentisonline = false) { + debugging('quiz_attempt::process_finish is deprecated. Please use quiz_attempt::process_submit to store ' . + 'answers and mark an attempt submitted, and quiz_attempt::process_grade_submission to do automatic grading.'); + $this->process_submit($timestamp, $processsubmitted, $timefinish, $studentisonline); + $this->process_grade_submission($timefinish ?? $timestamp); + } + + /** + * Submit the attempt. + * + * The separate $timefinish argument should be used when the quiz attempt + * is being processed asynchronously (for example when cron is submitting + * attempts where the time has expired). + * + * @param int $timestamp the time to record as last modified time. + * @param bool $processsubmitted if true, and question responses in the current + * POST request are stored to be graded, before the attempt is finished. + * @param ?int $timefinish if set, use this as the finish time for the attempt. + * (otherwise use $timestamp as the finish time as well). + * @param bool $studentisonline is the student currently interacting with Moodle? + */ + public function process_submit( + int $timestamp, + bool $processsubmitted, + ?int $timefinish = null, + bool $studentisonline = false + ): void { global $DB; $transaction = $DB->start_delegated_transaction(); @@ -1814,7 +1847,6 @@ class quiz_attempt { if ($processsubmitted) { $this->quba->process_all_actions($timestamp); } - $this->quba->finish_all_questions($timestamp); question_engine::save_questions_usage_by_activity($this->quba); @@ -1822,15 +1854,44 @@ class quiz_attempt { $this->attempt->timemodified = $timestamp; $this->attempt->timefinish = $timefinish ?? $timestamp; + $this->attempt->state = self::SUBMITTED; + $this->attempt->timecheckstate = null; + + $DB->update_record('quiz_attempts', $this->attempt); + + if (!$this->is_preview()) { + // Trigger event. + $this->fire_state_transition_event('\mod_quiz\event\attempt_submitted', $timestamp, $studentisonline); + \core\hook\manager::get_instance()->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); + } + + $transaction->allow_commit(); + } + + /** + * Perform automatic grading for a submitted attempt. + * + * @param int $timestamp the time to record as last modified time. + */ + public function process_grade_submission(int $timestamp): void { + global $DB; + + $transaction = $DB->start_delegated_transaction(); + $this->quba->finish_all_questions($timestamp); + question_engine::save_questions_usage_by_activity($this->quba); + + $originalattempt = clone $this->attempt; + + $this->attempt->timemodified = $timestamp; + $this->attempt->sumgrades = $this->quba->get_total_mark(); $this->attempt->state = self::FINISHED; - $this->attempt->timecheckstate = null; - $this->attempt->gradednotificationsenttime = null; - if (!$this->requires_manual_grading() || - !has_capability('mod/quiz:emailnotifyattemptgraded', $this->get_quizobj()->get_context(), - $this->get_userid())) { - $this->attempt->gradednotificationsenttime = $this->attempt->timefinish; + if ( + !$this->requires_manual_grading() || + !has_capability('mod/quiz:emailnotifyattemptgraded', $this->get_quizobj()->get_context(), $this->get_userid()) + ) { + $this->attempt->gradednotificationsenttime = $timestamp; } $DB->update_record('quiz_attempts', $this->attempt); @@ -1839,7 +1900,7 @@ class quiz_attempt { $this->recompute_final_grade(); // Trigger event. - $this->fire_state_transition_event('\mod_quiz\event\attempt_submitted', $timestamp, $studentisonline); + $this->fire_state_transition_event('\mod_quiz\event\attempt_graded', $timestamp, false); di::get(hook\manager::class)->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); // Tell any access rules that care that the attempt is over. @@ -1947,7 +2008,8 @@ class quiz_attempt { di::get(hook\manager::class)->dispatch(new attempt_state_changed($originalattempt, $this->attempt)); $timeclose = $this->get_access_manager($timestamp)->get_end_time($this->attempt); if ($timeclose && $timestamp > $timeclose) { - $this->process_finish($timestamp, false, $timeclose); + $this->process_submit($timestamp, false, $timeclose); + $this->process_grade_submission($timeclose); } $transaction->allow_commit(); @@ -2158,7 +2220,8 @@ class quiz_attempt { // late to be processed, record the close time, to reduce confusion. $finishtime = $timeclose; } - $this->process_finish($timenow, !$toolate, $finishtime, true); + $this->process_submit($timenow, !$toolate, $finishtime, true); + $this->process_grade_submission($finishtime); } } catch (question_out_of_sequence_exception $e) { diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index f0b4a5c9796..58e3712431c 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -370,6 +370,7 @@ $string['eventpagebreakcreated'] = 'Page break created'; $string['eventpagebreakdeleted'] = 'Page break deleted'; $string['eventquestionmanuallygraded'] = 'Question manually graded'; $string['eventquizattemptabandoned'] = 'Quiz attempt abandoned'; +$string['eventquizattemptgraded'] = 'Quiz attempt graded'; $string['eventquizattemptregraded'] = 'Quiz attempt regraded'; $string['eventquizattemptreopened'] = 'Quiz attempt reopened'; $string['eventquizattemptstarted'] = 'Quiz attempt started'; diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php index f3950652477..9f76382a948 100644 --- a/mod/quiz/lib.php +++ b/mod/quiz/lib.php @@ -524,9 +524,10 @@ function quiz_get_user_attempts($quizids, $userid, $status = 'finished', $includ break; case 'finished': - $statuscondition = ' AND state IN (:state1, :state2)'; + $statuscondition = ' AND state IN (:state1, :state2, :state3)'; $params['state1'] = quiz_attempt::FINISHED; $params['state2'] = quiz_attempt::ABANDONED; + $params['state3'] = quiz_attempt::SUBMITTED; break; case 'unfinished': diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 344a2263964..02a24a9b3e3 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -128,29 +128,19 @@ function quiz_create_attempt(quiz_settings $quizobj, $attemptnumber, $lastattemp } $attempt->attempt = $attemptnumber; - $attempt->timestart = $timenow; $attempt->timefinish = 0; $attempt->timemodified = $timenow; $attempt->timemodifiedoffline = 0; - $attempt->state = quiz_attempt::IN_PROGRESS; $attempt->currentpage = 0; $attempt->sumgrades = null; $attempt->gradednotificationsenttime = null; + $attempt->timecheckstate = null; // If this is a preview, mark it as such. if ($ispreview) { $attempt->preview = 1; } - $timeclose = $quizobj->get_access_manager($timenow)->get_end_time($attempt); - if ($timeclose === false || $ispreview) { - $attempt->timecheckstate = null; - } else { - $attempt->timecheckstate = $timeclose; - } - - di::get(hook\manager::class)->dispatch(new attempt_state_changed(null, $attempt)); - return $attempt; } /** @@ -354,19 +344,44 @@ function quiz_start_attempt_built_on_last($quba, $attempt, $lastattempt) { } /** - * The save started question usage and quiz attempt in db and log the started attempt. + * Create or update the quiz attempt record, and the question usage. * * @param quiz_settings $quizobj * @param question_usage_by_activity $quba * @param stdClass $attempt + * @param ?int $timenow The time to use for the attempt's timestart property. Defaults to time(). * @return stdClass attempt object with uniqueid and id set. */ -function quiz_attempt_save_started($quizobj, $quba, $attempt) { +function quiz_attempt_save_started( + quiz_settings $quizobj, + question_usage_by_activity $quba, + \stdClass $attempt, + ?int $timenow = null, +): stdClass { global $DB; - // Save the attempt in the database. - question_engine::save_questions_usage_by_activity($quba); - $attempt->uniqueid = $quba->get_id(); - $attempt->id = $DB->insert_record('quiz_attempts', $attempt); + + $attempt->timestart = $timenow ?? time(); + + $timeclose = $quizobj->get_access_manager($attempt->timestart)->get_end_time($attempt); + if ($timeclose === false || $attempt->preview) { + $attempt->timecheckstate = null; + } else { + $attempt->timecheckstate = $timeclose; + } + + $originalattempt = null; + if (isset($attempt->id) && $attempt->state === quiz_attempt::NOT_STARTED) { + $originalattempt = clone $attempt; + // Update the attempt's state. + $attempt->state = quiz_attempt::IN_PROGRESS; + $DB->update_record('quiz_attempts', $attempt); + } else { + // Save the attempt in the database. + question_engine::save_questions_usage_by_activity($quba); + $attempt->uniqueid = $quba->get_id(); + $attempt->state = quiz_attempt::IN_PROGRESS; + $attempt->id = $DB->insert_record('quiz_attempts', $attempt); + } // Params used by the events below. $params = [ @@ -391,6 +406,26 @@ function quiz_attempt_save_started($quizobj, $quba, $attempt) { $event->add_record_snapshot('quiz_attempts', $attempt); $event->trigger(); + di::get(hook\manager::class)->dispatch(new attempt_state_changed($originalattempt, $attempt)); + + return $attempt; +} + +/** + * Create the quiz attempt record, and the question usage. + * + * @param question_usage_by_activity $quba + * @param stdClass $attempt + * @return stdClass attempt object with uniqueid and id set. + */ +function quiz_attempt_save_not_started(question_usage_by_activity $quba, stdClass $attempt): stdClass { + global $DB; + // Save the attempt in the database. + question_engine::save_questions_usage_by_activity($quba); + $attempt->uniqueid = $quba->get_id(); + $attempt->state = quiz_attempt::NOT_STARTED; + $attempt->id = $DB->insert_record('quiz_attempts', $attempt); + di::get(hook\manager::class)->dispatch(new attempt_state_changed(null, $attempt)); return $attempt; } @@ -2047,15 +2082,33 @@ function quiz_prepare_and_start_new_attempt(quiz_settings $quizobj, $attemptnumb $quba = question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); - // Create the new attempt and initialize the question sessions - $timenow = time(); // Update time now, in case the server is running really slowly. - $attempt = quiz_create_attempt($quizobj, $attemptnumber, $lastattempt, $timenow, $ispreviewuser, $userid); + $attempt = $DB->get_record( + 'quiz_attempts', + [ + 'quiz' => $quizobj->get_quizid(), + 'userid' => $userid, + 'preview' => 0, + 'state' => quiz_attempt::NOT_STARTED, + ], + ); + if (!$attempt) { + // Create the new attempt and initialize the question sessions. + $timenow = time(); // Update time now, in case the server is running really slowly. + $attempt = quiz_create_attempt($quizobj, $attemptnumber, $lastattempt, $timenow, $ispreviewuser, $userid); - if (!($quizobj->get_quiz()->attemptonlast && $lastattempt)) { - $attempt = quiz_start_new_attempt($quizobj, $quba, $attempt, $attemptnumber, $timenow, - $forcedrandomquestions, $forcedvariants); - } else { - $attempt = quiz_start_attempt_built_on_last($quba, $attempt, $lastattempt); + if (!($quizobj->get_quiz()->attemptonlast && $lastattempt)) { + $attempt = quiz_start_new_attempt( + $quizobj, + $quba, + $attempt, + $attemptnumber, + $timenow, + $forcedrandomquestions, + $forcedvariants, + ); + } else { + $attempt = quiz_start_attempt_built_on_last($quba, $attempt, $lastattempt); + } } $transaction = $DB->start_delegated_transaction(); diff --git a/mod/quiz/report/overview/tests/report_test.php b/mod/quiz/report/overview/tests/report_test.php index ee9f10113bb..8a31fbde31b 100644 --- a/mod/quiz/report/overview/tests/report_test.php +++ b/mod/quiz/report/overview/tests/report_test.php @@ -109,7 +109,8 @@ final class report_test extends \advanced_testcase { [$quiz, $student1, 2, 5.0, quiz_attempt::FINISHED], [$quiz, $student1, 3, 8.0, quiz_attempt::FINISHED], [$quiz, $student1, 4, null, quiz_attempt::ABANDONED], - [$quiz, $student1, 5, null, quiz_attempt::IN_PROGRESS], + [$quiz, $student1, 5, null, quiz_attempt::SUBMITTED], + [$quiz, $student1, 6, null, quiz_attempt::IN_PROGRESS], [$quiz, $student2, 1, null, quiz_attempt::ABANDONED], [$quiz, $student2, 2, null, quiz_attempt::ABANDONED], [$quiz, $student2, 3, 7.0, quiz_attempt::FINISHED], @@ -143,11 +144,29 @@ final class report_test extends \advanced_testcase { // Do nothing. break; + case quiz_attempt::SUBMITTED: + // Save answers but do not grade attempt. + $attemptobj->process_submitted_actions( + $timestart + 300, + false, + [ + 1 => ['answer' => 'My essay by ' . $student->firstname, 'answerformat' => FORMAT_PLAIN], + ] + ); + $attemptobj->process_submit($timestart + 600, false); + break; + case quiz_attempt::FINISHED: // Save answer and finish attempt. - $attemptobj->process_submitted_actions($timestart + 300, false, [ - 1 => ['answer' => 'My essay by ' . $student->firstname, 'answerformat' => FORMAT_PLAIN]]); - $attemptobj->process_finish($timestart + 600, false); + $attemptobj->process_submitted_actions( + $timestart + 300, + false, + [ + 1 => ['answer' => 'My essay by ' . $student->firstname, 'answerformat' => FORMAT_PLAIN], + ] + ); + $attemptobj->process_submit($timestart + 600, false); + $attemptobj->process_grade_submission($timestart + 600); // Manually grade it. $quba = $attemptobj->get_question_usage(); @@ -207,7 +226,7 @@ final class report_test extends \advanced_testcase { $this->assertArrayHasKey($student1->id . '#3', $table->rawdata); $this->assertEquals(1, $table->rawdata[$student1->id . '#3']->gradedattempt); $this->assertArrayHasKey($student1->id . '#3', $table->rawdata); - $this->assertEquals(0, $table->rawdata[$student1->id . '#5']->gradedattempt); + $this->assertEquals(0, $table->rawdata[$student1->id . '#6']->gradedattempt); $this->assertArrayHasKey($student2->id . '#3', $table->rawdata); $this->assertEquals(1, $table->rawdata[$student2->id . '#3']->gradedattempt); $this->assertArrayHasKey($student3->id . '#0', $table->rawdata); @@ -361,7 +380,8 @@ final class report_test extends \advanced_testcase { $attempt = quiz_prepare_and_start_new_attempt($quizobj, 1, null); $attemptobj = quiz_attempt::create($attempt->id); $attemptobj->process_submitted_actions(time(), false, [1 => ['answer' => 'toad']]); - $attemptobj->process_finish(time(), false); + $attemptobj->process_submit(time(), false); + $attemptobj->process_grade_submission(time()); // We should be using 'always latest' version, which is currently v2, so should be right. $this->assertEquals(10, $attemptobj->get_question_usage()->get_total_mark()); diff --git a/mod/quiz/tests/attempt_walkthrough_test.php b/mod/quiz/tests/attempt_walkthrough_test.php index e2f875e8fdd..be92c959098 100644 --- a/mod/quiz/tests/attempt_walkthrough_test.php +++ b/mod/quiz/tests/attempt_walkthrough_test.php @@ -129,21 +129,30 @@ final class attempt_walkthrough_test extends \advanced_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); // Re-load quiz attempt data. $attemptobj = quiz_attempt::create($attempt->id); // Check that results are stored as expected. $this->assertEquals(1, $attemptobj->get_attempt_number()); - $this->assertEquals(3, $attemptobj->get_sum_marks()); - $this->assertEquals(true, $attemptobj->is_finished()); + $this->assertEquals(false, $attemptobj->is_finished()); $this->assertEquals($timenow, $attemptobj->get_submitted_date()); $this->assertEquals($user1->id, $attemptobj->get_userid()); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); $this->assertEquals(0, $attemptobj->get_number_of_unanswered_questions()); + // Check we don't have grades yet. + $this->assertEmpty(quiz_get_user_grades($quiz, $user1->id)); + $this->assertNull($attemptobj->get_sum_marks()); + + // Now grade the submission. + $attemptobj->process_grade_submission($timenow); + $attemptobj = quiz_attempt::create($attempt->id); // Check quiz grades. + $this->assertEquals(true, $attemptobj->is_finished()); + $this->assertEquals(3, $attemptobj->get_sum_marks()); + $grades = quiz_get_user_grades($quiz, $user1->id); $grade = array_shift($grades); $this->assertEquals(100.0, $grade->rawgrade); @@ -412,20 +421,31 @@ final class attempt_walkthrough_test extends \advanced_testcase { $attemptobj = quiz_attempt::create($attempt->id); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); $this->assertEquals(0, $attemptobj->get_number_of_unanswered_questions()); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); // Re-load quiz attempt data. $attemptobj = quiz_attempt::create($attempt->id); // Check that results are stored as expected. $this->assertEquals(1, $attemptobj->get_attempt_number()); - $this->assertEquals(4, $attemptobj->get_sum_marks()); - $this->assertEquals(true, $attemptobj->is_finished()); + $this->assertEquals(false, $attemptobj->is_finished()); $this->assertEquals($timenow, $attemptobj->get_submitted_date()); $this->assertEquals($user1->id, $attemptobj->get_userid()); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); $this->assertEquals(0, $attemptobj->get_number_of_unanswered_questions()); + // Check we don't have grades yet. + $this->assertEmpty(quiz_get_user_grades($quiz, $user1->id)); + $this->assertNull($attemptobj->get_sum_marks()); + + // Now grade the submission. + $attemptobj->process_grade_submission($timenow); + $attemptobj = quiz_attempt::create($attempt->id); + + // Check quiz grades. + $this->assertEquals(true, $attemptobj->is_finished()); + $this->assertEquals(4, $attemptobj->get_sum_marks()); + // Check quiz grades. $grades = quiz_get_user_grades($quiz, $user1->id); $grade = array_shift($grades); @@ -510,20 +530,30 @@ final class attempt_walkthrough_test extends \advanced_testcase { $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); $this->assertEquals(0, $attemptobj->get_number_of_unanswered_questions()); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); // Re-load quiz attempt data. $attemptobj = quiz_attempt::create($attempt->id); // Check that results are stored as expected. $this->assertEquals(1, $attemptobj->get_attempt_number()); - $this->assertEquals(1, $attemptobj->get_sum_marks()); - $this->assertEquals(true, $attemptobj->is_finished()); + $this->assertEquals(false, $attemptobj->is_finished()); $this->assertEquals($timenow, $attemptobj->get_submitted_date()); $this->assertEquals($user1->id, $attemptobj->get_userid()); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); $this->assertEquals(0, $attemptobj->get_number_of_unanswered_questions()); + // Check we don't have grades yet. + $this->assertEmpty(quiz_get_user_grades($this->quizwithvariants, $user1->id)); + $this->assertNull($attemptobj->get_sum_marks()); + + // Now grade the submission. + $attemptobj->process_grade_submission($timenow); + $attemptobj = quiz_attempt::create($attempt->id); + + // Check quiz grades. + $this->assertEquals(true, $attemptobj->is_finished()); + $this->assertEquals(1, $attemptobj->get_sum_marks()); // Check quiz grades. $grades = quiz_get_user_grades($this->quizwithvariants, $user1->id); $grade = array_shift($grades); @@ -641,22 +671,27 @@ final class attempt_walkthrough_test extends \advanced_testcase { // Verify this was logged correctly - there are some gradebook events between the two we want to check. $events = $sink->get_events(); - $this->assertGreaterThanOrEqual(2, $events); + $this->assertGreaterThanOrEqual(3, $events); + $attempturl = new moodle_url( + '/mod/quiz/review.php', + ['attempt' => $attemptobj->get_attemptid()], + ); $reopenedevent = array_shift($events); $this->assertInstanceOf('\mod_quiz\event\attempt_reopened', $reopenedevent); $this->assertEquals($attemptobj->get_context(), $reopenedevent->get_context()); - $this->assertEquals( - new moodle_url('/mod/quiz/review.php', ['attempt' => $attemptobj->get_attemptid()]), - $reopenedevent->get_url() - ); - $submittedevent = array_pop($events); + $this->assertEquals($attempturl, $reopenedevent->get_url()); + + $submittedevent = array_shift($events); $this->assertInstanceOf('\mod_quiz\event\attempt_submitted', $submittedevent); $this->assertEquals($attemptobj->get_context(), $submittedevent->get_context()); - $this->assertEquals( - new moodle_url('/mod/quiz/review.php', ['attempt' => $attemptobj->get_attemptid()]), - $submittedevent->get_url() - ); + + $this->assertEquals($attempturl, $submittedevent->get_url()); + + $gradedevent = array_pop($events); + $this->assertInstanceOf('\mod_quiz\event\attempt_graded', $gradedevent); + $this->assertEquals($attemptobj->get_context(), $gradedevent->get_context()); + $this->assertEquals($attempturl, $gradedevent->get_url()); } } diff --git a/mod/quiz/tests/backup/restore_date_test.php b/mod/quiz/tests/backup/restore_date_test.php index cba06ab56af..65262a33667 100644 --- a/mod/quiz/tests/backup/restore_date_test.php +++ b/mod/quiz/tests/backup/restore_date_test.php @@ -55,7 +55,7 @@ final class restore_date_test extends \restore_date_testcase { $quba = \question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); quiz_start_new_attempt($quizobj, $quba, $attempt, 1, $timestamp); - quiz_attempt_save_started($quizobj, $quba, $attempt); + quiz_attempt_save_started($quizobj, $quba, $attempt, $timestamp); // Quiz grade. $grade = new \stdClass(); diff --git a/mod/quiz/tests/behat/behat_mod_quiz.php b/mod/quiz/tests/behat/behat_mod_quiz.php index d98aa8ac08a..c21820d7953 100644 --- a/mod/quiz/tests/behat/behat_mod_quiz.php +++ b/mod/quiz/tests/behat/behat_mod_quiz.php @@ -1015,7 +1015,8 @@ class behat_mod_quiz extends behat_question_base { $attempts = quiz_get_user_attempts($quizid, $user->id, 'unfinished', true); $attemptobj = quiz_attempt::create(key($attempts)); - $attemptobj->process_finish(time(), true); + $attemptobj->process_submit(time(), true); + $attemptobj->process_grade_submission(time()); $this->set_user(); } diff --git a/mod/quiz/tests/classes/attempt_walkthrough_testcase.php b/mod/quiz/tests/classes/attempt_walkthrough_testcase.php index cfe4575ae72..9b9f6b33c97 100644 --- a/mod/quiz/tests/classes/attempt_walkthrough_testcase.php +++ b/mod/quiz/tests/classes/attempt_walkthrough_testcase.php @@ -314,7 +314,8 @@ abstract class attempt_walkthrough_testcase extends \advanced_testcase { // Finish the attempt. if (!isset($step['finished']) || ($step['finished'] == 1)) { $attemptobj = quiz_attempt::create($attemptid); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); } } return $attemptids; diff --git a/mod/quiz/tests/classes/question_helper_test_trait.php b/mod/quiz/tests/classes/question_helper_test_trait.php index 9f9ee850739..381df437849 100644 --- a/mod/quiz/tests/classes/question_helper_test_trait.php +++ b/mod/quiz/tests/classes/question_helper_test_trait.php @@ -127,7 +127,8 @@ trait question_helper_test_trait { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($starttime, false); + $attemptobj->process_submit($starttime, false); + $attemptobj->process_grade_submission($starttime); $this->setUser(); return [$quizobj, $quba, $attemptobj]; diff --git a/mod/quiz/tests/custom_completion_test.php b/mod/quiz/tests/custom_completion_test.php index 2ed22234418..ccd496fde92 100644 --- a/mod/quiz/tests/custom_completion_test.php +++ b/mod/quiz/tests/custom_completion_test.php @@ -132,7 +132,8 @@ final class custom_completion_test extends advanced_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); } /** diff --git a/mod/quiz/tests/event/events_test.php b/mod/quiz/tests/event/events_test.php index 6ad7934018f..8cd37eff509 100644 --- a/mod/quiz/tests/event/events_test.php +++ b/mod/quiz/tests/event/events_test.php @@ -118,21 +118,51 @@ final class events_test extends \advanced_testcase { public function test_attempt_submitted(): void { - list($quizobj, $quba, $attempt) = $this->prepare_quiz_data(); + [$quizobj, , $attempt] = $this->prepare_quiz_data(); $attemptobj = quiz_attempt::create($attempt->id); // Catch the event. $sink = $this->redirectEvents(); $timefinish = time(); - $attemptobj->process_finish($timefinish, false); + $attemptobj->process_submit($timefinish, false); + $events = $sink->get_events(); + $sink->close(); + + // Validate the event. + $this->assertCount(1, $events); + $event = $events[0]; + $this->assertInstanceOf('\mod_quiz\event\attempt_submitted', $event); + $this->assertEquals('quiz_attempts', $event->objecttable); + $this->assertEquals($quizobj->get_context(), $event->get_context()); + $this->assertEquals($attempt->userid, $event->relateduserid); + $this->assertEquals(null, $event->other['submitterid']); // Should be the user, but PHP Unit complains... + $this->assertEventContextNotUsed($event); + } + + /** + * The \mod_quiz\event\attempt_graded event should be fired when an attempt is graded. + * + * @return void + * @covers \mod_quiz\quiz_attempt::process_grade_submission + */ + public function test_attempt_graded(): void { + + [$quizobj, , $attempt] = $this->prepare_quiz_data(); + $attemptobj = quiz_attempt::create($attempt->id); + $timefinish = time(); + $attemptobj->process_submit($timefinish, false); + + // Catch the event. + $sink = $this->redirectEvents(); + $attemptobj->process_grade_submission($timefinish); $events = $sink->get_events(); $sink->close(); // Validate the event. $this->assertCount(3, $events); $event = $events[2]; - $this->assertInstanceOf('\mod_quiz\event\attempt_submitted', $event); + $this->assertInstanceOf('\mod_quiz\event\attempt_graded', $event); $this->assertEquals('quiz_attempts', $event->objecttable); $this->assertEquals($quizobj->get_context(), $event->get_context()); $this->assertEquals($attempt->userid, $event->relateduserid); diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index 09fb3d66a96..5dccf215295 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -206,7 +206,8 @@ final class external_test extends externallib_advanced_testcase { $attemptobj->process_submitted_actions(time(), false, $tosubmit); // Finish the attempt. - $attemptobj->process_finish(time(), false); + $attemptobj->process_submit(time(), false); + $attemptobj->process_grade_submission(time()); } return [$quiz, $context, $quizobj, $attempt, $attemptobj, $quba]; } else { @@ -661,7 +662,8 @@ final class external_test extends externallib_advanced_testcase { $attemptobj->process_submitted_actions($timenow, false, [1 => ['answer' => '3.14']]); // Finish the attempt. - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); $result = mod_quiz_external::get_user_best_grade($quizapi1->id); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_best_grade_returns(), $result); @@ -723,7 +725,8 @@ final class external_test extends externallib_advanced_testcase { $attemptobj->process_submitted_actions($timenow, false, [1 => ['answer' => '3.14']]); // Finish the attempt. - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); $result = mod_quiz_external::get_user_best_grade($quizapi2->id); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_best_grade_returns(), $result); @@ -809,7 +812,8 @@ final class external_test extends externallib_advanced_testcase { // Now, finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); $expected = [ "someoptions" => [ @@ -1026,7 +1030,8 @@ final class external_test extends externallib_advanced_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attemptid); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); // We should be able to start a new attempt. $result = mod_quiz_external::start_attempt($quiz->id, [["name" => "quizpassword", "value" => 'abc']]); @@ -1127,7 +1132,8 @@ final class external_test extends externallib_advanced_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish(time(), false); + $attemptobj->process_submit(time(), false); + $attemptobj->process_grade_submission(time()); try { testable_mod_quiz_external::validate_attempt($params, false); @@ -1233,7 +1239,8 @@ final class external_test extends externallib_advanced_testcase { $this->assertEquals(false, $result['questions'][0]['hasautosavedstep']); // Finish previous attempt. - $attemptobj->process_finish(time(), false); + $attemptobj->process_submit(time(), false); + $attemptobj->process_grade_submission(time()); // Now we should receive the question state. $result = mod_quiz_external::get_attempt_review($attempt->id, 1); @@ -2126,7 +2133,8 @@ final class external_test extends externallib_advanced_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); // Can we start a new attempt? We shall not! $result = mod_quiz_external::get_attempt_access_information($quiz->id, $attempt->id); diff --git a/mod/quiz/tests/generator/lib.php b/mod/quiz/tests/generator/lib.php index 319fe05cf23..da94f9b80b4 100644 --- a/mod/quiz/tests/generator/lib.php +++ b/mod/quiz/tests/generator/lib.php @@ -181,7 +181,8 @@ class mod_quiz_generator extends testing_module_generator { } if ($finishattempt) { - $attemptobj->process_finish(time(), false); + $attemptobj->process_submit(time(), false); + $attemptobj->process_grade_submission(time()); } } diff --git a/mod/quiz/tests/lib_test.php b/mod/quiz/tests/lib_test.php index f9e8d084def..1cdde5fa2b0 100644 --- a/mod/quiz/tests/lib_test.php +++ b/mod/quiz/tests/lib_test.php @@ -193,7 +193,8 @@ final class lib_test extends \advanced_testcase { quiz_attempt_save_started($quizobj1a, $quba1a, $attempt); $attemptobj = quiz_attempt::create($attempt->id); $attemptobj->process_submitted_actions($timenow, false, [1 => ['answer' => '3.14']]); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); // User 2 goes overdue in quiz 1. $attempt = quiz_create_attempt($quizobj1b, 1, false, $timenow, false, $u2->id); @@ -231,7 +232,8 @@ final class lib_test extends \advanced_testcase { quiz_start_new_attempt($quizobj2a, $quba2a, $attempt, 2, $timenow); quiz_attempt_save_started($quizobj2a, $quba2a, $attempt); $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); $quba2a = \question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj2a->get_context()); $quba2a->set_preferred_behaviour($quizobj2a->get_quiz()->preferredbehaviour); @@ -689,7 +691,8 @@ final class lib_test extends \advanced_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); // Create a calendar event. $event = $this->create_action_event($course->id, $quiz->id, QUIZ_EVENT_TYPE_OPEN); @@ -744,7 +747,8 @@ final class lib_test extends \advanced_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); // Create a calendar event. $event = $this->create_action_event($course->id, $quiz->id, QUIZ_EVENT_TYPE_OPEN); diff --git a/mod/quiz/tests/privacy/provider_test.php b/mod/quiz/tests/privacy/provider_test.php index 369abf1c974..2fa4a9c61a7 100644 --- a/mod/quiz/tests/privacy/provider_test.php +++ b/mod/quiz/tests/privacy/provider_test.php @@ -273,7 +273,8 @@ final class provider_test extends \core_privacy\tests\provider_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); $this->assertTrue($attemptobj->has_response_to_at_least_one_graded_question()); - $attemptobj->process_finish($starttime, false); + $attemptobj->process_submit($starttime, false); + $attemptobj->process_grade_submission($starttime); // Fetch the contexts - no context should be returned. $this->setUser(); @@ -456,7 +457,8 @@ final class provider_test extends \core_privacy\tests\provider_testcase { // Finish the attempt. $attemptobj = quiz_attempt::create($attempt->id); - $attemptobj->process_finish($starttime, false); + $attemptobj->process_submit($starttime, false); + $attemptobj->process_grade_submission($starttime); $this->setUser(); diff --git a/mod/quiz/tests/quiz_notify_attempt_manual_grading_completed_test.php b/mod/quiz/tests/quiz_notify_attempt_manual_grading_completed_test.php index e1f83c3731a..565fcf440c0 100644 --- a/mod/quiz/tests/quiz_notify_attempt_manual_grading_completed_test.php +++ b/mod/quiz/tests/quiz_notify_attempt_manual_grading_completed_test.php @@ -128,7 +128,8 @@ final class quiz_notify_attempt_manual_grading_completed_test extends advanced_t $attemptobj1 = quiz_attempt::create($attempt1->id); $tosubmit = [2 => ['answer' => 'Student 1 answer', 'answerformat' => FORMAT_HTML]]; $attemptobj1->process_submitted_actions($timenow - 30 * MINSECS, false, $tosubmit); - $attemptobj1->process_finish($timenow - 20 * MINSECS, false); + $attemptobj1->process_submit($timenow - 20 * MINSECS, false); + $attemptobj1->process_grade_submission($timenow - 10 * MINSECS); // Finish the attempt of student (now). $attemptobj1->get_question_usage()->manual_grade(2, 'Good!', 1, FORMAT_HTML); @@ -171,7 +172,8 @@ final class quiz_notify_attempt_manual_grading_completed_test extends advanced_t $attemptobj2 = quiz_attempt::create($attempt2->id); $tosubmit = [2 => ['answer' => 'Answer of student 2.', 'answerformat' => FORMAT_HTML]]; $attemptobj2->process_submitted_actions($timenow - 30 * MINSECS, false, $tosubmit); - $attemptobj2->process_finish($timenow, false); + $attemptobj2->process_submit($timenow, false); + $attemptobj2->process_grade_submission($timenow); // After time to notify, except attempt not graded, so it won't appear. $task = new quiz_notify_attempt_manual_grading_completed(); @@ -189,15 +191,19 @@ final class quiz_notify_attempt_manual_grading_completed_test extends advanced_t // Create an attempt for a user without the capability. $timenow = time(); + $timestart = $timenow - HOURSECS; + $timefinish = $timenow - 20 * MINSECS; + $gradetime = $timenow - 10 * MINSECS; $attempt = quiz_create_attempt($this->quizobj, 3, false, $timenow, false, $this->student->id); - quiz_start_new_attempt($this->quizobj, $this->quba, $attempt, 3, $timenow - HOURSECS); - quiz_attempt_save_started($this->quizobj, $this->quba, $attempt); + quiz_start_new_attempt($this->quizobj, $this->quba, $attempt, 3, $timestart); + quiz_attempt_save_started($this->quizobj, $this->quba, $attempt, $timestart); // Process some responses and submit. $attemptobj = quiz_attempt::create($attempt->id); $tosubmit = [2 => ['answer' => 'Essay answer.', 'answerformat' => FORMAT_HTML]]; $attemptobj->process_submitted_actions($timenow - 30 * MINSECS, false, $tosubmit); - $attemptobj->process_finish($timenow - 20 * MINSECS, false); + $attemptobj->process_submit($timefinish, false); + $attemptobj->process_grade_submission($gradetime); // Grade the attempt. $attemptobj->get_question_usage()->manual_grade(2, 'Good!', 1, FORMAT_HTML); @@ -219,7 +225,7 @@ final class quiz_notify_attempt_manual_grading_completed_test extends advanced_t $task->execute(); $attemptobj = quiz_attempt::create($attempt->id); - $this->assertEquals($attemptobj->get_attempt()->timefinish, $attemptobj->get_attempt()->gradednotificationsenttime); + $this->assertEquals($timefinish, $attemptobj->get_attempt()->gradednotificationsenttime); } /** @@ -241,7 +247,8 @@ final class quiz_notify_attempt_manual_grading_completed_test extends advanced_t $attemptobj = quiz_attempt::create($attempt->id); $tosubmit = [2 => ['answer' => 'Answer of student.', 'answerformat' => FORMAT_HTML]]; $attemptobj->process_submitted_actions($timenow - 30 * MINSECS, false, $tosubmit); - $attemptobj->process_finish($timenow - 20 * MINSECS, false); + $attemptobj->process_submit($timenow - 20 * MINSECS, false); + $attemptobj->process_grade_submission($timenow - 10 * MINSECS); // Finish the attempt of student. $attemptobj->get_question_usage()->manual_grade(2, 'Good!', 1, FORMAT_HTML); diff --git a/question/engine/tests/datalib_reporting_queries_test.php b/question/engine/tests/datalib_reporting_queries_test.php index 8e6bb24a3cb..edfe6abd4fd 100644 --- a/question/engine/tests/datalib_reporting_queries_test.php +++ b/question/engine/tests/datalib_reporting_queries_test.php @@ -373,7 +373,8 @@ final class datalib_reporting_queries_test extends \qbehaviour_walkthrough_test_ // Submit attempt. $attemptobj = quiz_attempt::create($attempt->id); $attemptobj->process_submitted_actions($timenow, false); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); // Calculate the statistics. $this->expectOutputRegex('~.*Calculations completed.*~'); diff --git a/question/tests/local/statistics/statistics_bulk_loader_test.php b/question/tests/local/statistics/statistics_bulk_loader_test.php index 785400eac77..d76dd180a73 100644 --- a/question/tests/local/statistics/statistics_bulk_loader_test.php +++ b/question/tests/local/statistics/statistics_bulk_loader_test.php @@ -204,7 +204,8 @@ final class statistics_bulk_loader_test extends advanced_testcase { // Submit attempt. $attemptobj = quiz_attempt::create($attempt->id); $attemptobj->process_submitted_actions($timenow, false, $answers); - $attemptobj->process_finish($timenow, false); + $attemptobj->process_submit($timenow, false); + $attemptobj->process_grade_submission($timenow); } /** From 0b3b0fcce0d612a04bed9e479ae4b340672f83b3 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Thu, 8 Feb 2024 09:04:28 +0000 Subject: [PATCH 2/8] MDL-68806 quiz: Add task to pre-create quiz attempts. --- .upgradenotes/MDL-68806-2024060613482737.yml | 20 + .../backup/moodle2/backup_quiz_stepslib.php | 2 +- mod/quiz/classes/external.php | 11 +- mod/quiz/classes/task/precreate_attempts.php | 155 ++++++ mod/quiz/db/install.xml | 3 +- mod/quiz/db/tasks.php | 9 + mod/quiz/db/upgrade.php | 12 + mod/quiz/lang/en/quiz.php | 31 +- mod/quiz/locallib.php | 9 +- mod/quiz/mod_form.php | 33 ++ mod/quiz/settings.php | 22 + mod/quiz/startattempt.php | 21 +- mod/quiz/tests/backup/restore_date_test.php | 13 +- mod/quiz/tests/behat/attempt_begin.feature | 17 + mod/quiz/tests/behat/behat_mod_quiz.php | 13 + .../settings_form_fields_disableif.feature | 21 + mod/quiz/tests/external/external_test.php | 5 +- .../tests/task/precreate_attempts_test.php | 446 ++++++++++++++++++ mod/quiz/version.php | 2 +- 19 files changed, 823 insertions(+), 22 deletions(-) create mode 100644 .upgradenotes/MDL-68806-2024060613482737.yml create mode 100644 mod/quiz/classes/task/precreate_attempts.php create mode 100644 mod/quiz/tests/task/precreate_attempts_test.php diff --git a/.upgradenotes/MDL-68806-2024060613482737.yml b/.upgradenotes/MDL-68806-2024060613482737.yml new file mode 100644 index 00000000000..3f455f1ca1d --- /dev/null +++ b/.upgradenotes/MDL-68806-2024060613482737.yml @@ -0,0 +1,20 @@ +issueNumber: MDL-68806 +notes: + mod_quiz: + - message: > + New quiz setting "precreateperiod" controls the period before timeopen + during which attempts will be pre-created using the new + + NOT_STARTED state. This setting is marked advanced and locked by + default, so can only be set by administrators. This setting + + is read by the \mod_quiz\task\precreate_attempts task to identify + quizzes due for pre-creation. + type: improved + - message: > + quiz_attempt_save_started Now takes an additional $timenow parameter, to + specify the timestart of the attempt. This was previously + + set in quiz_create_attempt, but is now set in quiz_attempt_save_started + and quiz_attempt_save_not_started. + type: changed diff --git a/mod/quiz/backup/moodle2/backup_quiz_stepslib.php b/mod/quiz/backup/moodle2/backup_quiz_stepslib.php index c3914d17d39..f7dd9ef4343 100644 --- a/mod/quiz/backup/moodle2/backup_quiz_stepslib.php +++ b/mod/quiz/backup/moodle2/backup_quiz_stepslib.php @@ -41,7 +41,7 @@ class backup_quiz_activity_structure_step extends backup_questions_activity_stru 'sumgrades', 'grade', 'timecreated', 'timemodified', 'password', 'subnet', 'browsersecurity', 'delay1', 'delay2', 'showuserpicture', 'showblocks', 'completionattemptsexhausted', - 'completionminattempts', 'allowofflineattempts']); + 'completionminattempts', 'allowofflineattempts', 'precreateattempts']); // Define elements for access rule subplugin settings. $this->add_subplugin_structure('quizaccess', $quiz, true); diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index 685d409632a..3d78bb774df 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -145,7 +145,14 @@ class mod_quiz_external extends external_api { // Fields only for managers. if (has_capability('moodle/course:manageactivities', $context)) { - $additionalfields = ['shuffleanswers', 'timecreated', 'timemodified', 'password', 'subnet']; + $additionalfields = [ + 'shuffleanswers', + 'timecreated', + 'timemodified', + 'password', + 'subnet', + 'precreateattempts', + ]; $viewablefields = array_merge($viewablefields, $additionalfields); } @@ -267,6 +274,8 @@ class mod_quiz_external extends external_api { 'hasfeedback' => new external_value(PARAM_INT, 'Whether the quiz has any non-blank feedback text', VALUE_OPTIONAL), 'hasquestions' => new external_value(PARAM_INT, 'Whether the quiz has questions', VALUE_OPTIONAL), + 'precreateattempts' => new external_value(PARAM_INT, 'Whether attempt pre-creation is enabled', + VALUE_OPTIONAL), ] )) ), diff --git a/mod/quiz/classes/task/precreate_attempts.php b/mod/quiz/classes/task/precreate_attempts.php new file mode 100644 index 00000000000..b2340c5e06a --- /dev/null +++ b/mod/quiz/classes/task/precreate_attempts.php @@ -0,0 +1,155 @@ +. + +namespace mod_quiz\task; + +use core\task\scheduled_task; +use mod_quiz\quiz_settings; +use question_engine; + +/** + * Pre-create attempts for quizzes that have passed their threshold. + * + * @package mod_quiz + * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class precreate_attempts extends scheduled_task { + /** + * Create new instance of the task. + * + * @param int $maxruntime The number of seconds to allow the task to start processing new quizzes. + */ + public function __construct( + /** @var int $maxruntime The number of seconds to allow the task to start processing new quizzes. */ + protected int $maxruntime = 600, + ) { + } + + #[\Override] + public function get_name(): string { + return get_string('precreatetask', 'mod_quiz'); + } + + /** + * Pre-create quiz attempts for configured quizzes. + * + * Find all quizzes with timeopen where the current time is later + * than timeopen-precreateperiod, the quiz has questions, but no attempts. + * + * If the precreateperiod setting is unlocked, also filter by quizzes with precreateattempts enabled. + * + * Find all the users enrolled on the course who can attempt the quiz and create an attempt + * in the NOT_STARTED state. + * + * This will run for $this->maxruntime seconds, then stop to avoid hogging the cron process. Remaining quizzes will be + * processed on subsequent runs. + * + * @return void + */ + public function execute(): void { + global $DB; + $starttime = time(); + $precreateperiod = (int)get_config('quiz', 'precreateperiod'); + $precreatedefault = (int)get_config('quiz', 'precreateattempts'); + if ($precreateperiod === 0) { + mtrace('Pre-creation of quiz attempts is disabled. Nothing to do.'); + return; + } + $sql = " + SELECT DISTINCT q.id, q.name, q.course, q.timeopen + FROM {quiz} q + JOIN {quiz_slots} qs ON q.id = qs.quizid + LEFT JOIN {quiz_attempts} qa ON q.id = qa.quiz + WHERE qa.id IS NULL + AND q.timeopen > :now + AND q.timeopen < :threshold + AND ( + q.precreateattempts = :precreateattempts + OR (1 = :precreatedefault AND q.precreateattempts IS NULL) + ) + ORDER BY q.timeopen ASC"; + $params = [ + 'now' => $starttime, + 'threshold' => $starttime + $precreateperiod, + 'precreateattempts' => 1, + 'precreatedefault' => $precreatedefault, + ]; + + $quizzes = $DB->get_records_sql($sql, $params); + mtrace('Found ' . count($quizzes) . ' quizzes to create attempts for.'); + $quizcount = 0; + foreach ($quizzes as $quiz) { + $transaction = $DB->start_delegated_transaction(); + try { + $quizstart = microtime(true); + mtrace('Creating attempts for ' . $quiz->name); + $attemptcount = self::precreate_attempts_for_quiz($quiz->id, $quiz->course); + $quizend = microtime(true); + $quizduration = round($quizend - $quizstart, 2); + mtrace('Created ' . $attemptcount . ' attempts for ' . $quiz->name . ' in ' . $quizduration . ' seconds'); + $quizcount++; + $transaction->allow_commit(); + } catch (\Throwable $e) { + mtrace('Failed to create attempts for ' . $quiz->name); + $transaction->rollback($e); + } + + if (microtime(true) - $starttime > $this->maxruntime) { + // Stop to let other tasks run, then do some more next run. + mtrace('Time limit reached.'); + break; + } + } + mtrace('Created attempts for ' . $quizcount . ' quizzes.'); + } + + /** + * Pre-create attempts for a quiz. + * + * @param int $quizid + * @param int $courseid + * @return int The number of attempts created. + */ + public static function precreate_attempts_for_quiz(int $quizid, int $courseid): int { + global $DB; + $coursecontext = \context_course::instance($courseid); + $users = get_enrolled_users($coursecontext, 'mod/quiz:attempt'); + $attemptcount = 0; + $timenow = time(); + foreach ($users as $user) { + if ($DB->record_exists('quiz_attempts', ['userid' => $user->id, 'quiz' => $quizid])) { + // Last-minute safety check in case the quiz opened and the user started an attempt since the task started. + continue; + } + $quizobj = quiz_settings::create($quizid, $user->id); + $quba = question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + $attempt = quiz_create_attempt($quizobj, 1, false, $timenow, false, $user->id); + quiz_start_new_attempt( + $quizobj, + $quba, + $attempt, + 1, + $timenow, + ); + quiz_attempt_save_not_started($quba, $attempt); + $attemptcount++; + } + return $attemptcount; + } +} diff --git a/mod/quiz/db/install.xml b/mod/quiz/db/install.xml index 8a7d8ee9f30..3b6aa39662b 100644 --- a/mod/quiz/db/install.xml +++ b/mod/quiz/db/install.xml @@ -1,5 +1,5 @@ - @@ -48,6 +48,7 @@ + diff --git a/mod/quiz/db/tasks.php b/mod/quiz/db/tasks.php index 242562ae21d..c680daa8960 100644 --- a/mod/quiz/db/tasks.php +++ b/mod/quiz/db/tasks.php @@ -53,4 +53,13 @@ $tasks = [ 'month' => '*', 'dayofweek' => '*', ], + [ + 'classname' => 'mod_quiz\task\precreate_attempts', + 'blocking' => 0, + 'minute' => 'R', + 'hour' => '*', + 'day' => '*', + 'dayofweek' => '*', + 'month' => '*', + ], ]; diff --git a/mod/quiz/db/upgrade.php b/mod/quiz/db/upgrade.php index 7e367036194..00984992c08 100644 --- a/mod/quiz/db/upgrade.php +++ b/mod/quiz/db/upgrade.php @@ -120,6 +120,18 @@ function xmldb_quiz_upgrade($oldversion) { // Automatically generated Moodle v4.5.0 release upgrade line. // Put any upgrade step following this. + if ($oldversion < 2025011300) { + // Define field precreateattempts to be added to quiz. + $table = new xmldb_table('quiz'); + $field = new xmldb_field('precreateattempts', XMLDB_TYPE_INTEGER, '1', null, null, null, null, 'allowofflineattempts'); + + // Conditionally launch add field precreateattempts. + if (!$dbman->field_exists($table, $field)) { + $dbman->add_field($table, $field); + } + + upgrade_mod_savepoint(true, 2025011300, 'quiz'); + } return true; } diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index 58e3712431c..85e2446b0e6 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -137,7 +137,7 @@ $string['cachedef_overrides'] = 'User and group override information'; $string['calculated'] = 'Calculated'; $string['calculatedquestion'] = 'Calculated question not supported at line {$a}. The question will be ignored'; $string['cannotcreatepath'] = 'Path cannot be created ({$a})'; -$string['cannoteditafterattempts'] = 'You cannot add or remove questions because this quiz has been attempted. ({$a})'; +$string['cannoteditafterattempts'] = 'You cannot add or remove questions because this quiz has attempts. ({$a})'; $string['cannotfindprevattempt'] = 'Cannot find previous attempt to build on.'; $string['cannotfindquestionregard'] = 'Failed to get questions for regrading!'; $string['cannotinsert'] = 'Cannot insert question'; @@ -721,6 +721,35 @@ $string['pluginname'] = 'Quiz'; $string['popup'] = 'Show quiz in a \'secure\' window'; $string['popupblockerwarning'] = 'This section of the test is in secure mode, this means that you need to take the quiz in a secure window. Please turn off your popup blocker. Thank you.'; $string['popupnotice'] = 'Students will see this quiz in a secure window'; +$string['precreateattempts'] = 'Pre-create attempts'; +$string['precreateattempts_desc'] = 'If enabled, quizzes will have attempts created in advance of the quiz opening time, +based on "Pre-create period" which must also be set. + +This prevents spikes in server load when students start complex or numerous quizzes all at once. + +Quizzes without an opening time are unaffected. + +If this setting is locked, it is forced on or off for all quizzes. +If unlocked, and "Pre-create period" is set, quizzes can opt in to this feature. If "Pre-create" period is not set, this setting +will not be shown on the quiz settings form.'; +$string['precreateattempts_help'] = 'Pre-creating attempts helps optimise site performance. Quiz questions cannot be modified once a quiz has attempts.'; +$string['precreateoff'] = 'Do not pre-create attempts'; +$string['precreateperiod'] = 'Pre-create period'; +$string['precreateperiod_desc'] = 'The period ahead of the quiz open time that attempts will be pre-created. + +Attempts are generated by a scheduled task (mod_quiz\task\precreate_attempts) running each hour by default. + +Since quizzes cannot be edited once they have attempts, it is best to keep this period as short as possible. +If you tend to have one quiz or set of quizzes starting at once time, a short period like 1 hour is sensible. + +If you have several starting at different times through the day, and wish all pre-creation to happen when the system is quiet such +as overnight, you may which to set this to a longer period such as 12 hours and adjust the scheduled task to run during quieter +hours. + +This setting is controlled at site-level, it cannot be overridden by individual quizzes.'; +$string['precreatetask'] = 'Pre-create attempts'; +$string['precreateusedefault'] = 'Use site-wide default ({$a})'; +$string['precreateyes'] = 'Yes, {$a} hours before quiz open time'; $string['preprocesserror'] = 'Error occurred during pre-processing!'; $string['preview'] = 'Preview'; $string['previewquestion'] = 'Preview question'; diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 02a24a9b3e3..3c5bdb31840 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -2004,10 +2004,13 @@ function quiz_validate_new_attempt(quiz_settings $quizobj, access_manager $acces $lastattempt = end($attempts); $attemptnumber = null; - // If an in-progress attempt exists, check password then redirect to it. - if ($lastattempt && ($lastattempt->state == quiz_attempt::IN_PROGRESS || - $lastattempt->state == quiz_attempt::OVERDUE)) { + if ( + $lastattempt + && in_array($lastattempt->state, [quiz_attempt::NOT_STARTED, quiz_attempt::IN_PROGRESS, quiz_attempt::OVERDUE]) + ) { + // If an in-progress or not-started attempt exists, check password then redirect to it. $currentattemptid = $lastattempt->id; + $messages = $accessmanager->prevent_access(); // If the attempt is now overdue, deal with that. diff --git a/mod/quiz/mod_form.php b/mod/quiz/mod_form.php index b9f27329876..264c84298ed 100644 --- a/mod/quiz/mod_form.php +++ b/mod/quiz/mod_form.php @@ -115,6 +115,25 @@ class mod_quiz_mod_form extends moodleform_mod { $mform->addHelpButton('graceperiod', 'graceperiod', 'quiz'); $mform->hideIf('graceperiod', 'overduehandling', 'neq', 'graceperiod'); + // Pre-create attempts. + // This is only shown if "Pre-create period" as been set at site level, and the quiz open time is enabled. + $precreateperiod = get_config('quiz', 'precreateperiod'); + if (!empty($precreateperiod)) { + $yesoption = get_string('precreateyes', 'quiz', $precreateperiod / HOURSECS); + $precreateoptions = [ + 1 => $yesoption, + 0 => get_string('no'), + ]; + $mform->addElement( + 'select', + 'precreateattempts', + get_string('precreateattempts', 'quiz'), + $precreateoptions + ); + $mform->hideIf('precreateattempts', 'timeopen[enabled]'); + $mform->addHelpButton('precreateattempts', 'precreateattempts', 'quiz'); + } + // ------------------------------------------------------------------------------- // Grade settings. $this->standard_grading_coursemodule_elements(); @@ -699,4 +718,18 @@ class mod_quiz_mod_form extends moodleform_mod { return $this->maxattemptsanyoverride; } + + /** + * Return a list of options for site-wide and per-instance precreate period settings. + * + * @return array + */ + public static function generate_precreate_options(): array { + $precreateoptions[0] = get_string('precreateoff', 'quiz'); + for ($i = 1; $i <= 24; $i++) { + $precreateoptions[$i * HOURSECS] = sprintf(get_string('dateintervalhrfull', 'langconfig'), $i); + } + return $precreateoptions; + } + } diff --git a/mod/quiz/settings.php b/mod/quiz/settings.php index 94f3f080b90..bd19402cd9e 100644 --- a/mod/quiz/settings.php +++ b/mod/quiz/settings.php @@ -87,6 +87,28 @@ if ($ADMIN->fulltree) { $setting->set_locked_flag_options(admin_setting_flag::ENABLED, false); $quizsettings->add($setting); + // Enable pre-creation of attempts. + $setting = new admin_setting_configcheckbox( + 'quiz/precreateattempts', + get_string('precreateattempts', 'quiz'), + get_string('precreateattempts_help', 'quiz'), + 0, + ); + $setting->set_locked_flag_options(admin_setting_flag::ENABLED, true); + $setting->set_advanced_flag_options(admin_setting_flag::ENABLED, true); + $quizsettings->add($setting); + + // Pre-create attempt period. + $precreateoptions = mod_quiz_mod_form::generate_precreate_options(); + $setting = new admin_setting_configselect( + 'quiz/precreateperiod', + get_string('precreateperiod', 'quiz'), + get_string('precreateperiod_desc', 'quiz'), + 0, + $precreateoptions, + ); + $quizsettings->add($setting); + // Minimum grace period used behind the scenes. $quizsettings->add(new admin_setting_configduration('quiz/graceperiodmin', get_string('graceperiodmin', 'quiz'), get_string('graceperiodmin_desc', 'quiz'), diff --git a/mod/quiz/startattempt.php b/mod/quiz/startattempt.php index 2e3f8367656..81f0f809b1c 100644 --- a/mod/quiz/startattempt.php +++ b/mod/quiz/startattempt.php @@ -99,15 +99,14 @@ if ($accessmanager->is_preflight_check_required($currentattemptid)) { // Pre-flight check passed. $accessmanager->notify_preflight_check_passed($currentattemptid); } -if ($currentattemptid) { - if ($lastattempt->state == quiz_attempt::OVERDUE) { - redirect($quizobj->summary_url($lastattempt->id)); - } else { - redirect($quizobj->attempt_url($currentattemptid, $page)); - } + +if (!$currentattemptid || $lastattempt->state == quiz_attempt::NOT_STARTED) { + $attempt = quiz_prepare_and_start_new_attempt($quizobj, $attemptnumber, $lastattempt); +} else { + $attempt = $lastattempt; +} +if ($attempt->state === quiz_attempt::OVERDUE) { + redirect($quizobj->summary_url($attempt->id)); +} else { + redirect($quizobj->attempt_url($attempt->id, $page)); } - -$attempt = quiz_prepare_and_start_new_attempt($quizobj, $attemptnumber, $lastattempt); - -// Redirect to the attempt page. -redirect($quizobj->attempt_url($attempt->id, $page)); diff --git a/mod/quiz/tests/backup/restore_date_test.php b/mod/quiz/tests/backup/restore_date_test.php index 65262a33667..705b84b4105 100644 --- a/mod/quiz/tests/backup/restore_date_test.php +++ b/mod/quiz/tests/backup/restore_date_test.php @@ -37,8 +37,16 @@ final class restore_date_test extends \restore_date_testcase { global $DB, $USER; // Create quiz data. - $record = ['timeopen' => 100, 'timeclose' => 100, 'timemodified' => 100, 'tiemcreated' => 100, 'questionsperpage' => 0, - 'grade' => 100.0, 'sumgrades' => 2]; + $record = [ + 'timeopen' => 100, + 'timeclose' => 100, + 'timemodified' => 100, + 'timecreated' => 100, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + 'precreateattempts' => 1, + ]; list($course, $quiz) = $this->create_course_and_module('quiz', $record); // Create questions. @@ -86,6 +94,7 @@ final class restore_date_test extends \restore_date_testcase { $this->assertFieldsNotRolledForward($quiz, $newquiz, ['timecreated', 'timemodified']); $props = ['timeclose', 'timeopen']; $this->assertFieldsRolledForward($quiz, $newquiz, $props); + $this->assertEquals($quiz->precreateattempts, $newquiz->precreateattempts); $newattempt = $DB->get_record('quiz_attempts', ['quiz' => $newquiz->id]); $newoverride = $DB->get_record('quiz_overrides', ['quiz' => $newquiz->id]); diff --git a/mod/quiz/tests/behat/attempt_begin.feature b/mod/quiz/tests/behat/attempt_begin.feature index 42275c1c066..1d36b01a24a 100644 --- a/mod/quiz/tests/behat/attempt_begin.feature +++ b/mod/quiz/tests/behat/attempt_begin.feature @@ -124,3 +124,20 @@ Feature: The various checks that may happen when an attept is started And I press the "back" button in the browser Then a new page should have loaded since I started watching And I should see "Continue your attempt" + + @javascript + Scenario: Start a quiz with pre-created attempts + Given the following config values are set as admin: + | precreateperiod | 1 | quiz | + Given the following "activities" exist: + | activity | name | intro | course | idnumber | timeopen | timelimit | quizpassword | attempts | + | quiz | Quiz 1 | Quiz 1 description | C1 | quiz1 | ## now ## | 3600 | Frog | 1 | + And quiz "Quiz 1" contains the following questions: + | question | page | + | TF1 | 1 | + And quiz "Quiz 1" has pre-created attempts + When I am on the "Quiz 1" "mod_quiz > View" page logged in as "student" + And I press "Attempt quiz" + And I set the field "Quiz password" to "Frog" + And I press "Start attempt" + Then I should see "Text of the first question" diff --git a/mod/quiz/tests/behat/behat_mod_quiz.php b/mod/quiz/tests/behat/behat_mod_quiz.php index c21820d7953..c23c318df0f 100644 --- a/mod/quiz/tests/behat/behat_mod_quiz.php +++ b/mod/quiz/tests/behat/behat_mod_quiz.php @@ -1057,4 +1057,17 @@ class behat_mod_quiz extends behat_question_base { ["//li[contains(@class,'qtype')]//span[@class='slotnumber' and contains(., %locator%)]/.."]) ]; } + + /** + * Generate pre-created attempts for a quiz. + * + * @param string $quizname the name of the quiz to create attempts for. + * @Given quiz :quizname has pre-created attempts + */ + public function quiz_has_precreated_attempts(string $quizname): void { + global $DB; + + $quiz = $DB->get_record('quiz', ['name' => $quizname], 'id, course', MUST_EXIST); + \mod_quiz\task\precreate_attempts::precreate_attempts_for_quiz($quiz->id, $quiz->course); + } } diff --git a/mod/quiz/tests/behat/settings_form_fields_disableif.feature b/mod/quiz/tests/behat/settings_form_fields_disableif.feature index cd4fadaba3d..08185ce0136 100644 --- a/mod/quiz/tests/behat/settings_form_fields_disableif.feature +++ b/mod/quiz/tests/behat/settings_form_fields_disableif.feature @@ -147,3 +147,24 @@ Feature: Settings form fields disabled if not required And I am on the "Quiz 1" "quiz activity editing" page And I expand all fieldsets And I should not see "Repaginate now" + + @javascript + Scenario Outline: Pre-create attempts setting is only shown if precreateperiod is set and timeopen is enabled. + Given the following config values are set as admin: + | precreateattempts | 1 | quiz | + | precreateattempts_locked | 0 | quiz | + | precreateperiod | | quiz | + And I log in as "admin" + When I add a "quiz" activity to course "Course 1" section 1 + And I click on "Timing" "link" + And I set the field "timeopen[enabled]" to "" + And I expand all fieldsets + Then I see "Pre-create attempts" + And "Pre-create attempts" "select" be visible + And I see "Yes, 1 hours before quiz open time" + + Examples: + | period | timeopen | exists | + | 3600 | 1 | should | + | 3600 | 0 | should not | + | 0 | 1 | should not | diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index 5dccf215295..9013688d99f 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -229,6 +229,9 @@ final class external_test extends externallib_advanced_testcase { $record = new \stdClass(); $record->course = $course2->id; $record->intro = ''; + $timeopen = time() - 1; + $record->timeopen = $timeopen; + $record->precreateattempts = 1; $quiz2 = self::getDataGenerator()->create_module('quiz', $record); // Execute real Moodle enrolment as we'll call unenrol() method on the instance later. @@ -258,7 +261,7 @@ final class external_test extends externallib_advanced_testcase { 'browsersecurity', 'delay1', 'delay2', 'showuserpicture', 'showblocks', 'completionattemptsexhausted', 'completionpass', 'autosaveperiod', 'hasquestions', 'overduehandling', 'graceperiod', 'canredoquestions', 'allowofflineattempts']; - $managerfields = ['shuffleanswers', 'timecreated', 'timemodified', 'password', 'subnet']; + $managerfields = ['shuffleanswers', 'timecreated', 'timemodified', 'password', 'subnet', 'precreateattempts']; // Add expected coursemodule and other data. $quiz1 = $this->quiz; diff --git a/mod/quiz/tests/task/precreate_attempts_test.php b/mod/quiz/tests/task/precreate_attempts_test.php new file mode 100644 index 00000000000..ee6d06b23a3 --- /dev/null +++ b/mod/quiz/tests/task/precreate_attempts_test.php @@ -0,0 +1,446 @@ +. + +namespace mod_quiz\task; + +use mod_quiz\quiz_attempt; +use mod_quiz\quiz_settings; + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot . '/mod/quiz/tests/quiz_question_helper_test_trait.php'); + +/** + * Unit tests for precreate_attempts + * + * @package mod_quiz + * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \mod_quiz\task\precreate_attempts + */ +final class precreate_attempts_test extends \advanced_testcase { + use \quiz_question_helper_test_trait; + + /** + * Generate the various possible combinations precreation settings and the corresponding task output. + * + * @return array + */ + public static function precreate_settings_provider(): array { + return [ + [ + 'period' => 0, + 'output' => 'Pre-creation of quiz attempts is disabled. Nothing to do.', + ], + [ + 'period' => 1, + 'output' => 'Found 0 quizzes to create attempts for.', + ], + ]; + } + + /** + * The scheduled task only looks for quizzes to generate if pre-creation is configured. + * + * Only look for quizzes to generate attempts for if precreateperiod is not 0, and precreateattempts is 1 or is unlocked. + * + * @param int $period + * @param string $output + * @dataProvider precreate_settings_provider + */ + public function test_execute_disabled(int $period, string $output): void { + $this->resetAfterTest(); + set_config('precreateperiod', $period, 'quiz'); + + $task = new precreate_attempts(); + ob_start(); + $task->execute(); + $log = ob_get_clean(); + $this->assertMatchesRegularExpression("/{$output}/", $log); + } + + /** + * Test precreate attempts task. + * + * Generate quizzes with a variety of timeopen and precreateperiod settings, and ensure those that match the criteria + * for attempt pre-generation are picked up by the scheduled task. + */ + public function test_execute(): void { + $this->resetAfterTest(); + + // Generate a course. + $course = $this->getDataGenerator()->create_course(); + // Generate 3 users. + $student1 = $this->getDataGenerator()->create_user(); + $student2 = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + // Enrol users on the course with the appropriate roles. + $this->getDataGenerator()->enrol_user($student1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($student2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, 'editingteacher'); + + set_config('precreateperiod', 12 * HOURSECS, 'quiz'); + set_config('precreateattempts', 1, 'quiz'); + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + // Generate a quiz with timeopen 1 day in the future. + $quizinfuture = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() + 86400, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // Generate a quiz with timeopen 0. + $quiznotimeopen = $quizgenerator->create_instance([ + 'course' => $course->id, + 'precreateperiod' => 43200, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // Generate a quiz with timeopen in the past. + $quizinpast = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() - 86400, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // Generate a quiz with timeopen 11 hours in the future. + $quizwithattempts = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // Generate second quiz with timeopen 11 hours in the future. + $quizinprecreateperiod = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // Generate second quiz with timeopen 11 hours in the future, + // but do not give it any questions. + $quizwithoutquestions = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // Add questions to the quizzes. + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $this->add_two_regular_questions($questiongenerator, $quizinfuture); + $this->add_two_regular_questions($questiongenerator, $quiznotimeopen); + $this->add_two_regular_questions($questiongenerator, $quizinpast); + $this->add_two_regular_questions($questiongenerator, $quizwithattempts); + $this->add_two_regular_questions($questiongenerator, $quizinprecreateperiod); + + // Create attempts for one student on quiz 5. + $quiz5settings = quiz_settings::create($quizwithattempts->id); + $quba = \question_engine::make_questions_usage_by_activity('mod_quiz', $quiz5settings->get_context()); + $quba->set_preferred_behaviour($quiz5settings->get_quiz()->preferredbehaviour); + $attempt = quiz_create_attempt($quiz5settings, 1, false, time(), false, $student1->id); + quiz_start_new_attempt($quiz5settings, $quba, $attempt, 1, time()); + quiz_attempt_save_started($quiz5settings, $quba, $attempt); + + $this->assertEmpty( + quiz_get_user_attempts( + [ + $quizinfuture->id, + $quiznotimeopen->id, + $quizinpast->id, + $quizwithoutquestions->id, + ], + $student1->id, + 'all' + ) + ); + $this->assertEmpty( + quiz_get_user_attempts( + [ + $quizinfuture->id, + $quiznotimeopen->id, + $quizinpast->id, + $quizwithattempts->id, + $quizwithoutquestions->id, + ], + $student2->id, + 'all', + ), + ); + $this->assertEmpty( + quiz_get_user_attempts( + [ + $quizinfuture->id, + $quiznotimeopen->id, + $quizinpast->id, + $quizwithattempts->id, + $quizinprecreateperiod->id, + $quizwithoutquestions->id, + ], + $teacher->id, + 'all', + ), + ); + + $student1existingattempts = quiz_get_user_attempts($quizwithattempts->id, $student1->id, 'all'); + $this->assertCount(1, $student1existingattempts); + $this->assertEquals(reset($student1existingattempts)->state, quiz_attempt::IN_PROGRESS); + $student1precreatedattempts = quiz_get_user_attempts($quizinprecreateperiod->id, $student1->id, 'all'); + $this->assertEmpty($student1precreatedattempts); + $student2precreatedattempts = quiz_get_user_attempts($quizinprecreateperiod->id, $student2->id, 'all'); + $this->assertEmpty($student2precreatedattempts); + + $task = new precreate_attempts(); + ob_start(); + $task->execute(); + $log = ob_get_clean(); + + $this->assertMatchesRegularExpression('/Found 1 quizzes to create attempts for/', $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quizinfuture->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quiznotimeopen->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quizinpast->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quizwithattempts->name}/", $log); + $this->assertMatchesRegularExpression("/Creating attempts for {$quizinprecreateperiod->name}/", $log); + $this->assertMatchesRegularExpression("/Created 2 attempts for {$quizinprecreateperiod->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quizwithoutquestions->name}/", $log); + $this->assertMatchesRegularExpression('/Created attempts for 1 quizzes./', $log); + + // Students should have no attempts on quizzes that didn't meet criteria for pre-creation. + $this->assertEmpty( + quiz_get_user_attempts( + [ + $quizinfuture->id, + $quiznotimeopen->id, + $quizinpast->id, + $quizwithoutquestions->id, + ], + $student1->id, + 'all' + ) + ); + $this->assertEmpty( + quiz_get_user_attempts( + [ + $quizinfuture->id, + $quiznotimeopen->id, + $quizinpast->id, + $quizwithattempts->id, + $quizwithoutquestions->id, + ], + $student2->id, + 'all', + ), + ); + // Teacher should not have any attempts on any quizzes. + $this->assertEmpty( + quiz_get_user_attempts( + [ + $quizinfuture->id, + $quiznotimeopen->id, + $quizinpast->id, + $quizwithattempts->id, + $quizinprecreateperiod->id, + $quizwithoutquestions->id, + ], + $teacher->id, + 'all', + ), + ); + + // Students existing attempts should remain. + $student1existingattempts = quiz_get_user_attempts($quizwithattempts->id, $student1->id, 'all'); + $this->assertCount(1, $student1existingattempts); + $this->assertEquals(reset($student1existingattempts)->state, quiz_attempt::IN_PROGRESS); + // They should have NOT_STARTED attempts on quizzes that meet the criteria for pre-creation. + $student1precreatedattempts = quiz_get_user_attempts($quizinprecreateperiod->id, $student1->id, 'all'); + $this->assertCount(1, $student1precreatedattempts); + $this->assertEquals(reset($student1precreatedattempts)->state, quiz_attempt::NOT_STARTED); + $student2precreatedattempts = quiz_get_user_attempts($quizinprecreateperiod->id, $student2->id, 'all'); + $this->assertCount(1, $student2precreatedattempts); + $this->assertEquals(reset($student1precreatedattempts)->state, quiz_attempt::NOT_STARTED); + } + + /** + * Processing should stop at the end of a quiz once maxruntime has been reached. + * + * @return void + */ + public function test_execute_maxruntime(): void { + $this->resetAfterTest(); + + // Generate a course. + $course = $this->getDataGenerator()->create_course(); + // Generate 3 users. + $student1 = $this->getDataGenerator()->create_user(); + // Enrol users on the course with the appropriate roles. + $this->getDataGenerator()->enrol_user($student1->id, $course->id, 'student'); + + set_config('precreateperiod', 12 * HOURSECS, 'quiz'); + set_config('precreateattempts', 1, 'quiz'); + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + // Generate 3 quizzes within the pre-creation window. + $timenow = time(); + $quiz1 = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => $timenow + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // This quiz opens first, so should be processed first. + $quiz2 = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => $timenow + 39599, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + $quiz3 = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => $timenow + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + // Add questions to the quizzes. + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $this->add_two_regular_questions($questiongenerator, $quiz1); + $this->add_two_regular_questions($questiongenerator, $quiz2); + $this->add_two_regular_questions($questiongenerator, $quiz3); + + // Run the task with a maxruntime of 0, so that it should stop after processing the first quiz. + $task = new precreate_attempts(0); + ob_start(); + $task->execute(); + $log = ob_get_clean(); + + // Verify that the task stopped after the quiz opening soonest. + $this->assertMatchesRegularExpression('/Found 3 quizzes to create attempts for/', $log); + $this->assertMatchesRegularExpression("/Creating attempts for {$quiz2->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quiz1->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quiz3->name}/", $log); + $this->assertMatchesRegularExpression("/Created 1 attempts for {$quiz2->name}/", $log); + $this->assertMatchesRegularExpression('/Time limit reached./', $log); + $this->assertMatchesRegularExpression('/Created attempts for 1 quizzes./', $log); + + // Run the task again with the default maxruntime. + ob_start(); + $task = new precreate_attempts(); + $task->execute(); + $log = ob_get_clean(); + + // Verify that it picks up the remaining quiz for processing. + $this->assertMatchesRegularExpression('/Found 2 quizzes to create attempts for/', $log); + $this->assertMatchesRegularExpression("/Creating attempts for {$quiz1->name}/", $log); + $this->assertMatchesRegularExpression("/Creating attempts for {$quiz3->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quiz2->name}/", $log); + $this->assertMatchesRegularExpression("/Created 1 attempts for {$quiz1->name}/", $log); + $this->assertMatchesRegularExpression("/Created 1 attempts for {$quiz3->name}/", $log); + $this->assertDoesNotMatchRegularExpression('/Time limit reached./', $log); + $this->assertMatchesRegularExpression('/Created attempts for 2 quizzes./', $log); + } + + /** + * Pre-creation is opt in based on quiz setting. + * + * @return void + */ + public function test_execute_optin(): void { + $this->resetAfterTest(); + + // Generate a course. + $course = $this->getDataGenerator()->create_course(); + // Generate 3 users. + $student1 = $this->getDataGenerator()->create_user(); + $student2 = $this->getDataGenerator()->create_user(); + $teacher = $this->getDataGenerator()->create_user(); + // Enrol users on the course with the appropriate roles. + $this->getDataGenerator()->enrol_user($student1->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($student2->id, $course->id, 'student'); + $this->getDataGenerator()->enrol_user($teacher->id, $course->id, 'editingteacher'); + + // Precreation is disabled by default. + set_config('precreateperiod', 12 * HOURSECS, 'quiz'); + set_config('precreateattempts', 0, 'quiz'); + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + // Generate a quiz with timeopen 11 hours in the future, and precreateattempts set to 1. + $quizprecreate = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + 'precreateattempts' => 1, + ]); + // Generate a quiz with timeopen 11 hours in the future, and precreateattempts set to 0. + $quiznoprecreate = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + 'precreateattempts' => 0, + ]); + // Generate a quiz with timeopen 11 hours in the future, and precreateattempts set to null. + $quizprecreatenull = $quizgenerator->create_instance([ + 'course' => $course->id, + 'timeopen' => time() + 39600, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 2, + ]); + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + $this->add_two_regular_questions($questiongenerator, $quizprecreate); + $this->add_two_regular_questions($questiongenerator, $quiznoprecreate); + $this->add_two_regular_questions($questiongenerator, $quizprecreatenull); + + // Run the task. + ob_start(); + $task = new precreate_attempts(); + $task->execute(); + $log = ob_get_clean(); + + // Attempts were now created for the opted-in quiz. + $this->assertMatchesRegularExpression('/Found 1 quizzes to create attempts for/', $log); + $this->assertMatchesRegularExpression("/Creating attempts for {$quizprecreate->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quiznoprecreate->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quizprecreatenull->name}/", $log); + $this->assertMatchesRegularExpression('/Created attempts for 1 quizzes./', $log); + + // Now enabled by default. + set_config('precreateattempts', 1, 'quiz'); + + // Run the task again. + ob_start(); + $task = new precreate_attempts(); + $task->execute(); + $log = ob_get_clean(); + + // The quiz with null now has attempts generated. + $this->assertMatchesRegularExpression('/Found 1 quizzes to create attempts for/', $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quizprecreate->name}/", $log); + $this->assertDoesNotMatchRegularExpression("/Creating attempts for {$quiznoprecreate->name}/", $log); + $this->assertMatchesRegularExpression("/Creating attempts for {$quizprecreatenull->name}/", $log); + $this->assertMatchesRegularExpression('/Created attempts for 1 quizzes./', $log); + } +} diff --git a/mod/quiz/version.php b/mod/quiz/version.php index cc3d953e0ec..135a8b05ff7 100644 --- a/mod/quiz/version.php +++ b/mod/quiz/version.php @@ -24,6 +24,6 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2024121800; +$plugin->version = 2025011300; $plugin->requires = 2024100100; $plugin->component = 'mod_quiz'; From 2e2e3adbbd600e7d9d154b8d499c414d50bea1e9 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 13 Feb 2024 11:50:45 +0000 Subject: [PATCH 3/8] MDL-68806 quiz: Display new attempt states on the attempts report --- .../local/reports/attempts_report_options.php | 12 ++++- .../reports/attempts_report_options_form.php | 22 +++++++- .../local/reports/attempts_report_table.php | 2 +- mod/quiz/lang/en/quiz.php | 18 +++++-- mod/quiz/locallib.php | 4 ++ .../tests/behat/attempt_states.feature | 52 +++++++++++++++++++ .../report/overview/tests/behat/basic.feature | 6 +-- .../responses/tests/behat/basic.feature | 6 +-- 8 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 mod/quiz/report/overview/tests/behat/attempt_states.feature diff --git a/mod/quiz/classes/local/reports/attempts_report_options.php b/mod/quiz/classes/local/reports/attempts_report_options.php index 8f55cbdcb32..bd9f2ab2e9e 100644 --- a/mod/quiz/classes/local/reports/attempts_report_options.php +++ b/mod/quiz/classes/local/reports/attempts_report_options.php @@ -46,8 +46,10 @@ class attempts_report_options { * @var array form field name => corresponding quiz_attempt:: state constant. */ protected static $statefields = [ + 'statenotstarted' => quiz_attempt::NOT_STARTED, 'stateinprogress' => quiz_attempt::IN_PROGRESS, 'stateoverdue' => quiz_attempt::OVERDUE, + 'statesubmitted' => quiz_attempt::SUBMITTED, 'statefinished' => quiz_attempt::FINISHED, 'stateabandoned' => quiz_attempt::ABANDONED, ]; @@ -65,8 +67,14 @@ class attempts_report_options { * @var array|null of quiz_attempt::IN_PROGRESS, etc. constants. null means * no restriction. */ - public $states = [quiz_attempt::IN_PROGRESS, quiz_attempt::OVERDUE, - quiz_attempt::FINISHED, quiz_attempt::ABANDONED]; + public $states = [ + quiz_attempt::NOT_STARTED, + quiz_attempt::IN_PROGRESS, + quiz_attempt::OVERDUE, + quiz_attempt::SUBMITTED, + quiz_attempt::FINISHED, + quiz_attempt::ABANDONED, + ]; /** * @var bool whether to show all finished attmepts, or just the one that gave diff --git a/mod/quiz/classes/local/reports/attempts_report_options_form.php b/mod/quiz/classes/local/reports/attempts_report_options_form.php index dfc3993fc9a..caeadd853a4 100644 --- a/mod/quiz/classes/local/reports/attempts_report_options_form.php +++ b/mod/quiz/classes/local/reports/attempts_report_options_form.php @@ -66,10 +66,14 @@ abstract class attempts_report_options_form extends \moodleform { ]); $stategroup = [ + $mform->createElement('advcheckbox', 'statenotstarted', '', + get_string('statenotstarted', 'quiz')), $mform->createElement('advcheckbox', 'stateinprogress', '', get_string('stateinprogress', 'quiz')), $mform->createElement('advcheckbox', 'stateoverdue', '', get_string('stateoverdue', 'quiz')), + $mform->createElement('advcheckbox', 'statesubmitted', '', + get_string('statesubmitted', 'quiz')), $mform->createElement('advcheckbox', 'statefinished', '', get_string('statefinished', 'quiz')), $mform->createElement('advcheckbox', 'stateabandoned', '', @@ -77,12 +81,17 @@ abstract class attempts_report_options_form extends \moodleform { ]; $mform->addGroup($stategroup, 'stateoptions', get_string('reportattemptsthatare', 'quiz'), [' '], false); + $mform->addHelpButton('stateoptions', 'stateoptions', 'quiz'); + $mform->setDefault('statenotstarted', 1); $mform->setDefault('stateinprogress', 1); $mform->setDefault('stateoverdue', 1); + $mform->setDefault('statesubmitted', 1); $mform->setDefault('statefinished', 1); $mform->setDefault('stateabandoned', 1); + $mform->disabledIf('statenotstarted', 'attempts', 'eq', attempts_report::ENROLLED_WITHOUT); $mform->disabledIf('stateinprogress', 'attempts', 'eq', attempts_report::ENROLLED_WITHOUT); $mform->disabledIf('stateoverdue', 'attempts', 'eq', attempts_report::ENROLLED_WITHOUT); + $mform->disabledIf('statesubmitted', 'attempts', 'eq', attempts_report::ENROLLED_WITHOUT); $mform->disabledIf('statefinished', 'attempts', 'eq', attempts_report::ENROLLED_WITHOUT); $mform->disabledIf('stateabandoned', 'attempts', 'eq', attempts_report::ENROLLED_WITHOUT); @@ -126,8 +135,17 @@ abstract class attempts_report_options_form extends \moodleform { public function validation($data, $files) { $errors = parent::validation($data, $files); - if ($data['attempts'] != attempts_report::ENROLLED_WITHOUT && !( - $data['stateinprogress'] || $data['stateoverdue'] || $data['statefinished'] || $data['stateabandoned'])) { + if ( + $data['attempts'] != attempts_report::ENROLLED_WITHOUT && + !( + $data['stateinprogress'] + || $data['stateoverdue'] + || $data['statefinished'] + || $data['stateabandoned'] + || $data['statenotstarted'] + || $data['statesubmitted'] + ) + ) { $errors['stateoptions'] = get_string('reportmustselectstate', 'quiz'); } diff --git a/mod/quiz/classes/local/reports/attempts_report_table.php b/mod/quiz/classes/local/reports/attempts_report_table.php index eb15fff95b4..18985b9217a 100644 --- a/mod/quiz/classes/local/reports/attempts_report_table.php +++ b/mod/quiz/classes/local/reports/attempts_report_table.php @@ -229,7 +229,7 @@ abstract class attempts_report_table extends \table_sql { * @return string HTML content to go inside the td. */ public function col_timestart($attempt) { - if ($attempt->attempt) { + if ($attempt->attempt && $attempt->timestart) { return userdate($attempt->timestart, $this->strtimeformat); } else { return '-'; diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index 85e2446b0e6..0e07c060130 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -951,10 +951,10 @@ $string['reports'] = 'Reports'; $string['reportshowonly'] = 'Show only attempts'; $string['reportshowonlyfinished'] = 'Show at most one finished attempt per user ({$a})'; $string['reportsimplestat'] = 'Simple statistics'; -$string['reportusersall'] = 'all users who have attempted the quiz'; -$string['reportuserswith'] = 'enrolled users who have attempted the quiz'; -$string['reportuserswithorwithout'] = 'enrolled users who have, or have not, attempted the quiz'; -$string['reportuserswithout'] = 'enrolled users who have not attempted the quiz'; +$string['reportusersall'] = 'all users who have a quiz attempt'; +$string['reportuserswith'] = 'enrolled users who have a quiz attempt'; +$string['reportuserswithorwithout'] = 'enrolled users who have, or do not have, a quiz attempt'; +$string['reportuserswithout'] = 'enrolled users who do not have a quiz attempt'; $string['reportwhattoinclude'] = 'What to include in the report'; $string['requirepassword'] = 'Require password'; $string['requirepassword_help'] = 'If a password is specified, a student must enter it in order to attempt the quiz.'; @@ -1081,8 +1081,18 @@ $string['statefinished'] = 'Finished'; $string['statefinisheddetails'] = 'Submitted {$a}'; $string['stateinprogress'] = 'In progress'; $string['statenotloaded'] = 'The state for question {$a} has not been loaded from the database'; +$string['statenotstarted'] = 'Not started'; +$string['stateoptions'] = 'Attempt state options'; +$string['stateoptions_help'] = ' +* Not started: The attempt was automatically created before the quiz opened. The student has not started the attempt yet. +* In progress: The student has started the attempt. They still have time to submit it. +* Overdue: The attempt has been open for longer than the allowed time limit. The student can still submit it within the grade period. +* Submitted: The student has completed the attempt and submitted their responses. It is queued for automatic marking. +* Finished: The attempt has been submitted and any automatic marking is complete. +* Never submitted: The student started the attempt, but did not submit it within the time limit or grace period.'; $string['stateoverdue'] = 'Overdue'; $string['stateoverduedetails'] = 'Must be submitted by {$a}'; +$string['statesubmitted'] = 'Submitted'; $string['status'] = 'Status'; $string['stoponerror'] = 'Stop on error'; $string['submission_confirmation'] = 'Submit all your answers and finish?'; diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 3c5bdb31840..320866827de 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -1006,10 +1006,14 @@ function quiz_questions_per_page_options() { */ function quiz_attempt_state_name($state) { switch ($state) { + case quiz_attempt::NOT_STARTED: + return get_string('statenotstarted', 'quiz'); case quiz_attempt::IN_PROGRESS: return get_string('stateinprogress', 'quiz'); case quiz_attempt::OVERDUE: return get_string('stateoverdue', 'quiz'); + case quiz_attempt::SUBMITTED: + return get_string('statesubmitted', 'quiz'); case quiz_attempt::FINISHED: return get_string('statefinished', 'quiz'); case quiz_attempt::ABANDONED: diff --git a/mod/quiz/report/overview/tests/behat/attempt_states.feature b/mod/quiz/report/overview/tests/behat/attempt_states.feature new file mode 100644 index 00000000000..7f658a26ab0 --- /dev/null +++ b/mod/quiz/report/overview/tests/behat/attempt_states.feature @@ -0,0 +1,52 @@ +@mod @mod_quiz @quiz @quiz_overview +Feature: View attempt states + In order to see how students are progressing through the quiz + As a teacher + I need to see different attempt states on the overview report + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + | student3 | Student | 3 | student3@example.com | + And the following "courses" exist: + | fullname | shortname | category | + | Course 1 | C1 | 0 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + | student3 | C1 | student | + And the following "question categories" exist: + | contextlevel | reference | name | + | Course | C1 | Test questions | + And the following "activities" exist: + | activity | name | intro | course | idnumber | + | quiz | Quiz 1 | Quiz 1 description | C1 | quiz1 | + And the following "questions" exist: + | questioncategory | qtype | name | questiontext | + | Test questions | description | Intro | Welcome to this quiz | + | Test questions | truefalse | TF1 | First question | + | Test questions | truefalse | TF2 | Second question | + And quiz "Quiz 1" contains the following questions: + | question | page | maxmark | + | Intro | 1 | | + | TF1 | 1 | | + | TF2 | 1 | 3.0 | + + Scenario: View attempts in different states + Given quiz "Quiz 1" has pre-created attempts + And user "student1" has started an attempt at quiz "Quiz 1" + And user "student2" has attempted "Quiz 1" with responses: + | slot | response | + | 2 | True | + | 3 | False | + When I am on the "Quiz 1" "mod_quiz > Grades report" page logged in as "teacher 1" + Then the following should exist in the "attempts" table: + | Email address | Status | Started | Completed | Grade/100.00 | + | student1@example.com | In progress | ## now ##%d %B %Y %I:%M %p## | - | - | + | student2@example.com | Finished | ## now ##%d %B %Y %I:%M %p## | ## now ##%d %B %Y %I:%M %p## | 25.00 | + | student3@example.com | Not started | - | - | - | diff --git a/mod/quiz/report/overview/tests/behat/basic.feature b/mod/quiz/report/overview/tests/behat/basic.feature index 868eb5d4615..c5d5dcac94a 100644 --- a/mod/quiz/report/overview/tests/behat/basic.feature +++ b/mod/quiz/report/overview/tests/behat/basic.feature @@ -77,13 +77,13 @@ Feature: Basic use of the Grades report And I should see "100.00" in the "S2 Student2" "table_row" # Check changing the form parameters - And I set the field "Attempts from" to "enrolled users who have not attempted the quiz" + And I set the field "Attempts from" to "enrolled users who do not have a quiz attempt" And I press "Show report" # Note: teachers should not appear in the report. # Check student3's grade And I should see "-" in the "S3 Student3" "table_row" - And I set the field "Attempts from" to "enrolled users who have, or have not, attempted the quiz" + And I set the field "Attempts from" to "enrolled users who have, or do not have, a quiz attempt" And I press "Show report" # Check student1's grade And I should see "25.00" in the "S1 Student1" "table_row" @@ -92,7 +92,7 @@ Feature: Basic use of the Grades report # Check student3's grade And I should see "-" in the "S3 Student3" "table_row" - And I set the field "Attempts from" to "all users who have attempted the quiz" + And I set the field "Attempts from" to "all users who have a quiz attempt" And I press "Show report" # Check student1's grade And I should see "25.00" in the "S1 Student1" "table_row" diff --git a/mod/quiz/report/responses/tests/behat/basic.feature b/mod/quiz/report/responses/tests/behat/basic.feature index c9524946158..50b822b3430 100644 --- a/mod/quiz/report/responses/tests/behat/basic.feature +++ b/mod/quiz/report/responses/tests/behat/basic.feature @@ -36,7 +36,7 @@ Feature: Basic use of the Responses report When I am on the "Quiz 1" "mod_quiz > Responses report" page logged in as teacher Then I should see "Attempts: 0" And I should see "Nothing to display" - And I set the field "Attempts from" to "enrolled users who have not attempted the quiz" + And I set the field "Attempts from" to "enrolled users who have a quiz attempt" @javascript Scenario: Report works when there are attempts @@ -56,7 +56,7 @@ Feature: Basic use of the Responses report Then I should see "Attempts: 1" And I should see "Student One" And I should not see "Student Two" - And I set the field "Attempts from" to "enrolled users who have, or have not, attempted the quiz" + And I set the field "Attempts from" to "enrolled users who have, or do not have, a quiz attempt" And I set the field "Which tries" to "All tries" And I should see "Response 1a" And I press "Show report" @@ -71,5 +71,5 @@ Feature: Basic use of the Responses report Scenario: Report does not allow strange combinations of options Given I am on the "Quiz 1" "mod_quiz > Responses report" page logged in as teacher And the "Which tries" "select" should be enabled - When I set the field "Attempts from" to "enrolled users who have not attempted the quiz" + When I set the field "Attempts from" to "enrolled users who do not have a quiz attempt" Then the "Which tries" "select" should be disabled From 50486f5d05defd2f261e3437fdd4b2be41650c31 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Thu, 15 Feb 2024 13:49:51 +0000 Subject: [PATCH 4/8] MDL-68806 quiz: Update questions in pre-created attempts when started In case questions used in pre-created attempts were edited after pre-creation, we check when the attempt is started and create new question attempts if needed. There is still an opportunity to improve performance further by finding a way to do this efficiently when a new question version is created. --- mod/quiz/locallib.php | 2 + mod/quiz/tests/attempt_test.php | 17 +++++ mod/quiz/tests/attempt_walkthrough_test.php | 79 +++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 320866827de..aa39169bc6e 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -372,6 +372,8 @@ function quiz_attempt_save_started( $originalattempt = null; if (isset($attempt->id) && $attempt->state === quiz_attempt::NOT_STARTED) { $originalattempt = clone $attempt; + // In case questions have been edited since attempts were pre-created, update questions now. + quiz_attempt::create($attempt->id)->update_questions_to_new_version_if_changed(); // Update the attempt's state. $attempt->state = quiz_attempt::IN_PROGRESS; $DB->update_record('quiz_attempts', $attempt); diff --git a/mod/quiz/tests/attempt_test.php b/mod/quiz/tests/attempt_test.php index 8a3695b30c4..f6bdc290b24 100644 --- a/mod/quiz/tests/attempt_test.php +++ b/mod/quiz/tests/attempt_test.php @@ -571,4 +571,21 @@ final class attempt_test extends \advanced_testcase { $this->assertEquals(0, $grades[$readinggrade->id]->grade); $this->assertEquals(1, $grades[$readinggrade->id]->maxgrade); } + + /** + * When creating a new quiz attempt, question attempts should be created with the first step's timecreated set to null. + * + * When the question attempt is rendered, it should be set to the current time. + * + * @return void + * @throws \coding_exception + * @covers ::quiz_start_new_attempt + */ + public function test_step_timecreated_unset_when_starting_quiz_attempt(): void { + $attempt = $this->create_quiz_and_attempt_with_layout('1'); + $questionattempt = $attempt->get_question_attempt(1); + $this->assertEquals(\question_attempt_step::TIMECREATED_ON_FIRST_RENDER, $questionattempt->get_step(0)->get_timecreated()); + $questionattempt->render(new \question_display_options(), 1); + $this->assertEqualsWithDelta(time(), $questionattempt->get_step(0)->get_timecreated(), 1); + } } diff --git a/mod/quiz/tests/attempt_walkthrough_test.php b/mod/quiz/tests/attempt_walkthrough_test.php index be92c959098..126f873ad4d 100644 --- a/mod/quiz/tests/attempt_walkthrough_test.php +++ b/mod/quiz/tests/attempt_walkthrough_test.php @@ -694,4 +694,83 @@ final class attempt_walkthrough_test extends \advanced_testcase { $this->assertEquals($attemptobj->get_context(), $gradedevent->get_context()); $this->assertEquals($attempturl, $gradedevent->get_url()); } + + /** + * Create a quiz with questions, pre-create an attempt, edit a question, then begin the attempt. + */ + public function test_quiz_attempt_walkthrough_update_question_after_precreate(): void { + global $SITE; + + $this->resetAfterTest(true); + + // Make a quiz. + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + + $quiz = $quizgenerator->create_instance( + [ + 'course' => $SITE->id, + 'questionsperpage' => 0, + 'grade' => 100.0, + 'sumgrades' => 3, + ], + ); + + // Create a couple of questions. + $questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question'); + + $cat = $questiongenerator->create_question_category(); + $saq = $questiongenerator->create_question('shortanswer', null, ['category' => $cat->id]); + $numq = $questiongenerator->create_question('numerical', null, ['category' => $cat->id]); + $matchq = $questiongenerator->create_question('match', null, ['category' => $cat->id]); + $description = $questiongenerator->create_question('description', null, ['category' => $cat->id]); + + // Add them to the quiz. + quiz_add_quiz_question($saq->id, $quiz); + quiz_add_quiz_question($numq->id, $quiz); + quiz_add_quiz_question($matchq->id, $quiz); + quiz_add_quiz_question($description->id, $quiz); + + // Make a user to do the quiz. + $user1 = $this->getDataGenerator()->create_user(); + + $quizobj = quiz_settings::create($quiz->id, $user1->id); + + // Start the attempt. + $quba = question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + + $timenow = time(); + $attempt = quiz_create_attempt($quizobj, 1, false, $timenow, false, $user1->id); + + quiz_start_new_attempt($quizobj, $quba, $attempt, 1, $timenow); + $this->assertEquals('1,2,3,4,0', $attempt->layout); + + quiz_attempt_save_not_started($quba, $attempt); + + $attemptobj = quiz_attempt::create($attempt->id); + + // Update question in quiz. + $newsa = $questiongenerator->update_question($saq, null, + ['name' => 'This is the second version of shortanswer']); + $newnumbq = $questiongenerator->update_question($numq, null, + ['name' => 'This is the second version of numerical']); + $newmatch = $questiongenerator->update_question($matchq, null, + ['name' => 'This is the second version of match']); + $newdescription = $questiongenerator->update_question($description, null, + ['name' => 'This is the second version of description']); + + $this->assertEquals($saq->id, $attemptobj->get_question_attempt(1)->get_question_id()); + $this->assertEquals($numq->id, $attemptobj->get_question_attempt(2)->get_question_id()); + $this->assertEquals($matchq->id, $attemptobj->get_question_attempt(3)->get_question_id()); + $this->assertEquals($description->id, $attemptobj->get_question_attempt(4)->get_question_id()); + + quiz_attempt_save_started($quizobj, $quba, $attempt); + + // Verify that the started attempt contains the new questions. + $attemptobj = quiz_attempt::create($attempt->id); + $this->assertEquals($newsa->id, $attemptobj->get_question_attempt(1)->get_question_id()); + $this->assertEquals($newnumbq->id, $attemptobj->get_question_attempt(2)->get_question_id()); + $this->assertEquals($newmatch->id, $attemptobj->get_question_attempt(3)->get_question_id()); + $this->assertEquals($newdescription->id, $attemptobj->get_question_attempt(4)->get_question_id()); + } } From 9dee9c99176ebc81a1bfcbc38ff826e9f5ac5046 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Fri, 16 Feb 2024 14:36:47 +0000 Subject: [PATCH 5/8] MDL-68806 question: Set first attempt step timecreated Previously, the timecreated for the first step of any question attempt was set to the current time when the quiz attempt was created. With quiz attempts being pre-created, this means the question attempts looking like they were started before the quiz attempt. It also limits the ability to analyse when a student actually started each question. This change adds a new class constant, question_attempt_step::TIMECREATED_ON_FIRST_RENDER. Passing this to the question_attempt_step constructor will cause the timecreated to be set to the time when the question attempt is first rendered, rather than when the step is created. --- .upgradenotes/MDL-68806-2024060613532440.yml | 15 +++++++++++++++ lib/db/install.xml | 2 +- mod/quiz/locallib.php | 2 +- .../behat/quiz_activity_completion.feature | 11 ++++++++--- mod/quiz/tests/external/external_test.php | 10 +++++----- question/engine/questionattempt.php | 17 +++++++++++++++++ question/engine/questionattemptstep.php | 18 +++++++++++++++++- 7 files changed, 64 insertions(+), 11 deletions(-) create mode 100644 .upgradenotes/MDL-68806-2024060613532440.yml diff --git a/.upgradenotes/MDL-68806-2024060613532440.yml b/.upgradenotes/MDL-68806-2024060613532440.yml new file mode 100644 index 00000000000..5d101d1b260 --- /dev/null +++ b/.upgradenotes/MDL-68806-2024060613532440.yml @@ -0,0 +1,15 @@ +issueNumber: MDL-68806 +notes: + core_question: + - message: > + question_attempt_step's constructor now accepts the class constant + TIMECREATED_ON_FIRST_RENDER as a value for the + + $timecreated parameter. Calling question_attempt::render for the first + time will now set the first step's timecreated + + to the current time if it is set to this value. Note, null could not be + used here as it is already used to indicate + + timecreated should be set to the current time. + type: changed diff --git a/lib/db/install.xml b/lib/db/install.xml index eb1534c80e9..5d5f1c27381 100644 --- a/lib/db/install.xml +++ b/lib/db/install.xml @@ -1619,7 +1619,7 @@ - + diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index aa39169bc6e..38d57e47a0e 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -256,7 +256,7 @@ function quiz_start_new_attempt($quizobj, $quba, $attempt, $attemptnumber, $time $forcedvariantsbyseed, $variantstrategy); } - $quba->start_all_questions($variantstrategy, $timenow, $attempt->userid); + $quba->start_all_questions($variantstrategy, question_attempt_step::TIMECREATED_ON_FIRST_RENDER, $attempt->userid); // Work out the attempt layout. $sections = $quizobj->get_sections(); diff --git a/mod/quiz/tests/behat/quiz_activity_completion.feature b/mod/quiz/tests/behat/quiz_activity_completion.feature index 75f7d82a168..393c074dd7c 100644 --- a/mod/quiz/tests/behat/quiz_activity_completion.feature +++ b/mod/quiz/tests/behat/quiz_activity_completion.feature @@ -50,9 +50,10 @@ Feature: View activity completion in the quiz activity And the "Receive a grade" completion condition of "Test quiz name" is displayed as "todo" And the "Receive a passing grade" completion condition of "Test quiz name" is displayed as "todo" And the "Receive a pass grade or complete all available attempts" completion condition of "Test quiz name" is displayed as "todo" - And user "student1" has attempted "Test quiz name" with responses: - | slot | response | - | 1 | False | + And I press "Attempt quiz" + And I set the field "False" to "1" + And I press "Finish attempt ..." + And I press "Submit all and finish" And I am on "Course 1" course homepage And I follow "Test quiz name" And the "View" completion condition of "Test quiz name" is displayed as "done" @@ -60,6 +61,10 @@ Feature: View activity completion in the quiz activity And the "Receive a grade" completion condition of "Test quiz name" is displayed as "done" And the "Receive a passing grade" completion condition of "Test quiz name" is displayed as "failed" And the "Receive a pass grade or complete all available attempts" completion condition of "Test quiz name" is displayed as "todo" + And I run all adhoc tasks + And I reload the page + And the "Receive a grade" completion condition of "Test quiz name" is displayed as "done" + And the "Receive a passing grade" completion condition of "Test quiz name" is displayed as "failed" And I press "Re-attempt quiz" And I set the field "" to "1" And I press "Finish attempt ..." diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index 9013688d99f..de4a7f016c7 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -1217,7 +1217,7 @@ final class external_test extends externallib_advanced_testcase { $this->assertEmpty($result['questions'][0]['mark']); $this->assertEquals(1, $result['questions'][0]['maxmark']); $this->assertEquals(1, $result['questions'][0]['sequencecheck']); - $this->assertGreaterThanOrEqual($timenow, $result['questions'][0]['lastactiontime']); + $this->assertEquals(\question_attempt_step::TIMECREATED_ON_FIRST_RENDER, $result['questions'][0]['lastactiontime']); $this->assertEquals(false, $result['questions'][0]['hasautosavedstep']); // Now try the last page. @@ -1238,7 +1238,7 @@ final class external_test extends externallib_advanced_testcase { $this->assertFalse($result['questions'][0]['flagged']); $this->assertEquals(1, $result['questions'][0]['page']); $this->assertEquals(1, $result['questions'][0]['sequencecheck']); - $this->assertGreaterThanOrEqual($timenow, $result['questions'][0]['lastactiontime']); + $this->assertEquals(\question_attempt_step::TIMECREATED_ON_FIRST_RENDER, $result['questions'][0]['lastactiontime']); $this->assertEquals(false, $result['questions'][0]['hasautosavedstep']); // Finish previous attempt. @@ -1356,8 +1356,8 @@ final class external_test extends externallib_advanced_testcase { $this->assertEmpty($result['questions'][1]['mark']); $this->assertEquals(1, $result['questions'][0]['sequencecheck']); $this->assertEquals(1, $result['questions'][1]['sequencecheck']); - $this->assertGreaterThanOrEqual($timenow, $result['questions'][0]['lastactiontime']); - $this->assertGreaterThanOrEqual($timenow, $result['questions'][1]['lastactiontime']); + $this->assertEquals(\question_attempt_step::TIMECREATED_ON_FIRST_RENDER, $result['questions'][0]['lastactiontime']); + $this->assertEquals(\question_attempt_step::TIMECREATED_ON_FIRST_RENDER, $result['questions'][1]['lastactiontime']); $this->assertEquals(false, $result['questions'][0]['hasautosavedstep']); $this->assertEquals(false, $result['questions'][1]['hasautosavedstep']); @@ -1435,7 +1435,7 @@ final class external_test extends externallib_advanced_testcase { $this->assertEquals(1, $result['questions'][0]['sequencecheck']); $this->assertEquals(1, $result['questions'][1]['sequencecheck']); $this->assertGreaterThanOrEqual($timenow, $result['questions'][0]['lastactiontime']); - $this->assertGreaterThanOrEqual($timenow, $result['questions'][1]['lastactiontime']); + $this->assertEquals(\question_attempt_step::TIMECREATED_ON_FIRST_RENDER, $result['questions'][1]['lastactiontime']); $this->assertEquals(true, $result['questions'][0]['hasautosavedstep']); $this->assertEquals(false, $result['questions'][1]['hasautosavedstep']); diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index 4176f60d379..33ca2c1909e 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -900,6 +900,7 @@ class question_attempt { */ public function render($options, $number, $page = null) { $this->ensure_question_initialised(); + $this->set_first_step_timecreated(); if (is_null($page)) { global $PAGE; $page = $PAGE; @@ -1713,6 +1714,22 @@ class question_attempt { public function get_steps_with_submitted_response_iterator() { return new question_attempt_steps_with_submitted_response_iterator($this); } + + /** + * If the first step has a timecreated set to TIMECREATED_ON_FIRST_RENDER, set it to the current time. + * + * @return void + */ + protected function set_first_step_timecreated(): void { + global $DB; + $firststep = $this->get_step(0); + if ((int)$firststep->get_timecreated() === question_attempt_step::TIMECREATED_ON_FIRST_RENDER) { + $timenow = time(); + $firststep->set_timecreated($timenow); + $this->observer->notify_step_modified($firststep, $this, 0); + $DB->set_field('question_attempt_steps', 'timecreated', $timenow, ['id' => $firststep->get_id()]); + } + } } diff --git a/question/engine/questionattemptstep.php b/question/engine/questionattemptstep.php index da1750a090e..b845e9d45a0 100644 --- a/question/engine/questionattemptstep.php +++ b/question/engine/questionattemptstep.php @@ -69,6 +69,11 @@ defined('MOODLE_INTERNAL') || die(); * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class question_attempt_step { + /** + * @var int Indicates that timecreated will be set the first time the attempt is rendered + */ + const TIMECREATED_ON_FIRST_RENDER = -1; + /** * @var integer if this attempts is stored in the question_attempts table, * the id of that row. @@ -107,7 +112,8 @@ class question_attempt_step { * normally created by {@see question_attempt} methods like * {@see question_attempt::process_action()}. * @param array $data the submitted data that defines this step. - * @param int|null $timecreated the time to record for the action. (If not given, use now.) + * @param int|null $timecreated the time to record for the action. If null, use now. If + * {@see self::TIMECREATED_ON_FIRST_RENDER}, the time will be set the first time the attempt is rendered. * @param int|null $userid the user to attribute the aciton to. (If not given, use the current user.) * @param int|null $existingstepid if this step is going to replace an existing step * (for example, during a regrade) this is the id of the previous step we are replacing. @@ -220,6 +226,16 @@ class question_attempt_step { return $this->timecreated; } + /** + * Setter for $this->timecreated. + * + * @param int $timecreated + * @return void + */ + public function set_timecreated(int $timecreated): void { + $this->timecreated = $timecreated; + } + /** * @param string $name the name of a question type variable to look for in the submitted data. * @return bool whether a variable with this name exists in the question type data. From 2841c53c7cc85ec007fb5ebff155d1de3c9f3d20 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Wed, 21 Feb 2024 09:23:21 +0000 Subject: [PATCH 6/8] MDL-68806 backup: Fix faulty question attempt step restore test Previously, the test was passing by fluke because all question attempt steps had the same timecreated date. However, the $originaliterator only contained the first step of the question, not all 3 attempts that were being backed up and restored. This means we were comparing the timecreated of the first backed-up step with the timecreated on the third restored step. As of MDL-68806, the first step's timecreated is question_attempt_step::TIMECREATED_ON_FIRST_RENDER, which shows up this error. This change gets the question_usage_by_activity object back from the attempt object which contains all the attempt steps, rather than using the one created in the test, which doesn't. --- backup/moodle2/tests/restore_stepslib_date_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backup/moodle2/tests/restore_stepslib_date_test.php b/backup/moodle2/tests/restore_stepslib_date_test.php index 6156ed50675..c362338b554 100644 --- a/backup/moodle2/tests/restore_stepslib_date_test.php +++ b/backup/moodle2/tests/restore_stepslib_date_test.php @@ -413,7 +413,7 @@ final class restore_stepslib_date_test extends \restore_date_testcase { $attemptobj->process_grade_submission($timenow); $questionattemptstepdates = []; - $originaliterator = $quba->get_attempt_iterator(); + $originaliterator = $attemptobj->get_question_usage()->get_attempt_iterator(); foreach ($originaliterator as $questionattempt) { $questionattemptstepdates[] = ['originaldate' => $questionattempt->get_last_action_time()]; } From 8d5d4f5314e0d33a1bbdafd3da0c4dc42de11a18 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Mon, 12 Aug 2024 11:28:18 +0100 Subject: [PATCH 7/8] MDL-68806 quiz: Backwards compatibility for mod_quiz_get_user_attempts --- .upgradenotes/MDL-68806-2024081214032268.yml | 24 ++ mod/quiz/classes/external.php | 173 +++++++++- mod/quiz/db/services.php | 12 +- mod/quiz/locallib.php | 7 + mod/quiz/tests/external/external_test.php | 320 +++++++++++++++++++ 5 files changed, 531 insertions(+), 5 deletions(-) create mode 100644 .upgradenotes/MDL-68806-2024081214032268.yml diff --git a/.upgradenotes/MDL-68806-2024081214032268.yml b/.upgradenotes/MDL-68806-2024081214032268.yml new file mode 100644 index 00000000000..90d2725e052 --- /dev/null +++ b/.upgradenotes/MDL-68806-2024081214032268.yml @@ -0,0 +1,24 @@ +issueNumber: MDL-68806 +notes: + mod_quiz: + - message: >+ + The webservice function `mod_quiz_get_user_attempts` is now deprecated + in favour of `mod_quiz_get_user_quiz_attempts`. + + + With the introduction of the new NOT_STARTED quiz attempt state, + `mod_quiz_get_user_attempts` has been modified to not return NOT_STARTED + attempts, allowing clients such as the mobile app to continue working + without modifications. + + + `mod_quiz_get_user_quiz_attempts` will return attempts in all states, as + `mod_quiz_get_user_attempts` did before. Once clients are updated to + handle NOT_STARTED attempts, they can migrate to use this function. + + + A minor modification to `mod_quiz_start_attempt` has been made to allow + it to transparently start an existing attempt that is in the NOT_STARTED + state, rather than creating a new one. + + type: deprecated diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index 3d78bb774df..3098b701075 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -363,7 +363,15 @@ class mod_quiz_external extends external_api { * * @return external_function_parameters * @since Moodle 3.1 + * @deprecated Since Moodle 5.0 MDL-68806. + * @todo Final deprecation in Moodle 6.0 (MDL-80956) */ + #[\core\attribute\deprecated( + 'mod_quiz_external::get_user_quiz_attempts_parameters', + since: '5.0', + reason: 'The old API for fetching attempts doesn\'t return true states for NOT_STARTED and SUBMITTED attempts', + mdl: 'MDL-68806' + )] public static function get_user_attempts_parameters() { return new external_function_parameters ( [ @@ -379,15 +387,27 @@ class mod_quiz_external extends external_api { /** * Return a list of attempts for the given quiz and user. * + * For backwards compatibility, SUBMITTED attempts will be treated as FINISHED with marks hidden, and NOT_STARTED will not + * be returned. To return all real states, call get_user_quiz_attempts instead. + * * @param int $quizid quiz instance id * @param int $userid user id * @param string $status quiz status: all, finished or unfinished * @param bool $includepreviews whether to include previews or not * @return array of warnings and the list of attempts * @since Moodle 3.1 + * @deprecated Since Moodle 5.0 MDL-68806. + * @todo Final deprecation in Moodle 6.0 (MDL-80956) */ + #[\core\attribute\deprecated( + 'mod_quiz_external::get_user_quiz_attempts', + since: '5.0', + reason: 'The old API for fetching attempts doesn\'t return true states for NOT_STARTED and SUBMITTED attempts', + mdl: 'MDL-68806' + )] public static function get_user_attempts($quizid, $userid = 0, $status = 'finished', $includepreviews = false) { global $USER; + \core\deprecation::emit_deprecation_if_present(__METHOD__); $warnings = []; @@ -426,9 +446,20 @@ class mod_quiz_external extends external_api { array_column($attempts, 'uniqueid')); $attemptresponse = []; foreach ($attempts as $attempt) { + if ($attempt->state == quiz_attempt::NOT_STARTED) { + continue; // For backwards compatibility, do not return Not Started attempts. + } $reviewoptions = quiz_get_review_options($quiz, $attempt, $context); - if (!has_capability('mod/quiz:viewreports', $context) && - ($reviewoptions->marks < question_display_options::MARK_AND_MAX || $attempt->state != quiz_attempt::FINISHED)) { + if ( + $attempt->state == quiz_attempt::SUBMITTED || + ( + !has_capability('mod/quiz:viewreports', $context) && + ( + $reviewoptions->marks < question_display_options::MARK_AND_MAX || + $attempt->state != quiz_attempt::FINISHED + ) + ) + ) { // Blank the mark if the teacher does not allow it. $attempt->sumgrades = null; } else if (isset($gradeitemmarks[$attempt->uniqueid])) { @@ -441,6 +472,9 @@ class mod_quiz_external extends external_api { ]; } } + if ($attempt->state == quiz_attempt::SUBMITTED) { + $attempt->state = quiz_attempt::FINISHED; // For backwards-compatibility. + } $attemptresponse[] = $attempt; } $result = []; @@ -498,13 +532,144 @@ class mod_quiz_external extends external_api { * * @return external_single_structure * @since Moodle 3.1 + * @deprecated Since Moodle 5.0 MDL-68806. + * @todo Final deprecation in Moodle 6.0 (MDL-80956) */ + #[\core\attribute\deprecated( + 'mod_quiz_external::get_user_quiz_attempts_returns', + since: '5.0', + reason: 'The old API for fetching attempts doesn\'t return true states for NOT_STARTED and SUBMITTED attempts', + mdl: 'MDL-68806' + )] public static function get_user_attempts_returns() { + $attemptstructure = self::attempt_structure(); + $attemptstructure->keys['state']->desc .= " For backwards compatibility, attempts in 'submitted' state will return " . + "'finished' and attempts in 'notstarted' state will return 'inprogress'. To get attempts with all real states, call " . + "get_user_quiz_attempts() instead."; + return new external_single_structure( + [ + 'attempts' => new external_multiple_structure($attemptstructure), + 'warnings' => new external_warnings(), + ] + ); + } + + /** + * Mark get_user_attempts as deprecated. + * + * @return bool + */ + public static function get_user_attempts_is_deprecated(): bool { + return true; + } + + /** + * Describes the parameters for get_user_quiz_attempts. + * + * @return external_function_parameters + * @since Moodle 4.5 + */ + public static function get_user_quiz_attempts_parameters(): external_function_parameters { + return new external_function_parameters ( + [ + 'quizid' => new external_value(PARAM_INT, 'quiz instance id'), + 'userid' => new external_value(PARAM_INT, 'user id, empty for current user', VALUE_DEFAULT, 0), + 'status' => new external_value(PARAM_ALPHA, 'quiz status: all, finished or unfinished', VALUE_DEFAULT, 'finished'), + 'includepreviews' => new external_value(PARAM_BOOL, 'whether to include previews or not', VALUE_DEFAULT, false), + ], + ); + } + + /** + * Return a list of attempts for the given quiz and user. + * + * @param int $quizid quiz instance id + * @param int $userid user id + * @param string $status quiz status: all, finished or unfinished + * @param bool $includepreviews whether to include previews or not + * @return array of warnings and the list of attempts + * @since Moodle 4.5 + */ + public static function get_user_quiz_attempts( + int $quizid, + int $userid = 0, + string $status = 'finished', + bool $includepreviews = false + ): array { + global $USER; + + $warnings = []; + + $params = [ + 'quizid' => $quizid, + 'userid' => $userid, + 'status' => $status, + 'includepreviews' => $includepreviews, + ]; + $params = self::validate_parameters(self::get_user_quiz_attempts_parameters(), $params); + + [$quiz, $course, $cm, $context] = self::validate_quiz($params['quizid']); + + if (!in_array($params['status'], ['all', 'finished', 'unfinished'])) { + throw new invalid_parameter_exception('Invalid status value'); + } + + // Default value for userid. + if (empty($params['userid'])) { + $params['userid'] = $USER->id; + } + + $user = core_user::get_user($params['userid'], '*', MUST_EXIST); + core_user::require_active_user($user); + + // Extra checks so only users with permissions can view other users attempts. + if ($USER->id != $user->id) { + require_capability('mod/quiz:viewreports', $context); + } + + // Update quiz with override information. + $quiz = quiz_update_effective_access($quiz, $params['userid']); + $attempts = quiz_get_user_attempts($quiz->id, $user->id, $params['status'], $params['includepreviews']); + $quizobj = new quiz_settings($quiz, $cm, $course); + $gradeitemmarks = $quizobj->get_grade_calculator()->compute_grade_item_totals_for_attempts( + array_column($attempts, 'uniqueid')); + $attemptresponse = []; + foreach ($attempts as $attempt) { + $reviewoptions = quiz_get_review_options($quiz, $attempt, $context); + if (!has_capability('mod/quiz:viewreports', $context) && + ($reviewoptions->marks < question_display_options::MARK_AND_MAX || $attempt->state != quiz_attempt::FINISHED)) { + // Blank the mark if the teacher does not allow it. + $attempt->sumgrades = null; + } else if (isset($gradeitemmarks[$attempt->uniqueid])) { + $attempt->gradeitemmarks = []; + foreach ($gradeitemmarks[$attempt->uniqueid] as $gradeitem) { + $attempt->gradeitemmarks[] = [ + 'name' => \core_external\util::format_string($gradeitem->name, $context), + 'grade' => $gradeitem->grade, + 'maxgrade' => $gradeitem->maxgrade, + ]; + } + } + $attemptresponse[] = $attempt; + } + $result = []; + $result['attempts'] = $attemptresponse; + $result['warnings'] = $warnings; + return $result; + } + + /** + * Describes the get_user_attempts return value. + * + * @return external_single_structure + * @since Moodle 4.5 + */ + public static function get_user_quiz_attempts_returns(): external_single_structure { return new external_single_structure( [ 'attempts' => new external_multiple_structure(self::attempt_structure()), 'warnings' => new external_warnings(), - ] + ], ); } @@ -812,7 +977,7 @@ class mod_quiz_external extends external_api { $accessmanager->notify_preflight_check_passed($currentattemptid); } - if ($currentattemptid) { + if ($currentattemptid && $lastattempt->state !== quiz_attempt::NOT_STARTED) { if ($lastattempt->state == quiz_attempt::OVERDUE) { throw new moodle_exception('stateoverdue', 'quiz', $quizobj->view_url()); } else { diff --git a/mod/quiz/db/services.php b/mod/quiz/db/services.php index 568570b9e0c..00ae4d4b851 100644 --- a/mod/quiz/db/services.php +++ b/mod/quiz/db/services.php @@ -50,12 +50,22 @@ $functions = [ 'mod_quiz_get_user_attempts' => [ 'classname' => 'mod_quiz_external', 'methodname' => 'get_user_attempts', - 'description' => 'Return a list of attempts for the given quiz and user.', + 'description' => 'Return a list of attempts for the given quiz and user. ' . + '(Deprecated in favour of mod_quiz_get_user_quiz_attempts).', 'type' => 'read', 'capabilities' => 'mod/quiz:view', 'services' => [MOODLE_OFFICIAL_MOBILE_SERVICE] ], + 'mod_quiz_get_user_quiz_attempts' => [ + 'classname' => 'mod_quiz_external', + 'methodname' => 'get_user_quiz_attempts', + 'description' => 'Return a list of attempts for the given quiz and user.', + 'type' => 'read', + 'capabilities' => 'mod/quiz:view', + 'services' => [MOODLE_OFFICIAL_MOBILE_SERVICE], + ], + 'mod_quiz_get_user_best_grade' => [ 'classname' => 'mod_quiz_external', 'methodname' => 'get_user_best_grade', diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 38d57e47a0e..d6464655742 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -346,6 +346,10 @@ function quiz_start_attempt_built_on_last($quba, $attempt, $lastattempt) { /** * Create or update the quiz attempt record, and the question usage. * + * If the attempt already exists in the database with the NOT_STARTED state, it will be transitioned + * to IN_PROGRESS and the timestart updated. If it does not already exist, a new record will be created + * already in the IN_PROGRESS state. + * * @param quiz_settings $quizobj * @param question_usage_by_activity $quba * @param stdClass $attempt @@ -416,6 +420,9 @@ function quiz_attempt_save_started( /** * Create the quiz attempt record, and the question usage. * + * This saves an attempt in the NOT_STARTED state, and is designed for use when pre-creating attempts + * ahead of the quiz start time to spread out the processing load. + * * @param question_usage_by_activity $quba * @param stdClass $attempt * @return stdClass attempt object with uniqueid and id set. diff --git a/mod/quiz/tests/external/external_test.php b/mod/quiz/tests/external/external_test.php index de4a7f016c7..151f8f11ecd 100644 --- a/mod/quiz/tests/external/external_test.php +++ b/mod/quiz/tests/external/external_test.php @@ -429,6 +429,11 @@ final class external_test extends externallib_advanced_testcase { } + /** + * Test get_user_attempts + * + * @todo Remove in Moodle 6.0 as part of MDL-80956 final deprecations. + */ public function test_get_user_attempts(): void { // Create a quiz with one attempt finished. @@ -436,6 +441,7 @@ final class external_test extends externallib_advanced_testcase { $this->setUser($this->student); $result = mod_quiz_external::get_user_attempts($quiz->id); + $this->assertDebuggingCalled(); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); $this->assertCount(1, $result['attempts']); @@ -445,24 +451,31 @@ final class external_test extends externallib_advanced_testcase { $this->assertEquals(1, $result['attempts'][0]['attempt']); $this->assertArrayHasKey('sumgrades', $result['attempts'][0]); $this->assertEquals(1.0, $result['attempts'][0]['sumgrades']); + $this->assertEquals(quiz_attempt::FINISHED, $result['attempts'][0]['state']); // Test filters. Only finished. + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, 0, 'finished', false); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); $this->assertEquals($attempt->id, $result['attempts'][0]['id']); // Test filters. All attempts. + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, 0, 'all', false); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); $this->assertEquals($attempt->id, $result['attempts'][0]['id']); // Test filters. Unfinished. + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, 0, 'unfinished', false); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(0, $result['attempts']); @@ -476,40 +489,55 @@ final class external_test extends externallib_advanced_testcase { quiz_attempt_save_started($quizobj, $quba, $attempt); // Test filters. All attempts. + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, 0, 'all', false); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(2, $result['attempts']); // Test filters. Unfinished. + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, 0, 'unfinished', false); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); // Test manager can see user attempts. $this->setUser($this->teacher); + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, $this->student->id); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, $this->student->id, 'all'); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(2, $result['attempts']); $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); // Invalid parameters. try { + $this->resetDebugging(); mod_quiz_external::get_user_attempts($quiz->id, $this->student->id, 'INVALID_PARAMETER'); $this->fail('Exception expected due to missing capability.'); } catch (\invalid_parameter_exception $e) { + $this->assertDebuggingCalled(); $this->assertEquals('invalidparameter', $e->errorcode); } } + /** + * Test get_user_attempts with extra grades + * + * @todo Remove in Moodle 6.0 as part of MDL-80956 final deprecations. + */ public function test_get_user_attempts_with_extra_grades(): void { global $DB; @@ -525,8 +553,10 @@ final class external_test extends externallib_advanced_testcase { $structure->update_slot_grade_item($structure->get_slot_by_number(2), $readinggrade->id); $this->setUser($this->student); + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); $this->assertEquals($attempt->id, $result['attempts'][0]['id']); @@ -537,8 +567,10 @@ final class external_test extends externallib_advanced_testcase { // Now change the review options, so marks are not displayed, and check the result. $DB->set_field('quiz', 'reviewmarks', 0, ['id' => $quiz->id]); + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); $this->assertEquals($attempt->id, $result['attempts'][0]['id']); @@ -547,6 +579,8 @@ final class external_test extends externallib_advanced_testcase { /** * Test get_user_attempts with marks hidden + * + * @todo Remove in Moodle 6.0 as part of MDL-80956 final deprecations. */ public function test_get_user_attempts_with_marks_hidden(): void { // Create quiz with one attempt finished and hide the mark. @@ -556,8 +590,10 @@ final class external_test extends externallib_advanced_testcase { // Student cannot see the grades. $this->setUser($this->student); + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); $this->assertEquals($attempt->id, $result['attempts'][0]['id']); @@ -569,8 +605,10 @@ final class external_test extends externallib_advanced_testcase { // Test manager can see user grades. $this->setUser($this->teacher); + $this->resetDebugging(); $result = mod_quiz_external::get_user_attempts($quiz->id, $this->student->id); $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + $this->assertDebuggingCalled(); $this->assertCount(1, $result['attempts']); $this->assertEquals($attempt->id, $result['attempts'][0]['id']); @@ -581,6 +619,288 @@ final class external_test extends externallib_advanced_testcase { $this->assertEquals(1.0, $result['attempts'][0]['sumgrades']); } + /** + * Test get_user_attempts when the attempt is in 'submitted' state. + * + * @todo Remove in Moodle 6.0 as part of MDL-80956 final deprecations. + * @covers \mod_quiz_external::get_user_attempts + */ + public function test_get_user_attempts_submitted(): void { + + // Create a quiz with one attempt. + [$quiz, , , $attempt, $attemptobj] = $this->create_quiz_with_questions(true); + // Submit the attempt but do not finish it. + // Process some responses from the student. + $tosubmit = [1 => ['answer' => '3.14']]; + $attemptobj->process_submitted_actions(time(), false, $tosubmit); + $attemptobj->process_submit(time(), false); + + $this->setUser($this->student); + $result = mod_quiz_external::get_user_attempts($quiz->id); + $this->assertDebuggingCalled(); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + $this->assertEquals($quiz->id, $result['attempts'][0]['quiz']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + $this->assertEquals(1, $result['attempts'][0]['attempt']); + $this->assertArrayHasKey('sumgrades', $result['attempts'][0]); + $this->assertNull($result['attempts'][0]['sumgrades']); // No grades. + $this->assertEquals(quiz_attempt::FINISHED, $result['attempts'][0]['state']); // State is returned as finished. + } + + /** + * Test get_user_attempts when the attempt is in 'notstarted' state. The attempt should not be returned. + * + * @todo Remove in Moodle 6.0 as part of MDL-80956 final deprecations. + * @covers \mod_quiz_external::get_user_attempts + */ + public function test_get_user_attempts_notstarted(): void { + // Create a quiz. + [$quiz, , $quizobj, , ] = $this->create_quiz_with_questions(); + // Create an attempt but do not start it. + // Now, do one attempt. + $quba = \question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + + $timenow = time(); + $attempt = quiz_create_attempt($quizobj, 1, false, $timenow, false, $this->student->id); + quiz_start_new_attempt($quizobj, $quba, $attempt, 1, $timenow); + quiz_attempt_save_not_started($quba, $attempt); + + $this->setUser($this->student); + $result = mod_quiz_external::get_user_attempts($quiz->id, $this->student->id, 'all'); + $this->assertDebuggingCalled(); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_attempts_returns(), $result); + + $this->assertCount(0, $result['attempts']); + } + + /** + * Test get_quiz_user_attempts + * + * @covers \mod_quiz_external::get_user_quiz_attempts + */ + public function test_get_user_quiz_attempts(): void { + + // Create a quiz with one attempt finished. + [$quiz, , $quizobj, $attempt, ] = $this->create_quiz_with_questions(true, true); + + $this->setUser($this->student); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + $this->assertEquals($quiz->id, $result['attempts'][0]['quiz']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + $this->assertEquals(1, $result['attempts'][0]['attempt']); + $this->assertArrayHasKey('sumgrades', $result['attempts'][0]); + $this->assertEquals(1.0, $result['attempts'][0]['sumgrades']); + $this->assertEquals(quiz_attempt::FINISHED, $result['attempts'][0]['state']); + + // Test filters. Only finished. + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, 0, 'finished', false); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + + // Test filters. All attempts. + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, 0, 'all', false); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + + // Test filters. Unfinished. + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, 0, 'unfinished', false); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(0, $result['attempts']); + + // Start a new attempt, but not finish it. + $timenow = time(); + $attempt = quiz_create_attempt($quizobj, 2, false, $timenow, false, $this->student->id); + $quba = \question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + + quiz_start_new_attempt($quizobj, $quba, $attempt, 1, $timenow); + quiz_attempt_save_started($quizobj, $quba, $attempt); + + // Test filters. All attempts. + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, 0, 'all', false); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(2, $result['attempts']); + + // Test filters. Unfinished. + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, 0, 'unfinished', false); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + + // Test manager can see user attempts. + $this->setUser($this->teacher); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, $this->student->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, $this->student->id, 'all'); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(2, $result['attempts']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + + // Invalid parameters. + try { + mod_quiz_external::get_user_quiz_attempts($quiz->id, $this->student->id, 'INVALID_PARAMETER'); + $this->fail('Exception expected due to missing capability.'); + } catch (\invalid_parameter_exception $e) { + $this->assertEquals('invalidparameter', $e->errorcode); + } + } + + /** + * Test get_user_quiz_attempts with extra grades + */ + public function test_get_user_quiz_attempts_with_extra_grades(): void { + global $DB; + + // Create a quiz with one attempt finished. + [$quiz, , , $attempt, $attemptobj] = $this->create_quiz_with_questions(true, true); + + // Add some extra grade items. + $quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz'); + $listeninggrade = $quizgenerator->create_grade_item(['quizid' => $attemptobj->get_quizid(), 'name' => 'Listening']); + $readinggrade = $quizgenerator->create_grade_item(['quizid' => $attemptobj->get_quizid(), 'name' => 'Reading']); + $structure = $attemptobj->get_quizobj()->get_structure(); + $structure->update_slot_grade_item($structure->get_slot_by_number(1), $listeninggrade->id); + $structure->update_slot_grade_item($structure->get_slot_by_number(2), $readinggrade->id); + + $this->setUser($this->student); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + + // Verify additional grades. + $this->assertEquals(['name' => 'Listening', 'grade' => 1, 'maxgrade' => 1], $result['attempts'][0]['gradeitemmarks'][0]); + $this->assertEquals(['name' => 'Reading', 'grade' => 0, 'maxgrade' => 1], $result['attempts'][0]['gradeitemmarks'][1]); + + // Now change the review options, so marks are not displayed, and check the result. + $DB->set_field('quiz', 'reviewmarks', 0, ['id' => $quiz->id]); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + $this->assertArrayNotHasKey('gradeitemmarks', $result['attempts'][0]); + } + + /** + * Test get_user_quiz_attempts with marks hidden + * + * @covers \mod_quiz_external::get_user_quiz_attempts + */ + public function test_get_user_quiz_attempts_with_marks_hidden(): void { + // Create quiz with one attempt finished and hide the mark. + [$quiz, , , $attempt, ] = $this->create_quiz_with_questions( + true, true, 'deferredfeedback', false, + ['marksduring' => 0, 'marksimmediately' => 0, 'marksopen' => 0, 'marksclosed' => 0]); + + // Student cannot see the grades. + $this->setUser($this->student); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + $this->assertEquals($quiz->id, $result['attempts'][0]['quiz']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + $this->assertEquals(1, $result['attempts'][0]['attempt']); + $this->assertArrayHasKey('sumgrades', $result['attempts'][0]); + $this->assertEquals(null, $result['attempts'][0]['sumgrades']); + + // Test manager can see user grades. + $this->setUser($this->teacher); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, $this->student->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + $this->assertEquals($quiz->id, $result['attempts'][0]['quiz']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + $this->assertEquals(1, $result['attempts'][0]['attempt']); + $this->assertArrayHasKey('sumgrades', $result['attempts'][0]); + $this->assertEquals(1.0, $result['attempts'][0]['sumgrades']); + } + + /** + * Test get_user_quiz_attempts when the attempt is in 'submitted' state. + * + * @covers \mod_quiz_external::get_user_quiz_attempts + */ + public function test_get_user_quiz_attempts_submitted(): void { + + // Create a quiz with one attempt. + [$quiz, , , $attempt, $attemptobj] = $this->create_quiz_with_questions(true); + // Submit the attempt but do not finish it. + // Process some responses from the student. + $tosubmit = [1 => ['answer' => '3.14']]; + $attemptobj->process_submitted_actions(time(), false, $tosubmit); + $attemptobj->process_submit(time(), false); + + $this->setUser($this->student); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + $this->assertEquals($quiz->id, $result['attempts'][0]['quiz']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + $this->assertEquals(1, $result['attempts'][0]['attempt']); + $this->assertArrayHasKey('sumgrades', $result['attempts'][0]); + $this->assertNull($result['attempts'][0]['sumgrades']); // No grades. + $this->assertEquals(quiz_attempt::SUBMITTED, $result['attempts'][0]['state']); + } + + /** + * Test get_user_quiz_attempts when the attempt is in 'notstarted' state. + * + * @covers \mod_quiz_external::get_user_quiz_attempts + */ + public function test_get_user_quiz_attempts_notstarted(): void { + // Create a quiz. + [$quiz, , $quizobj, , ] = $this->create_quiz_with_questions(); + // Create an attempt but do not start it. + // Now, do one attempt. + $quba = \question_engine::make_questions_usage_by_activity('mod_quiz', $quizobj->get_context()); + $quba->set_preferred_behaviour($quizobj->get_quiz()->preferredbehaviour); + + $timenow = time(); + $attempt = quiz_create_attempt($quizobj, 1, false, $timenow, false, $this->student->id); + quiz_start_new_attempt($quizobj, $quba, $attempt, 1, $timenow); + quiz_attempt_save_not_started($quba, $attempt); + + $this->setUser($this->student); + $result = mod_quiz_external::get_user_quiz_attempts($quiz->id, $this->student->id, 'all'); + $result = external_api::clean_returnvalue(mod_quiz_external::get_user_quiz_attempts_returns(), $result); + + $this->assertCount(1, $result['attempts']); + $this->assertEquals($attempt->id, $result['attempts'][0]['id']); + $this->assertEquals($quiz->id, $result['attempts'][0]['quiz']); + $this->assertEquals($this->student->id, $result['attempts'][0]['userid']); + $this->assertEquals(1, $result['attempts'][0]['attempt']); + $this->assertArrayHasKey('sumgrades', $result['attempts'][0]); + $this->assertNull($result['attempts'][0]['sumgrades']); + $this->assertEquals(quiz_attempt::NOT_STARTED, $result['attempts'][0]['state']); + } + /** * Test get_user_best_grade */ From 580a0d6827b491070fa96bf601395fd940a37fdb Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Fri, 14 Mar 2025 09:19:07 +0000 Subject: [PATCH 8/8] MDL-68806 quiz: Separate global and per-quiz default settings --- mod/quiz/lang/en/quiz.php | 2 ++ mod/quiz/mod_form.php | 14 -------- mod/quiz/settings.php | 71 ++++++++++++++++++++++----------------- 3 files changed, 42 insertions(+), 45 deletions(-) diff --git a/mod/quiz/lang/en/quiz.php b/mod/quiz/lang/en/quiz.php index 0e07c060130..0d4ec099135 100644 --- a/mod/quiz/lang/en/quiz.php +++ b/mod/quiz/lang/en/quiz.php @@ -202,6 +202,7 @@ $string['configdelay2'] = 'If you set a time delay here, then a student has to w $string['configeachattemptbuildsonthelast'] = 'If multiple attempts are allowed then each new attempt contains the results of the previous attempt.'; $string['configgrademethod'] = 'When multiple attempts are allowed, which method should be used to calculate the student\'s final grade for the quiz.'; $string['configintro'] = 'The values you set here define the default values that are used in the settings form when you create a new quiz. You can also configure which quiz settings are considered advanced.'; +$string['configintroglobal'] = 'These settings control the system-wide behaviour of the Quiz activity.'; $string['configmaximumgrade'] = 'The default grade that the quiz grade is scaled to be out of.'; $string['confignewpageevery'] = 'When adding questions to the quiz page breaks will automatically be inserted according to the setting you choose here.'; $string['confignavmethod'] = 'In Free navigation, questions may be answered in any order using navigation. In Sequential, questions must be answered in strict sequence.'; @@ -264,6 +265,7 @@ $string['decimalpoints'] = 'Decimal places'; $string['default'] = 'Default'; $string['defaultgrade'] = 'Default question grade'; $string['defaultinfo'] = 'The default category for questions.'; +$string['defaultsettings'] = 'Default quiz settings'; $string['delaylater'] = 'Enforced delay between later attempts'; $string['delaylater_help'] = 'If enabled, a student must wait for the specified time to elapse before attempting the quiz a third time and any subsequent times.'; $string['delay1'] = 'Time delay between first and second attempt'; diff --git a/mod/quiz/mod_form.php b/mod/quiz/mod_form.php index 264c84298ed..1829badd4ba 100644 --- a/mod/quiz/mod_form.php +++ b/mod/quiz/mod_form.php @@ -718,18 +718,4 @@ class mod_quiz_mod_form extends moodleform_mod { return $this->maxattemptsanyoverride; } - - /** - * Return a list of options for site-wide and per-instance precreate period settings. - * - * @return array - */ - public static function generate_precreate_options(): array { - $precreateoptions[0] = get_string('precreateoff', 'quiz'); - for ($i = 1; $i <= 24; $i++) { - $precreateoptions[$i * HOURSECS] = sprintf(get_string('dateintervalhrfull', 'langconfig'), $i); - } - return $precreateoptions; - } - } diff --git a/mod/quiz/settings.php b/mod/quiz/settings.php index bd19402cd9e..c856c1c2c25 100644 --- a/mod/quiz/settings.php +++ b/mod/quiz/settings.php @@ -57,8 +57,45 @@ if (empty($reportsbyname) && empty($rulesbyname)) { $quizsettings = new admin_settingpage('modsettingquiz', $pagetitle, 'moodle/site:config'); if ($ADMIN->fulltree) { - // Introductory explanation that all the settings are defaults for the add quiz form. - $quizsettings->add(new admin_setting_heading('quizintro', '', get_string('configintro', 'quiz'))); + + $quizsettings->add(new admin_setting_heading('quizintro', '', get_string('configintroglobal', 'quiz'))); + + // Delay to notify graded attempts. + $quizsettings->add(new admin_setting_configduration('quiz/notifyattemptgradeddelay', + get_string('attemptgradeddelay', 'quiz'), get_string('attemptgradeddelay_desc', 'quiz'), 5 * HOURSECS, HOURSECS)); + + // Pre-create attempt period. + $precreateoptions = [get_string('precreateoff', 'quiz')]; + for ($i = 1; $i <= 24; $i++) { + $precreateoptions[$i * HOURSECS] = sprintf(get_string('dateintervalhrfull', 'langconfig'), $i); + } + $setting = new admin_setting_configselect( + 'quiz/precreateperiod', + get_string('precreateperiod', 'quiz'), + get_string('precreateperiod_desc', 'quiz'), + 0, + $precreateoptions, + ); + $quizsettings->add($setting); + + // Minimum grace period used behind the scenes. + $quizsettings->add(new admin_setting_configduration('quiz/graceperiodmin', + get_string('graceperiodmin', 'quiz'), get_string('graceperiodmin_desc', 'quiz'), + 60, 1)); + + // Initial number of feedback items. + $quizsettings->add(new admin_setting_configtext('quiz/initialnumfeedbacks', + get_string('initialnumfeedbacks', 'quiz'), get_string('initialnumfeedbacks_desc', 'quiz'), + 2, PARAM_INT, 5)); + + // Autosave frequency. + $quizsettings->add(new admin_setting_configduration('quiz/autosaveperiod', + get_string('autosaveperiod', 'quiz'), get_string('autosaveperiod_desc', 'quiz'), 60, 1)); + + // Heading explanation that all the settings below are defaults for the add quiz form. + $name = new lang_string('defaultsettings', 'quiz'); + $description = new lang_string('configintro', 'quiz'); + $quizsettings->add(new admin_setting_heading('defaultsettings', $name, $description)); // Time limit. $setting = new admin_setting_configduration('quiz/timelimit', @@ -68,10 +105,6 @@ if ($ADMIN->fulltree) { $setting->set_locked_flag_options(admin_setting_flag::ENABLED, false); $quizsettings->add($setting); - // Delay to notify graded attempts. - $quizsettings->add(new admin_setting_configduration('quiz/notifyattemptgradeddelay', - get_string('attemptgradeddelay', 'quiz'), get_string('attemptgradeddelay_desc', 'quiz'), 5 * HOURSECS, HOURSECS)); - // What to do with overdue attempts. $setting = new \mod_quiz\admin\overdue_handling_setting('quiz/overduehandling', get_string('overduehandling', 'quiz'), get_string('overduehandling_desc', 'quiz'), @@ -94,26 +127,10 @@ if ($ADMIN->fulltree) { get_string('precreateattempts_help', 'quiz'), 0, ); - $setting->set_locked_flag_options(admin_setting_flag::ENABLED, true); $setting->set_advanced_flag_options(admin_setting_flag::ENABLED, true); + $setting->set_locked_flag_options(admin_setting_flag::ENABLED, true); $quizsettings->add($setting); - // Pre-create attempt period. - $precreateoptions = mod_quiz_mod_form::generate_precreate_options(); - $setting = new admin_setting_configselect( - 'quiz/precreateperiod', - get_string('precreateperiod', 'quiz'), - get_string('precreateperiod_desc', 'quiz'), - 0, - $precreateoptions, - ); - $quizsettings->add($setting); - - // Minimum grace period used behind the scenes. - $quizsettings->add(new admin_setting_configduration('quiz/graceperiodmin', - get_string('graceperiodmin', 'quiz'), get_string('graceperiodmin_desc', 'quiz'), - 60, 1)); - // Number of attempts. $options = [get_string('unlimited')]; for ($i = 1; $i <= QUIZ_MAX_ATTEMPT_OPTION; $i++) { @@ -288,20 +305,12 @@ if ($ADMIN->fulltree) { $setting->set_locked_flag_options(admin_setting_flag::ENABLED, false); $quizsettings->add($setting); - $quizsettings->add(new admin_setting_configtext('quiz/initialnumfeedbacks', - get_string('initialnumfeedbacks', 'quiz'), get_string('initialnumfeedbacks_desc', 'quiz'), - 2, PARAM_INT, 5)); - // Allow user to specify if setting outcomes is an advanced setting. if (!empty($CFG->enableoutcomes)) { $quizsettings->add(new admin_setting_configcheckbox('quiz/outcomes_adv', get_string('outcomesadvanced', 'quiz'), get_string('configoutcomesadvanced', 'quiz'), '0')); } - - // Autosave frequency. - $quizsettings->add(new admin_setting_configduration('quiz/autosaveperiod', - get_string('autosaveperiod', 'quiz'), get_string('autosaveperiod_desc', 'quiz'), 60, 1)); } // Now, depending on whether any reports have their own settings page, add