From cbb1677cb93353255ac23bc41ee2bf7651ac917e Mon Sep 17 00:00:00 2001 From: Sam Hemelryk Date: Thu, 25 Sep 2014 11:34:47 +1200 Subject: [PATCH] MDL-3034 mod_choice: minor tidy up --- mod/choice/lib.php | 37 +++------ mod/choice/tests/behat/behat_mod_choice.php | 21 +++++ .../tests/behat/multiple_options.feature | 80 +++++++++++++++++++ mod/choice/tests/events_test.php | 2 +- mod/choice/view.php | 19 ++--- 5 files changed, 121 insertions(+), 38 deletions(-) create mode 100644 mod/choice/tests/behat/multiple_options.feature diff --git a/mod/choice/lib.php b/mod/choice/lib.php index 2208d3f858b..a2d3a35b243 100644 --- a/mod/choice/lib.php +++ b/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(" -SELECT - ca.* -FROM - {choice_answers} ca - INNER JOIN {groups_members} gm ON ca.userid=gm.userid -WHERE - optionid $insql - AND gm.groupid= :groupid", $params); + $sql = "SELECT ca.* + FROM {choice_answers} ca + INNER JOIN {groups_members} gm ON ca.userid=gm.userid + WHERE optionid $insql + AND gm.groupid= :groupid"; } else { // Groups are not used, retrieve all answers for this option ID - $answers = $DB->get_records_sql(" -SELECT - ca.* -FROM - {choice_answers} ca -WHERE - optionid $insql", $params); + $sql = "SELECT ca.* + FROM {choice_answers} ca + WHERE optionid $insql"; } + $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; - $newanswer->timemodified = time(); - $DB->update_record("choice_answers", $newanswer); - $answersnapshots[] = $newanswer; + $DB->set_field('choice_answers', 'timemodified', time(), array('id' => $c->id)); + $answersnapshots[] = $c; } 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) { diff --git a/mod/choice/tests/behat/behat_mod_choice.php b/mod/choice/tests/behat/behat_mod_choice.php index 71598e69346..ba012aaae68 100644 --- a/mod/choice/tests/behat/behat_mod_choice.php +++ b/mod/choice/tests/behat/behat_mod_choice.php @@ -45,6 +45,7 @@ class behat_mod_choice extends behat_base { * @Given /^I choose "(?P(?:[^"]|\\")*)" from "(?P(?:[^"]|\\")*)" 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"(?:[^"]|\\")*"(?:,"(?:[^"]|\\")*")*) from "(?P(?:[^"]|\\")*)" 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; + } + } diff --git a/mod/choice/tests/behat/multiple_options.feature b/mod/choice/tests/behat/multiple_options.feature new file mode 100644 index 00000000000..cd37384a630 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/mod/choice/tests/events_test.php b/mod/choice/tests/events_test.php index 1eb9d0a226e..90ce3567cd0 100644 --- a/mod/choice/tests/events_test.php +++ b/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]); diff --git a/mod/choice/view.php b/mod/choice/view.php index 368310979ce..176872d8cad 100644 --- a/mod/choice/view.php +++ b/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 ($action == 'delete') { //some responses need to be deleted - choice_delete_responses($attemptids, $choice, $cm, $course); //delete responses. - 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 (has_capability('mod/choice:deleteresponses', $context) && $action == 'delete') { + //some responses need to be deleted + choice_delete_responses($attemptids, $choice, $cm, $course); //delete responses. + redirect("view.php?id=$cm->id"); } 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); }