MDL-3034 mod_choice: minor tidy up

mod/choice/lib.php

@@ -263,11 +263,11 @@ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm
     $current = $DB->get_records('choice_answers', array('choiceid' => $choice->id, 'userid' => $userid));
     $context = context_module::instance($cm->id);
 
+    $choicesexceeded = false;
     $countanswers = array();
     foreach ($formanswers as $val) {
         $countanswers[$val] = 0;
     }
-
     if($choice->limitanswers) {
         // Find out whether groups are being used and enabled
         if (groups_get_activity_groupmode($cm) > 0) {
@@ -284,26 +284,19 @@ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm
             global $CFG;
 
             $params['groupid'] = $currentgroup;
-            $answers = $DB->get_records_sql("
+            $sql = "SELECT ca.*
-SELECT
+                      FROM {choice_answers} ca
-    ca.*
+                INNER JOIN {groups_members} gm ON ca.userid=gm.userid
-FROM
+                     WHERE optionid $insql
-    {choice_answers} ca
+                       AND gm.groupid= :groupid";
-    INNER JOIN {groups_members} gm ON ca.userid=gm.userid
-WHERE
-    optionid $insql
-    AND gm.groupid= :groupid", $params);
         } else {
             // Groups are not used, retrieve all answers for this option ID
-            $answers = $DB->get_records_sql("
+            $sql = "SELECT ca.*
-SELECT
+                      FROM {choice_answers} ca
-    ca.*
+                     WHERE optionid $insql";
-FROM
-    {choice_answers} ca
-WHERE
-    optionid $insql", $params);
         }
 
+        $answers = $DB->get_records_sql($sql, $params);
         if ($answers) {
             foreach ($answers as $a) { //only return enrolled users.
                 if (is_enrolled($context, $a->userid, 'mod/choice:choose')) {
@@ -311,8 +304,6 @@ WHERE
                 }
             }
         }
-
-        $choicesexceeded = false;
         foreach ($countanswers as $opt => $count) {
             if ($count > $choice->maxanswers[$opt]) {
                 $choicesexceeded = true;
@@ -321,6 +312,7 @@ WHERE
         }
     }
 
+    // Check the user hasn't exceeded the maximum selections for the choice(s) they have selected.
     if (!($choice->limitanswers && $choicesexceeded)) {
         $answersnapshots = array();
         if ($current) {
@@ -329,10 +321,8 @@ WHERE
             foreach ($current as $c) {
                 if (in_array($c->optionid, $formanswers)) {
                     $existingchoices[] = $c->optionid;
-                    $newanswer = $c;
+                    $DB->set_field('choice_answers', 'timemodified', time(), array('id' => $c->id));
-                    $newanswer->timemodified = time();
+                    $answersnapshots[] = $c;
-                    $DB->update_record("choice_answers", $newanswer);
-                    $answersnapshots[] = $newanswer;
                 } else {
                     $DB->delete_records('choice_answers', array('id' => $c->id));
                 }
@@ -419,7 +409,6 @@ WHERE
  * @return void Output is echo'd
  */
 function choice_show_reportlink($user, $cm) {
-    $responsecount =0;
     $userschosen = array();
     foreach($user as $optionid => $userlist) {
         if ($optionid) {

mod/choice/tests/behat/behat_mod_choice.php

@@ -45,6 +45,7 @@ class behat_mod_choice extends behat_base {
      * @Given /^I choose "(?P<option_string>(?:[^"]|\\")*)" from "(?P<choice_activity_string>(?:[^"]|\\")*)" choice activity$/
      * @param string $option
      * @param string $choiceactivity
+     * @return array
      */
     public function I_choose_option_from_activity($option, $choiceactivity) {
 
@@ -56,4 +57,24 @@ class behat_mod_choice extends behat_base {
         );
     }
 
+    /**
+     * Chooses the specified option from the choice activity named as specified and save the choice.
+     * You should be located in the activity's course page.
+     *
+     * @Given /^I choose options (?P<option_string>"(?:[^"]|\\")*"(?:,"(?:[^"]|\\")*")*) from "(?P<choice_activity_string>(?:[^"]|\\")*)" choice activity$/
+     * @param string $option
+     * @param string $choiceactivity
+     * @return array
+     */
+    public function I_choose_options_from_activity($option, $choiceactivity) {
+        // Escaping again the strings as backslashes have been removed by the automatic transformation.
+        $return = array(new Given('I follow "' . $this->escape($choiceactivity) . '"'));
+        $options = explode('","', trim($option, '"'));
+        foreach ($options as $option) {
+            $return[] = new Given('I set the field "' . $this->escape($option) . '" to "1"');
+        }
+        $return[] = new Given('I press "' . get_string('savemychoice', 'choice') . '"');
+        return $return;
+    }
+
 }

mod/choice/tests/behat/multiple_options.feature

@@ -0,0 +1,80 @@
+@mod @mod_choice
+Feature: Multiple option choice response
+  In order to ask questions as a choice of multiple responses
+  As a teacher
+  I need to add choice activities to courses with multiple options enabled
+
+  @javascript
+  Scenario: Complete a choice with multiple options enabled
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@asd.com |
+      | student1 | Student | 1 | student1@asd.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 I log in as "teacher1"
+    And I follow "Course 1"
+    And I turn editing mode on
+    And I add a "Choice" to section "1" and I fill the form with:
+      | Choice name | Choice name |
+      | Description | Choice Description |
+      | Allow more than one choice to be selected | Yes |
+      | option[0] | Option 1 |
+      | option[1] | Option 2 |
+    And I log out
+    When I log in as "student1"
+    And I follow "Course 1"
+    And I choose options "Option 1","Option 2" from "Choice name" choice activity
+    Then I should see "Your selection: Option 1; Option 2"
+    And I should see "Your choice has been saved"
+
+  @javascript
+  Scenario: Complete a choice with multiple options enabled and limited responses set
+    Given the following "users" exist:
+      | username | firstname | lastname | email |
+      | teacher1 | Teacher | 1 | teacher1@asd.com |
+      | student1 | Student | 1 | student1@asd.com |
+      | student2 | Student | 2 | student2@asd.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 |
+    And I log in as "teacher1"
+    And I follow "Course 1"
+    And I turn editing mode on
+    And I add a "Choice" to section "1" and I fill the form with:
+      | Choice name | Choice name |
+      | Description | Choice Description |
+      | Allow more than one choice to be selected | Yes |
+      | Limit the number of responses allowed | 1 |
+      | option[0] | Option 1 |
+      | limit[0] | 1 |
+      | option[1] | Option 2 |
+      | limit[1] | 1 |
+      | option[2] | Option 3 |
+      | limit[2] | 1 |
+    And I log out
+    When I log in as "student1"
+    And I follow "Course 1"
+    And I choose options "Option 1","Option 2" from "Choice name" choice activity
+    Then I should see "Your selection: Option 1; Option 2"
+    And I should see "Your choice has been saved"
+    And I log out
+    And I log in as "student2"
+    And I follow "Course 1"
+    And I follow "Choice name"
+    And I should see "Option 1 (Full)"
+    And I should see "Option 2 (Full)"
+    And I should see "Option 3"
+    And the "#choice_1" "css_element" should be disabled
+    And the "#choice_2" "css_element" should be disabled
+    And the "#choice_3" "css_element" should be enabled

mod/choice/tests/events_test.php

@@ -79,7 +79,7 @@ class mod_choice_events_testcase extends advanced_testcase {
         $this->assertEquals($user->id, $events[0]->userid);
         $this->assertEquals(context_module::instance($this->choice->cmid), $events[0]->get_context());
         $this->assertEquals($this->choice->id, $events[0]->other['choiceid']);
-        $this->assertEquals(3, $events[0]->other['optionid']);
+        $this->assertEquals(array(3), $events[0]->other['optionid']);
         $expected = array($this->course->id, "choice", "choose", 'view.php?id=' . $this->cm->id, $this->choice->id, $this->cm->id);
         $this->assertEventLegacyLogData($expected, $events[0]);
         $this->assertEventContextNotUsed($events[0]);

mod/choice/view.php

@@ -56,17 +56,10 @@ $completion->set_module_viewed($cm);
 /// Submit any new data if there is any
 if (data_submitted() && is_enrolled($context, NULL, 'mod/choice:choose') && confirm_sesskey()) {
     $timenow = time();
-    if (has_capability('mod/choice:deleteresponses', $context)) {
+    if (has_capability('mod/choice:deleteresponses', $context) && $action == 'delete') {
-        if ($action == 'delete') { //some responses need to be deleted
+        //some responses need to be deleted
-            choice_delete_responses($attemptids, $choice, $cm, $course); //delete responses.
+        choice_delete_responses($attemptids, $choice, $cm, $course); //delete responses.
-            redirect("view.php?id=$cm->id");
+        redirect("view.php?id=$cm->id");
-        }
-        echo $OUTPUT->header();
-        echo $OUTPUT->heading(format_string($choice->name), 2, null);
-        echo $OUTPUT->notification(get_string('choicesaved', 'choice'),'notifysuccess');
-    } else {
-        echo $OUTPUT->header();
-        echo $OUTPUT->heading(format_string($choice->name), 2, null);
     }
 
     if ($choice->allowmultiple) {
@@ -81,11 +74,11 @@ if (data_submitted() && is_enrolled($context, NULL, 'mod/choice:choose') && conf
         choice_user_submit_response($answer, $choice, $USER->id, $course, $cm);
     }
     echo $OUTPUT->header();
-    echo $OUTPUT->heading($choice->name, 2, null);
+    echo $OUTPUT->heading(format_string($choice->name), 2, null);
     echo $OUTPUT->notification(get_string('choicesaved', 'choice'),'notifysuccess');
 } else {
     echo $OUTPUT->header();
-    echo $OUTPUT->heading($choice->name, 2, null);
+    echo $OUTPUT->heading(format_string($choice->name), 2, null);
 }