GHI384 Fix for required checkbox issue and 'other' option (#460)

* GHI384 Ensure an option is checked

* For required checkbox questions, having an 'other' should not allow no selection to be made.
* If other checkbox is selected, ensure text has been entered.
* Create Behat test to verify.

* GHI384 Adding test for missing text.

* GHI384 New CI commands and CI fixes.

Author: mchurchward

.github/workflows/ci.yml

@@ -4,6 +4,28 @@ on: [push, pull_request]
 jobs:
   test:
     runs-on: 'ubuntu-latest'
+
+    services:
+      postgres:
+        image: postgres:13
+        env:
+          POSTGRES_USER: 'postgres'
+          POSTGRES_HOST_AUTH_METHOD: 'trust'
+        ports:
+          - 5432:5432
+        options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 3
+      mariadb:
+        image: mariadb:10
+        env:
+          MYSQL_USER: 'root'
+          MYSQL_ALLOW_EMPTY_PASSWORD: "true"
+          MYSQL_CHARACTER_SET_SERVER: "utf8mb4"
+          MYSQL_COLLATION_SERVER: "utf8mb4_unicode_ci"
+
+        ports:
+          - 3306:3306
+        options: --health-cmd="mysqladmin ping" --health-interval 10s --health-timeout 5s --health-retries 3
+
     strategy:
       fail-fast: false
       matrix:
@@ -24,34 +46,9 @@ jobs:
             moodle-branch: 'MOODLE_39_STABLE'
             database: 'pgsql'
 
-    services:
-      postgres:
-        image: postgres:10
-        env:
-          POSTGRES_USER: 'postgres'
-          POSTGRES_HOST_AUTH_METHOD: 'trust'
-        options: >-
-          --health-cmd pg_isready
-          --health-interval 10s
-          --health-timeout 5s
-          --health-retries 3
-        ports:
-          - 5432:5432
-
-      mariadb:
-        image: mariadb:10
-        env:
-          MYSQL_USER: 'root'
-          MYSQL_ALLOW_EMPTY_PASSWORD: "true"
-          MYSQL_CHARACTER_SET_SERVER: "utf8mb4"
-          MYSQL_COLLATION_SERVER: "utf8mb4_unicode_ci"
-        ports:
-          - 3306:3306
-        options: --health-cmd="mysqladmin ping" --health-interval 10s --health-timeout 5s --health-retries 3
-
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           path: plugin
 
@@ -61,6 +58,7 @@ jobs:
           php-version: ${{ matrix.php }}
           extensions: ${{ matrix.extensions }}
           ini-values: max_input_vars=5000
+          # none to use phpdbg fallback. Specify pcov (Moodle 3.10 and up) or xdebug to use them instead.
           coverage: none
 
       - name: Deploy moodle-plugin-ci

classes/question/check.php

@@ -197,6 +197,27 @@ protected function response_survey_display($response) {
         return $resptags;
     }
 
+    /**
+     * Check question's form data for complete response.
+     *
+     * @param object $responsedata The data entered into the response.
+     * @return boolean
+     */
+    public function response_complete($responsedata) {
+        if (isset($responsedata->{'q'.$this->id}) && $this->required() &&
+            is_array($responsedata->{'q'.$this->id})) {
+            foreach ($responsedata->{'q' . $this->id} as $key => $choice) {
+                // If only an 'other' choice is selected and empty, question is not completed.
+                if ((strpos($key, 'o') === 0) && empty($choice)) {
+                    return false;
+                } else {
+                    return true;
+                }
+            }
+        }
+        return parent::response_complete($responsedata);
+    }
+
     /**
      * Check question's form data for valid response. Override this is type has specific format requirements.
      *
@@ -218,16 +239,15 @@ public function response_valid($responsedata) {
                 }
             }
         } else if (isset($responsedata->{'q'.$this->id})) {
-            foreach ($responsedata->{'q'.$this->id} as $answer) {
-                if (strpos($answer, 'other_') !== false) {
+            foreach ($responsedata->{'q'.$this->id} as $key => $answer) {
+                if (strpos($key, 'o') === 0) {
                     // ..."other" choice is checked but text box is empty.
-                    $othercontent = "q".$this->id.substr($answer, 5);
-                    if (trim($responsedata->$othercontent) == false) {
+                    $okey = substr($key, 1);
+                    if (isset($responsedata->{'q'.$this->id}[$okey]) && empty(trim($answer))) {
                         $valid = false;
                         break;
                     }
-                    $nbrespchoices++;
-                } else if (is_numeric($answer)) {
+                } else if (is_numeric($key)) {
                     $nbrespchoices++;
                 }
             }

tests/behat/checkbox_other_responses.feature

@@ -0,0 +1,48 @@
+@mod @mod_questionnaire
+Feature: Checkbox questions can have other options that can be typed in.
+
+  Background: Add a checkbox question to a questionnaire with an 'other' option.
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@example.com |
+      | student1 | Student | 1 | student1@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 |
+    And the following "activities" exist:
+      | activity | name | description | course | idnumber |
+      | questionnaire | Test questionnaire | Test questionnaire description | C1 | questionnaire0 |
+    And I log in as "teacher1"
+    And I am on "Course 1" course homepage
+    And I follow "Test questionnaire"
+    And I navigate to "Questions" in current page administration
+    And I add a "Check Boxes" question and I fill the form with:
+      | Question Name | Q1 |
+      | Yes | y |
+      | Question Text | Select one or two choices only |
+      | Possible answers | One,Two,Three,Four,!other |
+    And I add a "Check Boxes" question and I fill the form with:
+      | Question Name | Q2 |
+      | No | n |
+      | Question Text | Select one or two choices only |
+      | Possible answers | Red,Blue,Yellow,Green,!other=Other colour |
+    And I log out
+
+  @javascript
+  Scenario: Student must enter a valid value when "other" is selected.
+    And I log in as "student1"
+    And I am on "Course 1" course homepage
+    And I follow "Test questionnaire"
+    And I navigate to "Answer the questions..." in current page administration
+    Then I should see "Select one or two choices only"
+    And I press "Submit questionnaire"
+    Then I should see "Please answer required question #1"
+    And I set the field "Other" to "checked"
+    And I set the field "Other colour" to "checked"
+    And I press "Submit questionnaire"
+    Then I should see "There is something wrong with your answer to questions:"
+    And I should see "#1. #2."