MDL-75210 mod_quiz: fix sequential quiz access
* As a student I should only be able to access the next question or the current question * The API should not show more than the current question * Once the quiz is previewed we can see the question in any order (existing behaviour) * Related to ticket MDL-71728 Co-authored-by: Rajneel Totaram <[email protected]> Co-authored-by: Tim Hunt <[email protected]>
This commit is contained in:
Laurent David
committed by
Ilya Tregubov
Ilya Tregubov
co-authored by
Rajneel Totaram
Tim Hunt
Rajneel Totaram
Tim Hunt
parent
d0f6744d66
commit
025e0297b6
+15
-9
@@ -2502,19 +2502,25 @@ class quiz_attempt {
|
||||
}
|
||||
|
||||
/**
|
||||
* Check a page access to see if is an out of sequence access.
|
||||
* Check a page read access to see if is an out of sequence access.
|
||||
*
|
||||
* @param int $page page number.
|
||||
* @return boolean false is is an out of sequence access, true otherwise.
|
||||
* If allownext is set then we also check whether access to the page
|
||||
* after the current one should be permitted.
|
||||
*
|
||||
* @param int $page page number.
|
||||
* @param bool $allownext in case of a sequential navigation, can we go to next page ?
|
||||
* @return boolean false is an out of sequence access, true otherwise.
|
||||
* @since Moodle 3.1
|
||||
*/
|
||||
public function check_page_access($page) {
|
||||
if ($this->get_currentpage() != $page) {
|
||||
if ($this->get_navigation_method() == QUIZ_NAVMETHOD_SEQ && $this->get_currentpage() > $page) {
|
||||
return false;
|
||||
}
|
||||
public function check_page_access(int $page, bool $allownext = true): bool {
|
||||
if ($this->get_navigation_method() != QUIZ_NAVMETHOD_SEQ) {
|
||||
return true;
|
||||
}
|
||||
return true;
|
||||
// Sequential access: allow access to the summary, current page or next page.
|
||||
// Or if the user review his/her attempt, see MDLQA-1523.
|
||||
return $page == -1
|
||||
|| $page == $this->get_currentpage()
|
||||
|| $allownext && ($page == $this->get_currentpage() + 1);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1009,8 +1009,9 @@ class mod_quiz_external extends external_api {
|
||||
if ($displayoptions->marks >= question_display_options::MARK_AND_MAX) {
|
||||
$question['mark'] = $attemptobj->get_question_mark($slot);
|
||||
}
|
||||
|
||||
$questions[] = $question;
|
||||
if ($attemptobj->check_page_access($attemptobj->get_question_page($slot), false)) {
|
||||
$questions[] = $question;
|
||||
}
|
||||
}
|
||||
return $questions;
|
||||
}
|
||||
|
||||
@@ -40,9 +40,10 @@ class mod_quiz_attempt_testcase extends advanced_testcase {
|
||||
* Create quiz and attempt data with layout.
|
||||
*
|
||||
* @param string $layout layout to set. Like quiz attempt.layout. E.g. '1,2,0,3,4,0,'.
|
||||
* @param string $navmethod quiz navigation method (defaults to free)
|
||||
* @return quiz_attempt the new quiz_attempt object
|
||||
*/
|
||||
protected function create_quiz_and_attempt_with_layout($layout) {
|
||||
protected function create_quiz_and_attempt_with_layout($layout, $navmethod = QUIZ_NAVMETHOD_FREE) {
|
||||
$this->resetAfterTest(true);
|
||||
|
||||
// Make a user to do the quiz.
|
||||
@@ -51,7 +52,7 @@ class mod_quiz_attempt_testcase extends advanced_testcase {
|
||||
// Make a quiz.
|
||||
$quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz');
|
||||
$quiz = $quizgenerator->create_instance(['course' => $course->id,
|
||||
'grade' => 100.0, 'sumgrades' => 2, 'layout' => $layout]);
|
||||
'grade' => 100.0, 'sumgrades' => 2, 'layout' => $layout, 'navmethod' => $navmethod]);
|
||||
|
||||
$quizobj = quiz::create($quiz->id, $user->id);
|
||||
|
||||
@@ -377,4 +378,58 @@ class mod_quiz_attempt_testcase extends advanced_testcase {
|
||||
$step = $quba->get_question_attempt(1)->get_step(0);
|
||||
$this->assertEquals($student1->id, $step->get_user_id());
|
||||
}
|
||||
|
||||
/**
|
||||
* Test check_page_access function
|
||||
* @covers \quiz_attempt::check_page_access
|
||||
*/
|
||||
public function test_check_page_access() {
|
||||
$timenow = time();
|
||||
|
||||
// Free navigation.
|
||||
$attempt = $this->create_quiz_and_attempt_with_layout('1,0,2,0,3,0,4,0,5,0', QUIZ_NAVMETHOD_FREE);
|
||||
|
||||
// Check access.
|
||||
$this->assertTrue($attempt->check_page_access(4));
|
||||
$this->assertTrue($attempt->check_page_access(3));
|
||||
$this->assertTrue($attempt->check_page_access(2));
|
||||
$this->assertTrue($attempt->check_page_access(1));
|
||||
$this->assertTrue($attempt->check_page_access(0));
|
||||
$this->assertTrue($attempt->check_page_access(2));
|
||||
|
||||
// Access page 2.
|
||||
$attempt->set_currentpage(2);
|
||||
$attempt = quiz_attempt::create($attempt->get_attempt()->id);
|
||||
|
||||
// Check access.
|
||||
$this->assertTrue($attempt->check_page_access(0));
|
||||
$this->assertTrue($attempt->check_page_access(1));
|
||||
$this->assertTrue($attempt->check_page_access(2));
|
||||
$this->assertTrue($attempt->check_page_access(3));
|
||||
$this->assertTrue($attempt->check_page_access(4));
|
||||
|
||||
// Sequential navigation.
|
||||
$attempt = $this->create_quiz_and_attempt_with_layout('1,0,2,0,3,0,4,0,5,0', QUIZ_NAVMETHOD_SEQ);
|
||||
|
||||
// Check access.
|
||||
$this->assertTrue($attempt->check_page_access(0));
|
||||
$this->assertTrue($attempt->check_page_access(1));
|
||||
$this->assertFalse($attempt->check_page_access(2));
|
||||
$this->assertFalse($attempt->check_page_access(3));
|
||||
$this->assertFalse($attempt->check_page_access(4));
|
||||
|
||||
// Access page 1.
|
||||
$attempt->set_currentpage(1);
|
||||
$attempt = quiz_attempt::create($attempt->get_attempt()->id);
|
||||
$this->assertTrue($attempt->check_page_access(1));
|
||||
|
||||
// Access page 2.
|
||||
$attempt->set_currentpage(2);
|
||||
$attempt = quiz_attempt::create($attempt->get_attempt()->id);
|
||||
$this->assertTrue($attempt->check_page_access(2));
|
||||
|
||||
$this->assertTrue($attempt->check_page_access(3));
|
||||
$this->assertFalse($attempt->check_page_access(4));
|
||||
$this->assertFalse($attempt->check_page_access(1));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
@mod @mod_quiz
|
||||
Feature: Attempt a quiz in a sequential mode
|
||||
As a student I should not be able to see the previous questions
|
||||
|
||||
Background:
|
||||
Given the following "users" exist:
|
||||
| username | firstname | lastname | email |
|
||||
| student | Student | One | student@example.com |
|
||||
| teacher | Teacher | One | teacher@example.com |
|
||||
And the following "courses" exist:
|
||||
| fullname | shortname | category |
|
||||
| Course 1 | C1 | 0 |
|
||||
And the following "course enrolments" exist:
|
||||
| user | course | role |
|
||||
| student | C1 | student |
|
||||
| teacher | C1 | teacher |
|
||||
And the following "question categories" exist:
|
||||
| contextlevel | reference | name |
|
||||
| Course | C1 | Test questions |
|
||||
And the following "questions" exist:
|
||||
| questioncategory | qtype | name | questiontext |
|
||||
| Test questions | truefalse | TF1 | First question |
|
||||
| Test questions | truefalse | TF2 | Second question |
|
||||
| Test questions | truefalse | TF3 | Third question |
|
||||
| Test questions | truefalse | TF4 | Fourth question |
|
||||
And the following "activities" exist:
|
||||
| activity | name | intro | course | idnumber | preferredbehaviour | navmethod |
|
||||
| quiz | Quiz 1 | Quiz 1 description | C1 | quiz1 | immediatefeedback | sequential |
|
||||
And quiz "Quiz 1" contains the following questions:
|
||||
| question | page | requireprevious |
|
||||
| TF1 | 1 | 1 |
|
||||
| TF2 | 2 | 1 |
|
||||
| TF3 | 3 | 1 |
|
||||
| TF4 | 4 | 1 |
|
||||
|
||||
@javascript
|
||||
Scenario Outline: As a student I should not be able to navigate out of sequence if sequential navigation is on.
|
||||
Given I am on the "Quiz 1" "mod_quiz > View" page logged in as "student"
|
||||
And I press "Attempt quiz"
|
||||
And I should see "First question"
|
||||
When I am on the "Quiz 1 > student > Attempt 1 > <pagenumber>" "mod_quiz > Attempt view" page
|
||||
And I should see "<canseequestion>"
|
||||
Then I should not see "<cannotseequestion>"
|
||||
Examples:
|
||||
| pagenumber | canseequestion | cannotseequestion |
|
||||
| 1 | First question | Second question |
|
||||
| 2 | Second question | First question |
|
||||
| 4 | First question | Fourth question |
|
||||
|
||||
@javascript
|
||||
Scenario: As a student I should not be able to navigate out of sequence by opening new windows on the same quiz.
|
||||
Given the following config values are set as admin:
|
||||
| config | value | plugin |
|
||||
| autosaveperiod | 60 | quiz |
|
||||
And I am on the "Quiz 1" "mod_quiz > View" page logged in as "student"
|
||||
And I press "Attempt quiz"
|
||||
And I should see "First question"
|
||||
And I click on "True" "radio" in the "First question" "question"
|
||||
And I click on "Next page" "button"
|
||||
When I am on the "Quiz 1 > student > Attempt 1 > 3" "mod_quiz > Attempt view" page
|
||||
And I click on "True" "radio" in the "Third question" "question"
|
||||
And I should see "Third question"
|
||||
And I click on "Next page" "button"
|
||||
And I am on the "Quiz 1 > student > Attempt 1 > 1" "mod_quiz > Attempt view" page
|
||||
Then I should see "Fourth question"
|
||||
|
||||
@javascript
|
||||
Scenario: As a student I should not be able to save my data by opening a given page out of sequence.
|
||||
Given the following config values are set as admin:
|
||||
| config | value | plugin |
|
||||
| autosaveperiod | 1 | quiz |
|
||||
When I am on the "Quiz 1" "mod_quiz > View" page logged in as "student"
|
||||
And I press "Attempt quiz"
|
||||
And I am on the "Quiz 1 > student > Attempt 1 > 2" "mod_quiz > Attempt view" page
|
||||
And I should see "Second question"
|
||||
And I click on "True" "radio" in the "Second question" "question"
|
||||
And I wait "2" seconds
|
||||
And I am on the "Quiz 1 > student > Attempt 1 > 1" "mod_quiz > Attempt view" page
|
||||
Then I should see "Second question"
|
||||
|
||||
@javascript
|
||||
Scenario: As a student I can review question I have finished in any order
|
||||
Given user "student" has attempted "Quiz 1" with responses:
|
||||
| slot | response |
|
||||
| 1 | True |
|
||||
| 2 | False |
|
||||
| 3 | False |
|
||||
| 4 | False |
|
||||
When I am on the "Quiz 1" "mod_quiz > View" page logged in as "student"
|
||||
And I follow "Review"
|
||||
And I am on the "Quiz 1 > student > Attempt 1 > 3" "mod_quiz > Attempt view" page
|
||||
And I should see "Third question"
|
||||
And I am on the "Quiz 1 > student > Attempt 1 > 2" "mod_quiz > Attempt view" page
|
||||
Then I should see "Second question"
|
||||
@@ -112,7 +112,21 @@ class behat_mod_quiz extends behat_question_base {
|
||||
case 'manual grading report':
|
||||
return new moodle_url('/mod/quiz/report.php',
|
||||
['id' => $this->get_cm_by_quiz_name($identifier)->id, 'mode' => 'grading']);
|
||||
|
||||
case 'attempt view':
|
||||
list($quizname, $username, $attemptno, $pageno) = explode(' > ', $identifier);
|
||||
$pageno = intval($pageno);
|
||||
$pageno = $pageno > 0 ? $pageno - 1 : 0;
|
||||
$attemptno = (int) trim(str_replace ('Attempt', '', $attemptno));
|
||||
$quiz = $this->get_quiz_by_name($quizname);
|
||||
$quizcm = get_coursemodule_from_instance('quiz', $quiz->id, $quiz->course);
|
||||
$user = $DB->get_record('user', ['username' => $username], '*', MUST_EXIST);
|
||||
$attempt = $DB->get_record('quiz_attempts',
|
||||
['quiz' => $quiz->id, 'userid' => $user->id, 'attempt' => $attemptno], '*', MUST_EXIST);
|
||||
return new moodle_url('/mod/quiz/attempt.php', [
|
||||
'attempt' => $attempt->id,
|
||||
'cmid' => $quizcm->id,
|
||||
'page' => $pageno
|
||||
]);
|
||||
case 'attempt review':
|
||||
if (substr_count($identifier, ' > ') !== 2) {
|
||||
throw new coding_exception('For "attempt review", name must be ' .
|
||||
|
||||
+134
-1
@@ -1625,7 +1625,7 @@ class external_test extends externallib_advanced_testcase {
|
||||
$this->assertEventContextNotUsed($event);
|
||||
$this->assertNotEmpty($event->get_name());
|
||||
|
||||
// Now, force the quiz with QUIZ_NAVMETHOD_SEQ (sequencial) navigation method.
|
||||
// Now, force the quiz with QUIZ_NAVMETHOD_SEQ (sequential) navigation method.
|
||||
$DB->set_field('quiz', 'navmethod', QUIZ_NAVMETHOD_SEQ, array('id' => $quiz->id));
|
||||
// Quiz requiring preflightdata.
|
||||
$DB->set_field('quiz', 'password', 'abcdef', array('id' => $quiz->id));
|
||||
@@ -2013,4 +2013,137 @@ class external_test extends externallib_advanced_testcase {
|
||||
|
||||
$this->assertEquals($expected, $result['questiontypes']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that a sequential navigation quiz is not allowing to see questions in advance except if reviewing
|
||||
*/
|
||||
public function test_sequential_navigation_view_attempt() {
|
||||
// Test user with full capabilities.
|
||||
$quiz = $this->prepare_sequential_quiz();
|
||||
$attemptobj = $this->create_quiz_attempt_object($quiz);
|
||||
$this->setUser($this->student);
|
||||
// Check out of sequence access for view.
|
||||
$this->assertNotEmpty(mod_quiz_external::view_attempt($attemptobj->get_attemptid(), 0, []));
|
||||
try {
|
||||
mod_quiz_external::view_attempt($attemptobj->get_attemptid(), 3, []);
|
||||
$this->fail('Exception expected due to out of sequence access.');
|
||||
} catch (\moodle_exception $e) {
|
||||
$this->assertStringContainsString('quiz/Out of sequence access', $e->getMessage());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that a sequential navigation quiz is not allowing to see questions in advance for a student
|
||||
*/
|
||||
public function test_sequential_navigation_attempt_summary() {
|
||||
// Test user with full capabilities.
|
||||
$quiz = $this->prepare_sequential_quiz();
|
||||
$attemptobj = $this->create_quiz_attempt_object($quiz);
|
||||
$this->setUser($this->student);
|
||||
// Check that we do not return other questions than the one currently viewed.
|
||||
$result = mod_quiz_external::get_attempt_summary($attemptobj->get_attemptid());
|
||||
$this->assertCount(1, $result['questions']);
|
||||
$this->assertStringContainsString('Question (1)', $result['questions'][0]['html']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test that a sequential navigation quiz is not allowing to see questions in advance for student
|
||||
*/
|
||||
public function test_sequential_navigation_get_attempt_data() {
|
||||
// Test user with full capabilities.
|
||||
$quiz = $this->prepare_sequential_quiz();
|
||||
$attemptobj = $this->create_quiz_attempt_object($quiz);
|
||||
$this->setUser($this->student);
|
||||
// Test invalid instance id.
|
||||
try {
|
||||
mod_quiz_external::get_attempt_data($attemptobj->get_attemptid(), 2);
|
||||
$this->fail('Exception expected due to out of sequence access.');
|
||||
} catch (\moodle_exception $e) {
|
||||
$this->assertStringContainsString('quiz/Out of sequence access', $e->getMessage());
|
||||
}
|
||||
// Now we moved to page 1, we should see page 2 and 1 but not 0 or 3.
|
||||
$attemptobj->set_currentpage(1);
|
||||
// Test invalid instance id.
|
||||
try {
|
||||
mod_quiz_external::get_attempt_data($attemptobj->get_attemptid(), 0);
|
||||
$this->fail('Exception expected due to out of sequence access.');
|
||||
} catch (\moodle_exception $e) {
|
||||
$this->assertStringContainsString('quiz/Out of sequence access', $e->getMessage());
|
||||
}
|
||||
|
||||
try {
|
||||
mod_quiz_external::get_attempt_data($attemptobj->get_attemptid(), 3);
|
||||
$this->fail('Exception expected due to out of sequence access.');
|
||||
} catch (\moodle_exception $e) {
|
||||
$this->assertStringContainsString('quiz/Out of sequence access', $e->getMessage());
|
||||
}
|
||||
|
||||
// Now we can see page 1.
|
||||
$result = mod_quiz_external::get_attempt_data($attemptobj->get_attemptid(), 1);
|
||||
$this->assertCount(1, $result['questions']);
|
||||
$this->assertStringContainsString('Question (2)', $result['questions'][0]['html']);
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepare quiz for sequential navigation tests
|
||||
*
|
||||
* @return quiz
|
||||
*/
|
||||
private function prepare_sequential_quiz(): quiz {
|
||||
// Create a new quiz with 5 questions and one attempt started.
|
||||
// Create a new quiz with attempts.
|
||||
$quizgenerator = $this->getDataGenerator()->get_plugin_generator('mod_quiz');
|
||||
$data = [
|
||||
'course' => $this->course->id,
|
||||
'sumgrades' => 2,
|
||||
'preferredbehaviour' => 'deferredfeedback',
|
||||
'navmethod' => QUIZ_NAVMETHOD_SEQ
|
||||
];
|
||||
$quiz = $quizgenerator->create_instance($data);
|
||||
|
||||
// Now generate the questions.
|
||||
$questiongenerator = $this->getDataGenerator()->get_plugin_generator('core_question');
|
||||
$cat = $questiongenerator->create_question_category();
|
||||
for ($pageindex = 1; $pageindex <= 5; $pageindex++) {
|
||||
$question = $questiongenerator->create_question('truefalse', null, [
|
||||
'category' => $cat->id,
|
||||
'questiontext' => ['text' => "Question ($pageindex)"]
|
||||
]);
|
||||
quiz_add_quiz_question($question->id, $quiz, $pageindex);
|
||||
}
|
||||
|
||||
$quizobj = quiz::create($quiz->id, $this->student->id);
|
||||
// Set grade to pass.
|
||||
$item = \grade_item::fetch(array('courseid' => $this->course->id, 'itemtype' => 'mod',
|
||||
'itemmodule' => 'quiz', 'iteminstance' => $quiz->id, 'outcomeid' => null));
|
||||
$item->gradepass = 80;
|
||||
$item->update();
|
||||
return $quizobj;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create question attempt
|
||||
*
|
||||
* @param quiz $quizobj
|
||||
* @param int|null $userid
|
||||
* @param bool|null $ispreview
|
||||
* @return quiz_attempt
|
||||
* @throws \moodle_exception
|
||||
*/
|
||||
private function create_quiz_attempt_object(quiz $quizobj, ?int $userid = null, ?bool $ispreview = false): quiz_attempt {
|
||||
global $USER;
|
||||
$timenow = time();
|
||||
// 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);
|
||||
$attemptnumber = 1;
|
||||
if (!empty($USER->id)) {
|
||||
$attemptnumber = count(quiz_get_user_attempts($quizobj->get_quizid(), $USER->id)) + 1;
|
||||
}
|
||||
$attempt = quiz_create_attempt($quizobj, $attemptnumber, false, $timenow, $ispreview, $userid ?? $this->student->id);
|
||||
quiz_start_new_attempt($quizobj, $quba, $attempt, $attemptnumber, $timenow);
|
||||
quiz_attempt_save_started($quizobj, $quba, $attempt);
|
||||
$attemptobj = quiz_attempt::create($attempt->id);
|
||||
return $attemptobj;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
This files describes API changes in the quiz code.
|
||||
|
||||
=== 4.0.3, 4.1 ===
|
||||
|
||||
* If a quiz is sequential the external functions mod_quiz_external::get_attempt_data, mod_quiz_external::get_attempt_summary will not return any data
|
||||
related to out of sequence question except while reviewing the quiz (MDL-75210).
|
||||
|
||||
=== 4.0.2, 4.1 ===
|
||||
|
||||
* No external code should be calling quiz_overview_report::regrade_attempt because it is an
|
||||
@@ -7,7 +12,6 @@ This files describes API changes in the quiz code.
|
||||
that the API changed slightly. It now returns an array listing any questions which could
|
||||
not be regraded.
|
||||
|
||||
|
||||
=== 4.0 ===
|
||||
|
||||
* The following API methods have a new parameter, $studentisonline, to define whether the student is currently interacting:
|
||||
|
||||
Reference in New Issue
Block a user