MDL-59606 quiz responses: fix edge cases in the report
authorTim Hunt <T.J.Hunt@open.ac.uk>
Mon, 31 Jul 2017 16:58:29 +0000 (18:58 +0200)
committerTim Hunt <T.J.Hunt@open.ac.uk>
Fri, 27 Oct 2017 15:57:01 +0000 (16:57 +0100)
This patch conbines valuable contributions from Kashmira Nagwekar and
Luca Bösch. Many thanks to them. However, the final form of the fix,
and hence the blame, falls to me -- Tim.

There were several issues here:

* The load_questions_usages_by_activity method in
  question/engine/datalib.php was incorrectly treating the case
  when no data was returned. (Looks like a historic copy-pase from
  other methods that fetch one item by unique id, which therefore
  must exist.)

* The report was not correctly handling the display when the 'Which
  tries' was set to 'with, and without, attempts'.

* It was possible to select the 'All tries' option when also saying
  'Users without attempts'. This combination makes not sense, so
  a disabledIf rule was added to the form.

mod/quiz/report/overview/tests/behat/basic.feature
mod/quiz/report/responses/first_or_all_responses_table.php
mod/quiz/report/responses/responses_form.php
mod/quiz/report/responses/tests/behat/basic.feature [new file with mode: 0644]
question/engine/datalib.php
question/type/numerical/tests/helper.php

index 8612ffd..8799c79 100644 (file)
@@ -1,4 +1,4 @@
-@mod @mod_quiz
+@mod @mod_quiz @quiz @quiz_overview
 Feature: Basic use of the Grades report
   In order to easily get an overview of quiz attempts
   As a teacher
