From 4911b5b2548cbd81a76ff465927df3add7fee136 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 21 Jan 2016 17:21:45 +0100 Subject: [PATCH 1/2] MDL-52863 mod_quiz: Move processattempt.php code to new API --- mod/quiz/attemptlib.php | 126 ++++++++++++++++++++++++++++++++++++ mod/quiz/processattempt.php | 119 +++------------------------------- 2 files changed, 136 insertions(+), 109 deletions(-) diff --git a/mod/quiz/attemptlib.php b/mod/quiz/attemptlib.php index 4f041fa9889..39f093e4780 100644 --- a/mod/quiz/attemptlib.php +++ b/mod/quiz/attemptlib.php @@ -2044,6 +2044,132 @@ class quiz_attempt { return $url; } } + + /** + * Process responses during an attempt at a quiz. + * + * @param int $timenow time when the processing started + * @param bool $finishattempt whether to finish the attempt or not + * @param bool $timeup true if form was submitted by timer + * @param int $thispage current page number + * @return string the attempt state once the data has been processed + * @since Moodle 3.1 + * @throws moodle_exception + */ + public function process_attempt($timenow, $finishattempt, $timeup, $thispage) { + global $DB; + + $transaction = $DB->start_delegated_transaction(); + + // If there is only a very small amount of time left, there is no point trying + // to show the student another page of the quiz. Just finish now. + $graceperiodmin = null; + $accessmanager = $this->get_access_manager($timenow); + $timeclose = $accessmanager->get_end_time($this->get_attempt()); + + // Don't enforce timeclose for previews. + if ($this->is_preview()) { + $timeclose = false; + } + $toolate = false; + if ($timeclose !== false && $timenow > $timeclose - QUIZ_MIN_TIME_TO_CONTINUE) { + $timeup = true; + $graceperiodmin = get_config('quiz', 'graceperiodmin'); + if ($timenow > $timeclose + $graceperiodmin) { + $toolate = true; + } + } + + // If time is running out, trigger the appropriate action. + $becomingoverdue = false; + $becomingabandoned = false; + if ($timeup) { + if ($this->get_quiz()->overduehandling == 'graceperiod') { + if (is_null($graceperiodmin)) { + $graceperiodmin = get_config('quiz', 'graceperiodmin'); + } + if ($timenow > $timeclose + $this->get_quiz()->graceperiod + $graceperiodmin) { + // Grace period has run out. + $finishattempt = true; + $becomingabandoned = true; + } else { + $becomingoverdue = true; + } + } else { + $finishattempt = true; + } + } + + // Don't log - we will end with a redirect to a page that is logged. + + if (!$finishattempt) { + // Just process the responses for this page and go to the next page. + if (!$toolate) { + try { + $this->process_submitted_actions($timenow, $becomingoverdue); + + } catch (question_out_of_sequence_exception $e) { + throw new moodle_exception('submissionoutofsequencefriendlymessage', 'question', + $this->attempt_url(null, $thispage)); + + } catch (Exception $e) { + // This sucks, if we display our own custom error message, there is no way + // to display the original stack trace. + $debuginfo = ''; + if (!empty($e->debuginfo)) { + $debuginfo = $e->debuginfo; + } + throw new moodle_exception('errorprocessingresponses', 'question', + $this->attempt_url(null, $thispage), $e->getMessage(), $debuginfo); + } + + if (!$becomingoverdue) { + foreach ($this->get_slots() as $slot) { + if (optional_param('redoslot' . $slot, false, PARAM_BOOL)) { + $this->process_redo_question($slot, $timenow); + } + } + } + + } else { + // The student is too late. + $this->process_going_overdue($timenow, true); + } + + $transaction->allow_commit(); + + return $becomingoverdue ? self::OVERDUE : self::IN_PROGRESS; + } + + // Update the quiz attempt record. + try { + if ($becomingabandoned) { + $this->process_abandon($timenow, true); + } else { + $this->process_finish($timenow, !$toolate); + } + + } catch (question_out_of_sequence_exception $e) { + throw new moodle_exception('submissionoutofsequencefriendlymessage', 'question', + $this->attempt_url(null, $thispage)); + + } catch (Exception $e) { + // This sucks, if we display our own custom error message, there is no way + // to display the original stack trace. + $debuginfo = ''; + if (!empty($e->debuginfo)) { + $debuginfo = $e->debuginfo; + } + throw new moodle_exception('errorprocessingresponses', 'question', + $this->attempt_url(null, $thispage), $e->getMessage(), $debuginfo); + } + + // Send the user to the review page. + $transaction->allow_commit(); + + return $becomingabandoned ? self::ABANDONED : self::FINISHED; + } + } diff --git a/mod/quiz/processattempt.php b/mod/quiz/processattempt.php index a604711361e..a9dc9e2fe90 100644 --- a/mod/quiz/processattempt.php +++ b/mod/quiz/processattempt.php @@ -45,7 +45,6 @@ $finishattempt = optional_param('finishattempt', false, PARAM_BOOL); $timeup = optional_param('timeup', 0, PARAM_BOOL); // True if form was submitted by timer. $scrollpos = optional_param('scrollpos', '', PARAM_RAW); -$transaction = $DB->start_delegated_transaction(); $attemptobj = quiz_attempt::create($attemptid); // Set $nexturl now. @@ -65,25 +64,6 @@ if ($page == -1) { } } -// If there is only a very small amount of time left, there is no point trying -// to show the student another page of the quiz. Just finish now. -$graceperiodmin = null; -$accessmanager = $attemptobj->get_access_manager($timenow); -$timeclose = $accessmanager->get_end_time($attemptobj->get_attempt()); - -// Don't enforce timeclose for previews -if ($attemptobj->is_preview()) { - $timeclose = false; -} -$toolate = false; -if ($timeclose !== false && $timenow > $timeclose - QUIZ_MIN_TIME_TO_CONTINUE) { - $timeup = true; - $graceperiodmin = get_config('quiz', 'graceperiodmin'); - if ($timenow > $timeclose + $graceperiodmin) { - $toolate = true; - } -} - // Check login. require_login($attemptobj->get_course(), false, $attemptobj->get_cm()); require_sesskey(); @@ -104,93 +84,14 @@ if ($attemptobj->is_finished()) { 'attemptalreadyclosed', null, $attemptobj->review_url()); } -// If time is running out, trigger the appropriate action. -$becomingoverdue = false; -$becomingabandoned = false; -if ($timeup) { - if ($attemptobj->get_quiz()->overduehandling == 'graceperiod') { - if (is_null($graceperiodmin)) { - $graceperiodmin = get_config('quiz', 'graceperiodmin'); - } - if ($timenow > $timeclose + $attemptobj->get_quiz()->graceperiod + $graceperiodmin) { - // Grace period has run out. - $finishattempt = true; - $becomingabandoned = true; - } else { - $becomingoverdue = true; - } - } else { - $finishattempt = true; - } +// Process the attempt, getting the new status for the attempt. +$status = $attemptobj->process_attempt($timenow, $finishattempt, $timeup, $thispage); + +if ($status == quiz_attempt::OVERDUE) { + redirect($attemptobj->summary_url()); +} else if ($status == quiz_attempt::IN_PROGRESS) { + redirect($nexturl); +} else { + // Attempt abandoned or finished. + redirect($attemptobj->review_url()); } - -// Don't log - we will end with a redirect to a page that is logged. - -if (!$finishattempt) { - // Just process the responses for this page and go to the next page. - if (!$toolate) { - try { - $attemptobj->process_submitted_actions($timenow, $becomingoverdue); - - } catch (question_out_of_sequence_exception $e) { - print_error('submissionoutofsequencefriendlymessage', 'question', - $attemptobj->attempt_url(null, $thispage)); - - } catch (Exception $e) { - // This sucks, if we display our own custom error message, there is no way - // to display the original stack trace. - $debuginfo = ''; - if (!empty($e->debuginfo)) { - $debuginfo = $e->debuginfo; - } - print_error('errorprocessingresponses', 'question', - $attemptobj->attempt_url(null, $thispage), $e->getMessage(), $debuginfo); - } - - if (!$becomingoverdue) { - foreach ($attemptobj->get_slots() as $slot) { - if (optional_param('redoslot' . $slot, false, PARAM_BOOL)) { - $attemptobj->process_redo_question($slot, $timenow); - } - } - } - - } else { - // The student is too late. - $attemptobj->process_going_overdue($timenow, true); - } - - $transaction->allow_commit(); - if ($becomingoverdue) { - redirect($attemptobj->summary_url()); - } else { - redirect($nexturl); - } -} - -// Update the quiz attempt record. -try { - if ($becomingabandoned) { - $attemptobj->process_abandon($timenow, true); - } else { - $attemptobj->process_finish($timenow, !$toolate); - } - -} catch (question_out_of_sequence_exception $e) { - print_error('submissionoutofsequencefriendlymessage', 'question', - $attemptobj->attempt_url(null, $thispage)); - -} catch (Exception $e) { - // This sucks, if we display our own custom error message, there is no way - // to display the original stack trace. - $debuginfo = ''; - if (!empty($e->debuginfo)) { - $debuginfo = $e->debuginfo; - } - print_error('errorprocessingresponses', 'question', - $attemptobj->attempt_url(null, $thispage), $e->getMessage(), $debuginfo); -} - -// Send the user to the review page. -$transaction->allow_commit(); -redirect($attemptobj->review_url()); From 98e6869088d25edc56805aa64070a564cf117c70 Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Thu, 21 Jan 2016 12:55:28 +0100 Subject: [PATCH 2/2] MDL-52863 mod_quiz: New Web Service mod_quiz_process_attempt --- mod/quiz/classes/external.php | 126 +++++++++++++++++++++++++++---- mod/quiz/db/services.php | 9 +++ mod/quiz/tests/external_test.php | 119 ++++++++++++++++++++++++++++- mod/quiz/version.php | 2 +- 4 files changed, 236 insertions(+), 20 deletions(-) diff --git a/mod/quiz/classes/external.php b/mod/quiz/classes/external.php index e2dd09f5b18..9cb983ef154 100644 --- a/mod/quiz/classes/external.php +++ b/mod/quiz/classes/external.php @@ -777,19 +777,18 @@ class mod_quiz_external extends external_api { /** * Utility function for validating a given attempt * - * @param array $params Array of parameters including the attemptid and preflight data + * @param array $params array of parameters including the attemptid and preflight data + * @param bool $checkaccessrules whether to check the quiz access rules or not + * @param bool $failifoverdue whether to return error if the attempt is overdue * @return array containing the attempt object and access messages * @throws moodle_quiz_exception * @since Moodle 3.1 */ - protected static function validate_attempt($params) { + protected static function validate_attempt($params, $checkaccessrules = true, $failifoverdue = true) { global $USER; $attemptobj = quiz_attempt::create($params['attemptid']); - // If the attempt is now overdue, or abandoned, deal with that. - $attemptobj->handle_if_time_expired(time(), true); - $context = context_module::instance($attemptobj->get_cm()->id); self::validate_context($context); @@ -804,21 +803,26 @@ class mod_quiz_external extends external_api { $attemptobj->require_capability('mod/quiz:attempt'); } + // Check the access rules. + $accessmanager = $attemptobj->get_access_manager(time()); + $messages = array(); + if ($checkaccessrules) { + // If the attempt is now overdue, or abandoned, deal with that. + $attemptobj->handle_if_time_expired(time(), true); + + $messages = $accessmanager->prevent_access(); + if (!$ispreviewuser && $messages) { + throw new moodle_quiz_exception($attemptobj->get_quizobj(), 'attempterror'); + } + } + // Attempt closed?. if ($attemptobj->is_finished()) { throw new moodle_quiz_exception($attemptobj->get_quizobj(), 'attemptalreadyclosed'); - } else if ($attemptobj->get_state() == quiz_attempt::OVERDUE) { + } else if ($failifoverdue && $attemptobj->get_state() == quiz_attempt::OVERDUE) { throw new moodle_quiz_exception($attemptobj->get_quizobj(), 'stateoverdue'); } - // Check the access rules. - $accessmanager = $attemptobj->get_access_manager(time()); - - $messages = $accessmanager->prevent_access(); - if (!$ispreviewuser && $messages) { - throw new moodle_quiz_exception($attemptobj->get_quizobj(), 'attempterror'); - } - // User submitted data (like the quiz password). if ($accessmanager->is_preflight_check_required($attemptobj->get_attemptid())) { $provideddata = array(); @@ -1049,7 +1053,7 @@ class mod_quiz_external extends external_api { ); $params = self::validate_parameters(self::get_attempt_summary_parameters(), $params); - list($attemptobj, $messages) = self::validate_attempt($params); + list($attemptobj, $messages) = self::validate_attempt($params, true, false); $result = array(); $result['warnings'] = $warnings; @@ -1158,4 +1162,96 @@ class mod_quiz_external extends external_api { ); } + /** + * Describes the parameters for process_attempt. + * + * @return external_external_function_parameters + * @since Moodle 3.1 + */ + public static function process_attempt_parameters() { + return new external_function_parameters ( + array( + 'attemptid' => new external_value(PARAM_INT, 'attempt id'), + 'data' => new external_multiple_structure( + new external_single_structure( + array( + 'name' => new external_value(PARAM_RAW, 'data name'), + 'value' => new external_value(PARAM_RAW, 'data value'), + ) + ), + 'the data to be saved', VALUE_DEFAULT, array() + ), + 'finishattempt' => new external_value(PARAM_BOOL, 'whether to finish or not the attempt', VALUE_DEFAULT, false), + 'timeup' => new external_value(PARAM_BOOL, 'whether the WS was called by a timer when the time is up', + VALUE_DEFAULT, false), + 'preflightdata' => new external_multiple_structure( + new external_single_structure( + array( + 'name' => new external_value(PARAM_ALPHANUMEXT, 'data name'), + 'value' => new external_value(PARAM_RAW, 'data value'), + ) + ), 'Preflight required data (like passwords)', VALUE_DEFAULT, array() + ) + ) + ); + } + + /** + * Process responses during an attempt at a quiz and also deals with attempts finishing. + * + * @param int $attemptid attempt id + * @param array $data the data to be saved + * @param bool $finishattempt whether to finish or not the attempt + * @param bool $timeup whether the WS was called by a timer when the time is up + * @param array $preflightdata preflight required data (like passwords) + * @return array of warnings and the attempt state after the processing + * @since Moodle 3.1 + */ + public static function process_attempt($attemptid, $data, $finishattempt = false, $timeup = false, $preflightdata = array()) { + + $warnings = array(); + + $params = array( + 'attemptid' => $attemptid, + 'data' => $data, + 'finishattempt' => $finishattempt, + 'timeup' => $timeup, + 'preflightdata' => $preflightdata, + ); + $params = self::validate_parameters(self::process_attempt_parameters(), $params); + + // Do not check access manager rules. + list($attemptobj, $messages) = self::validate_attempt($params, false); + + // Create the $_POST object required by the question engine. + $_POST = array(); + foreach ($params['data'] as $element) { + $_POST[$element['name']] = $element['value']; + } + $timenow = time(); + $finishattempt = $params['finishattempt']; + $timeup = $params['timeup']; + + $result = array(); + $result['state'] = $attemptobj->process_attempt($timenow, $finishattempt, $timeup, 0); + $result['warnings'] = $warnings; + return $result; + } + + /** + * Describes the process_attempt return value. + * + * @return external_single_structure + * @since Moodle 3.1 + */ + public static function process_attempt_returns() { + return new external_single_structure( + array( + 'state' => new external_value(PARAM_ALPHANUMEXT, 'state: the new attempt state: + inprogress, finished, overdue, abandoned'), + 'warnings' => new external_warnings(), + ) + ); + } + } diff --git a/mod/quiz/db/services.php b/mod/quiz/db/services.php index 24b3e38c213..0b68d9b9122 100644 --- a/mod/quiz/db/services.php +++ b/mod/quiz/db/services.php @@ -110,4 +110,13 @@ $functions = array( 'capabilities' => 'mod/quiz:attempt', 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE) ), + + 'mod_quiz_process_attempt' => array( + 'classname' => 'mod_quiz_external', + 'methodname' => 'process_attempt', + 'description' => 'Process responses during an attempt at a quiz and also deals with attempts finishing.', + 'type' => 'write', + 'capabilities' => 'mod/quiz:attempt', + 'services' => array(MOODLE_OFFICIAL_MOBILE_SERVICE) + ), ); diff --git a/mod/quiz/tests/external_test.php b/mod/quiz/tests/external_test.php index 6fab602d01d..5b02191f3f7 100644 --- a/mod/quiz/tests/external_test.php +++ b/mod/quiz/tests/external_test.php @@ -44,10 +44,12 @@ class testable_mod_quiz_external extends mod_quiz_external { * Public accessor. * * @param array $params Array of parameters including the attemptid and preflight data + * @param bool $checkaccessrules whether to check the quiz access rules or not + * @param bool $failifoverdue whether to return error if the attempt is overdue * @return array containing the attempt object and access messages */ - public static function validate_attempt($params) { - return parent::validate_attempt($params); + public static function validate_attempt($params, $checkaccessrules = true, $failifoverdue = true) { + return parent::validate_attempt($params, $checkaccessrules, $failifoverdue); } } @@ -779,11 +781,16 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { $quiz->timeopen = time() - WEEKSECS; $quiz->timeclose = time() - DAYSECS; $DB->update_record('quiz', $quiz); + + // This should work, ommit access rules. + testable_mod_quiz_external::validate_attempt($params, false); + + // Get a generic error because prior to checking the dates the attempt is closed. try { testable_mod_quiz_external::validate_attempt($params); $this->fail('Exception expected due to passed dates.'); } catch (moodle_quiz_exception $e) { - $this->assertEquals('attemptalreadyclosed', $e->errorcode); + $this->assertEquals('attempterror', $e->errorcode); } // Finish the attempt. @@ -791,7 +798,7 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { $attemptobj->process_finish(time(), false); try { - testable_mod_quiz_external::validate_attempt($params); + testable_mod_quiz_external::validate_attempt($params, false); $this->fail('Exception expected due to attempt finished.'); } catch (moodle_quiz_exception $e) { $this->assertEquals('attemptalreadyclosed', $e->errorcode); @@ -1010,4 +1017,108 @@ class mod_quiz_external_testcase extends externallib_advanced_testcase { } + /** + * Test process_attempt + */ + public function test_process_attempt() { + global $DB; + + // Create a new quiz with two questions and one attempt started. + list($quiz, $context, $quizobj, $attempt, $attemptobj, $quba) = $this->create_quiz_with_questions(true); + + // Response for slot 1. + $prefix = $quba->get_field_prefix(1); + $data = array( + array('name' => 'slots', 'value' => 1), + array('name' => $prefix . ':sequencecheck', + 'value' => $attemptobj->get_question_attempt(1)->get_sequence_check_count()), + array('name' => $prefix . 'answer', 'value' => 1), + ); + + $this->setUser($this->student); + + $result = mod_quiz_external::process_attempt($attempt->id, $data); + $result = external_api::clean_returnvalue(mod_quiz_external::process_attempt_returns(), $result); + $this->assertEquals(quiz_attempt::IN_PROGRESS, $result['state']); + + // Now, get the summary. + $result = mod_quiz_external::get_attempt_summary($attempt->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_attempt_summary_returns(), $result); + + // Check it's marked as completed only the first one. + $this->assertEquals('complete', $result['questions'][0]['state']); + $this->assertEquals('todo', $result['questions'][1]['state']); + $this->assertEquals(1, $result['questions'][0]['number']); + $this->assertEquals(2, $result['questions'][1]['number']); + $this->assertFalse($result['questions'][0]['flagged']); + $this->assertFalse($result['questions'][1]['flagged']); + $this->assertEmpty($result['questions'][0]['mark']); + $this->assertEmpty($result['questions'][1]['mark']); + + // Now, second slot. + $prefix = $quba->get_field_prefix(2); + $data = array( + array('name' => 'slots', 'value' => 2), + array('name' => $prefix . ':sequencecheck', + 'value' => $attemptobj->get_question_attempt(1)->get_sequence_check_count()), + array('name' => $prefix . 'answer', 'value' => 1), + array('name' => $prefix . ':flagged', 'value' => 1), + ); + + $result = mod_quiz_external::process_attempt($attempt->id, $data); + $result = external_api::clean_returnvalue(mod_quiz_external::process_attempt_returns(), $result); + $this->assertEquals(quiz_attempt::IN_PROGRESS, $result['state']); + + // Now, get the summary. + $result = mod_quiz_external::get_attempt_summary($attempt->id); + $result = external_api::clean_returnvalue(mod_quiz_external::get_attempt_summary_returns(), $result); + + // Check it's marked as completed only the first one. + $this->assertEquals('complete', $result['questions'][0]['state']); + $this->assertEquals('complete', $result['questions'][1]['state']); + $this->assertFalse($result['questions'][0]['flagged']); + $this->assertTrue($result['questions'][1]['flagged']); + + // Finish the attempt. + $result = mod_quiz_external::process_attempt($attempt->id, array(), true); + $result = external_api::clean_returnvalue(mod_quiz_external::process_attempt_returns(), $result); + $this->assertEquals(quiz_attempt::FINISHED, $result['state']); + + // Start new 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, 2, false, $timenow, false, $this->student->id); + quiz_start_new_attempt($quizobj, $quba, $attempt, 2, $timenow); + quiz_attempt_save_started($quizobj, $quba, $attempt); + + // Force grace period, attempt going to overdue. + $quiz->timeclose = $timenow - 10; + $quiz->graceperiod = 60; + $quiz->overduehandling = 'graceperiod'; + $DB->update_record('quiz', $quiz); + + $result = mod_quiz_external::process_attempt($attempt->id, array()); + $result = external_api::clean_returnvalue(mod_quiz_external::process_attempt_returns(), $result); + $this->assertEquals(quiz_attempt::OVERDUE, $result['state']); + + // New attempt. + $timenow = time(); + $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, 3, 2, $timenow, false, $this->student->id); + quiz_start_new_attempt($quizobj, $quba, $attempt, 3, $timenow); + quiz_attempt_save_started($quizobj, $quba, $attempt); + + // Force abandon. + $quiz->timeclose = $timenow - HOURSECS; + $DB->update_record('quiz', $quiz); + + $result = mod_quiz_external::process_attempt($attempt->id, array()); + $result = external_api::clean_returnvalue(mod_quiz_external::process_attempt_returns(), $result); + $this->assertEquals(quiz_attempt::ABANDONED, $result['state']); + + } + } diff --git a/mod/quiz/version.php b/mod/quiz/version.php index e332f17e966..fc8f6adaac1 100644 --- a/mod/quiz/version.php +++ b/mod/quiz/version.php @@ -24,7 +24,7 @@ defined('MOODLE_INTERNAL') || die(); -$plugin->version = 2015111610; +$plugin->version = 2015111611; $plugin->requires = 2015111000; $plugin->component = 'mod_quiz'; $plugin->cron = 60;