index 4a90a08..4f2600a 100644 (file)
@@ -68,6 +68,15 @@ class quiz_first_or_all_responses_table extends quiz_last_responses_table {
         // Insert an extra field in attempt data and extra rows where necessary.
         $newrawdata = array();
         foreach ($this->rawdata as $attempt) {
+            if (!isset($this->questionusagesbyactivity[$attempt->usageid])) {
+                // This is a user without attempts.
+                $attempt->try = 0;
+                $attempt->lasttryforallparts = true;
+                $newrawdata[] = $attempt;
+                continue;
+            }
+
+            // We have an attempt, which may require several rows.
             $maxtriesinanyslot = 1;
             foreach ($this->questionusagesbyactivity[$attempt->usageid]->get_slots() as $slot) {
                 $tries = $this->get_no_of_tries($attempt, $slot);
@@ -230,7 +239,7 @@ class quiz_first_or_all_responses_table extends quiz_last_responses_table {
      * @return string   What to put in the cell for this column, for this row data.
      */
     public function col_email($tablerow) {
-        if ($tablerow->try != 1) {
+        if ($tablerow->try > 1) {
             return '';
         } else {
             return $tablerow->email;
@@ -244,18 +253,27 @@ class quiz_first_or_all_responses_table extends quiz_last_responses_table {
      * @return string   What to put in the cell for this column, for this row data.
      */
     public function col_sumgrades($tablerow) {
-        if (!$tablerow->lasttryforallparts) {
+        if ($tablerow->try == 0) {
+            // We are showing a user without a quiz attempt.
+            return '-';
+        } else if (!$tablerow->lasttryforallparts) {
+            // There are more rows to come for this quiz attempt, so we will show this later.
             return '';
         } else {
+            // Last row for this attempt. Now is the time to show attempt-related data.
             return parent::col_sumgrades($tablerow);
         }
     }
 
-
     public function col_state($tablerow) {
-        if (!$tablerow->lasttryforallparts) {
+        if ($tablerow->try == 0) {
+            // We are showing a user without a quiz attempt.
+            return '-';
+        } else if (!$tablerow->lasttryforallparts) {
+            // There are more rows to come for this quiz attempt, so we will show this later.
             return '';
         } else {
+            // Last row for this attempt. Now is the time to show attempt-related data.
             return parent::col_state($tablerow);
         }
     }
index 461dcd2..4fdbe90 100644 (file)
@@ -70,6 +70,7 @@ class quiz_responses_settings_form extends mod_quiz_attempts_report_form {
                                            question_attempt::ALL_TRIES    => get_string('alltries', 'question'))
             );
             $mform->setDefault('whichtries', question_attempt::LAST_TRY);
+            $mform->disabledIf('whichtries', 'attempts', 'eq', quiz_attempts_report::ENROLLED_WITHOUT);
         }
     }
 }
diff --git a/mod/quiz/report/responses/tests/behat/basic.feature b/mod/quiz/report/responses/tests/behat/basic.feature
new file mode 100644 (file)
index 0000000..b8344ea
--- /dev/null
@@ -0,0 +1,91 @@
+@mod @mod_quiz @quiz @quiz_reponses
+Feature: Basic use of the Responses report
+  In order to see how my students are progressing
+  As a teacher
+  I need to see all their quiz responses
+
+  Background: Using the Responses report
+    Given the following "users" exist:
+      | username | firstname | lastname |
+      | teacher  | The       | Teacher  |
+      | student1 | Student   | One      |
+      | student2 | Student   | Two      |
+    And the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | teacher  | C1     | editingteacher |
+      | student1 | C1     | student        |
+      | student2 | 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 | preferredbehaviour |
+      | quiz       | Quiz 1 | Quiz 1 description | C1     | quiz1    | interactive        |
+    And the following "questions" exist:
+      | questioncategory | qtype     | name | template |
+      | Test questions   | numerical | NQ   | pi3tries |
+    And quiz "Quiz 1" contains the following questions:
+      | question | page | maxmark |
+      | NQ       | 1    | 3.0     |
+
+  @javascript
+  Scenario: Report works when there are no attempts
+    When I log in as "teacher"
+    And I am on "Course 1" course homepage
+    And I follow "Quiz 1"
+    And I navigate to "Results > Responses" in current page administration
+    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 press "Show report"
+    And "Student One" row "State" column of "responses" table should contain "-"
+
+  @javascript
+  Scenario: Report works when there are attempts
+    # Add an attempt
+    Given I log in as "student1"
+    And I am on "Course 1" course homepage
+    And I follow "Quiz 1"
+    And I press "Attempt quiz now"
+    And I set the field "Answer" to "1.0"
+    And I press "Check"
+    And I press "Try again"
+    And I set the field "Answer" to "3.0"
+    And I press "Check"
+    And I press "Try again"
+    And I set the field "Answer" to "3.14"
+    And I press "Check"
+    And I press "Finish attempt ..."
+    And I press "Submit all and finish"
+    And I click on "Submit all and finish" "button" in the "Confirmation" "dialogue"
+    And I log out
+
+    When I log in as "teacher"
+    And I am on "Course 1" course homepage
+    And I follow "Quiz 1"
+    And I navigate to "Results > Responses" in current page administration
+    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 "Which tries" to "All tries"
+    And I press "Show report"
+    And "Student OneReview attempt" row "Response 1Sort by Response 1 Ascending" column of "responses" table should contain "1.0"
+    And "Student OneReview attempt" row "State" column of "responses" table should contain ""
+    And "Finished" row "Grade/100.00Sort by Grade/100.00 Ascending" column of "responses" table should contain "33.33"
+    And "Finished" row "Response 1Sort by Response 1 Ascending" column of "responses" table should contain "3.14"
+    And "Student Two" row "State" column of "responses" table should contain "-"
+    And "Student Two" row "Response 1Sort by Response 1 Ascending" column of "responses" table should contain "-"
+
+  @javascript
+  Scenario: Report does not allow strange combinations of options
+    When I log in as "teacher"
+    And I am on "Course 1" course homepage
+    And I follow "Quiz 1"
+    And I navigate to "Results > Responses" in current page administration
+    And the "Which tries" "select" should be enabled
+    And I set the field "Attempts from" to "enrolled users who have not attempted the quiz"
+    Then the "Which tries" "select" should be disabled
index e486e46..0ef5816 100644 (file)
@@ -525,15 +525,11 @@ ORDER BY
     qas.sequencenumber
     ", $qubaids->usage_id_in_params());
 
-        if (!$records->valid()) {
-            throw new coding_exception('Failed to load questions_usages_by_activity for qubaid_condition :' . $qubaids);
-        }
-
         $qubas = array();
-        do {
+        while ($records->valid()) {
             $record = $records->current();
             $qubas[$record->qubaid] = question_usage_by_activity::load_from_records($records, $record->qubaid);
-        } while ($records->valid());
+        }
 
         $records->close();
 
index 7157dfb..650674b 100644 (file)
@@ -35,7 +35,7 @@ defined('MOODLE_INTERNAL') || die();
  */
 class qtype_numerical_test_helper extends question_test_helper {
     public function get_test_questions() {
-        return array('pi', 'unit', 'currency');
+        return array('pi', 'unit', 'currency', 'pi3tries');
     }
 
     /**
@@ -71,6 +71,15 @@ class qtype_numerical_test_helper extends question_test_helper {
         return $num;
     }
 
+    /**
+     * Get the form data that corresponds to saving a numerical question.
+     *
+     * This question asks for Pi to two decimal places. It has feedback
+     * for various wrong responses. There is hint data there, but
+     * it is all blank, so no hints are created if this question is saved.
+     *
+     * @return stdClass simulated question form data.
+     */
     public function get_numerical_question_form_data_pi() {
         $form = new stdClass();
         $form->name = 'Pi to two d.p.';
@@ -156,6 +165,22 @@ class qtype_numerical_test_helper extends question_test_helper {
         return $form;
     }
 
+    /**
+     * Get the form data that corresponds to saving a numerical question.
+     *
+     * Like {@link get_numerical_question_form_data_pi()}, but
+     * this time with two hints, making this suitable for use
+     * with the Interactive with multiple tries behaviour.
+     *
+     * @return stdClass simulated question form data.
+     */
+    public function get_numerical_question_form_data_pi3tries() {
+        $form = $this->get_numerical_question_form_data_pi();
+        $form->hint[0]['text'] = 'First hint';
+        $form->hint[1]['text'] = 'Second hint';
+        return $form;
+    }
+
     public function get_numerical_question_data_pi() {
         $q = new stdClass();
         $q->name = 'Pi to two d.p.';