From 8676e7c3e0ac2a2b5f7eec2a83060fb73760cd47 Mon Sep 17 00:00:00 2001 From: Marina Glancy Date: Fri, 27 May 2016 17:16:45 +0800 Subject: [PATCH] MDL-18592 mod_choice: Allow teachers to make choice for students Capability to delete reponses becomes capability to delete or modify responses. This also resolves MDL-51659 (incorrect events answer_submitted and answer_updated) by deprecating them and replacing with the answer_created --- mod/choice/classes/event/answer_created.php | 154 ++++++++++++++++++ mod/choice/classes/event/answer_deleted.php | 30 +++- mod/choice/classes/event/answer_submitted.php | 14 +- mod/choice/classes/event/answer_updated.php | 14 ++ mod/choice/lang/en/choice.php | 6 +- mod/choice/lib.php | 127 +++++++++------ mod/choice/renderer.php | 34 +++- mod/choice/report.php | 34 ++-- mod/choice/styles.css | 3 +- mod/choice/tests/behat/modify_choice.feature | 127 +++++++++++++++ mod/choice/tests/events_test.php | 120 +++++++++----- mod/choice/tests/generator/lib.php | 2 + mod/choice/upgrade.txt | 9 + mod/choice/view.php | 30 ++-- 14 files changed, 566 insertions(+), 138 deletions(-) create mode 100644 mod/choice/classes/event/answer_created.php create mode 100644 mod/choice/tests/behat/modify_choice.feature diff --git a/mod/choice/classes/event/answer_created.php b/mod/choice/classes/event/answer_created.php new file mode 100644 index 00000000000..c0f0e3e08ba --- /dev/null +++ b/mod/choice/classes/event/answer_created.php @@ -0,0 +1,154 @@ +. + +/** + * The mod_choice answer created event. + * + * @package mod_choice + * @copyright 2016 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace mod_choice\event; + +defined('MOODLE_INTERNAL') || die(); + +/** + * The mod_choice answer created event class. + * + * @property-read array $other { + * Extra information about event. + * + * - int choiceid: id of choice. + * - int optionid: id of the option. + * } + * + * @package mod_choice + * @since Moodle 3.2 + * @copyright 2016 Marina Glancy + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class answer_created extends \core\event\base { + + /** + * Creates an instance of the event from the records + * + * @param stdClass $choiceanswer record from 'choice_answers' table + * @param stdClass $choice record from 'choice' table + * @param stdClass $cm record from 'course_modules' table + * @param stdClass $course + * @return self + */ + public static function create_from_object($choiceanswer, $choice, $cm, $course) { + global $USER; + $eventdata = array(); + $eventdata['objectid'] = $choiceanswer->id; + $eventdata['context'] = \context_module::instance($cm->id); + $eventdata['userid'] = $USER->id; + $eventdata['courseid'] = $course->id; + $eventdata['relateduserid'] = $choiceanswer->userid; + $eventdata['other'] = array(); + $eventdata['other']['choiceid'] = $choice->id; + $eventdata['other']['optionid'] = $choiceanswer->optionid; + $event = self::create($eventdata); + $event->add_record_snapshot('course', $course); + $event->add_record_snapshot('course_modules', $cm); + $event->add_record_snapshot('choice', $choice); + $event->add_record_snapshot('choice_answers', $choiceanswer); + return $event; + } + + /** + * Returns description of what happened. + * + * @return string + */ + public function get_description() { + return "The user with id '$this->userid' has added the option with id '" . $this->other['optionid'] . "' for the + user with id '$this->relateduserid' from the choice activity with course module id '$this->contextinstanceid'."; + } + + /** + * Return localised event name. + * + * @return string + */ + public static function get_name() { + return get_string('eventanswercreated', 'mod_choice'); + } + + /** + * Get URL related to the action + * + * @return \moodle_url + */ + public function get_url() { + return new \moodle_url('/mod/choice/view.php', array('id' => $this->contextinstanceid)); + } + + /** + * Init method. + * + * @return void + */ + protected function init() { + $this->data['objecttable'] = 'choice_answers'; + $this->data['crud'] = 'c'; + $this->data['edulevel'] = self::LEVEL_PARTICIPATING; + } + + /** + * Custom validation. + * + * @throws \coding_exception + * @return void + */ + protected function validate_data() { + parent::validate_data(); + + if (!isset($this->other['choiceid'])) { + throw new \coding_exception('The \'choiceid\' value must be set in other.'); + } + + if (!isset($this->other['optionid'])) { + throw new \coding_exception('The \'optionid\' value must be set in other.'); + } + } + + /** + * This is used when restoring course logs where it is required that we + * map the objectid to it's new value in the new course. + * + * @return string the name of the restore mapping the objectid links to + */ + public static function get_objectid_mapping() { + return array('db' => 'choice_answers', 'restore' => 'answer'); + } + + /** + * This is used when restoring course logs where it is required that we + * map the information in 'other' to it's new value in the new course. + * + * @return array an array of other values and their corresponding mapping + */ + public static function get_other_mapping() { + $othermapped = array(); + $othermapped['choiceid'] = array('db' => 'choice', 'restore' => 'choice'); + $othermapped['optionid'] = array('db' => 'choice_options', 'restore' => 'choice_option'); + + return $othermapped; + } +} diff --git a/mod/choice/classes/event/answer_deleted.php b/mod/choice/classes/event/answer_deleted.php index 7b8cb18bd06..29f38d9695f 100644 --- a/mod/choice/classes/event/answer_deleted.php +++ b/mod/choice/classes/event/answer_deleted.php @@ -27,7 +27,7 @@ namespace mod_choice\event; defined('MOODLE_INTERNAL') || die(); /** - * The mod_choice answer updated event class. + * The mod_choice answer deleted event class. * * @property-read array $other { * Extra information about event. @@ -43,6 +43,34 @@ defined('MOODLE_INTERNAL') || die(); */ class answer_deleted extends \core\event\base { + /** + * Creates an instance of the event from the records + * + * @param stdClass $choiceanswer record from 'choice_answers' table + * @param stdClass $choice record from 'choice' table + * @param stdClass $cm record from 'course_modules' table + * @param stdClass $course + * @return self + */ + public static function create_from_object($choiceanswer, $choice, $cm, $course) { + global $USER; + $eventdata = array(); + $eventdata['objectid'] = $choiceanswer->id; + $eventdata['context'] = \context_module::instance($cm->id); + $eventdata['userid'] = $USER->id; + $eventdata['courseid'] = $course->id; + $eventdata['relateduserid'] = $choiceanswer->userid; + $eventdata['other'] = array(); + $eventdata['other']['choiceid'] = $choice->id; + $eventdata['other']['optionid'] = $choiceanswer->optionid; + $event = self::create($eventdata); + $event->add_record_snapshot('course', $course); + $event->add_record_snapshot('course_modules', $cm); + $event->add_record_snapshot('choice', $choice); + $event->add_record_snapshot('choice_answers', $choiceanswer); + return $event; + } + /** * Returns description of what happened. * diff --git a/mod/choice/classes/event/answer_submitted.php b/mod/choice/classes/event/answer_submitted.php index baa6817b44f..76fb819e645 100644 --- a/mod/choice/classes/event/answer_submitted.php +++ b/mod/choice/classes/event/answer_submitted.php @@ -29,6 +29,13 @@ defined('MOODLE_INTERNAL') || die(); /** * The mod_choice answer submitted event class. * + * This event is deprecated in Moodle 3.2, it can no longer be triggered, do not + * write event observers for it. This event can only be initiated during + * restore from previous Moodle versions and appear in the logs. + * + * Event observers should listen to mod_choice\event\answer_created instead that + * will be triggered once for each option selected + * * @property-read array $other { * Extra information about event. * @@ -36,6 +43,7 @@ defined('MOODLE_INTERNAL') || die(); * - int optionid: (optional) id of option. * } * + * @deprecated since 3.2 * @package mod_choice * @since Moodle 2.6 * @copyright 2013 Adrian Greeve @@ -75,7 +83,7 @@ class answer_submitted extends \core\event\base { * @return string */ public static function get_name() { - return get_string('eventanswercreated', 'mod_choice'); + return get_string('eventanswersubmitted', 'mod_choice'); } /** @@ -111,6 +119,10 @@ class answer_submitted extends \core\event\base { protected function validate_data() { parent::validate_data(); + debugging('Event \\mod_choice\event\\answer_submitted should not be used ' + . 'any more for triggering new events and can only be initiated during restore. ' + . 'For new events please use \\mod_choice\\event\\answer_created', DEBUG_DEVELOPER); + if (!isset($this->other['choiceid'])) { throw new \coding_exception('The \'choiceid\' value must be set in other.'); } diff --git a/mod/choice/classes/event/answer_updated.php b/mod/choice/classes/event/answer_updated.php index e6ab08a87d4..60e3720e333 100644 --- a/mod/choice/classes/event/answer_updated.php +++ b/mod/choice/classes/event/answer_updated.php @@ -29,6 +29,14 @@ defined('MOODLE_INTERNAL') || die(); /** * The mod_choice answer updated event class. * + * This event is deprecated in Moodle 3.2, it can no longer be triggered, do not + * write event observers for it. This event can only be initiated during + * restore from previous Moodle versions and appear in the logs. + * + * Event observers should listen to mod_choice\event\answer_created and + * mod_choice\event\answer_deleted instead, these events will be triggered for + * each option that was user has selected or unselected + * * @property-read array $other { * Extra information about event. * @@ -36,6 +44,7 @@ defined('MOODLE_INTERNAL') || die(); * - int optionid: (optional) id of option. * } * + * @deprecated since 3.2 * @package mod_choice * @since Moodle 2.6 * @copyright 2013 Adrian Greeve @@ -111,6 +120,11 @@ class answer_updated extends \core\event\base { protected function validate_data() { parent::validate_data(); + debugging('Event \\mod_choice\event\\answer_updated should not be used ' + . 'any more for triggering new events and can only be initiated during restore. ' + . 'For new events please use \\mod_choice\\event\\answer_created ' + . 'and \\mod_choice\\event\\answer_deleted', DEBUG_DEVELOPER); + if (!isset($this->other['choiceid'])) { throw new \coding_exception('The \'choiceid\' value must be set in other.'); } diff --git a/mod/choice/lang/en/choice.php b/mod/choice/lang/en/choice.php index dcd01e5b84e..e877076d268 100644 --- a/mod/choice/lang/en/choice.php +++ b/mod/choice/lang/en/choice.php @@ -35,8 +35,9 @@ $string['completionsubmit'] = 'Show as complete when user makes a choice'; $string['displayhorizontal'] = 'Display horizontally'; $string['displaymode'] = 'Display mode for the options'; $string['displayvertical'] = 'Display vertically'; -$string['eventanswercreated'] = 'Choice made'; +$string['eventanswercreated'] = 'Choice answer added'; $string['eventanswerdeleted'] = 'Choice answer deleted'; +$string['eventanswersubmitted'] = 'Choice made'; $string['eventanswerupdated'] = 'Choice updated'; $string['eventreportdownloaded'] = 'Choice report downloaded'; $string['eventreportviewed'] = 'Choice report viewed'; @@ -48,7 +49,7 @@ $string['choice'] = 'Choice'; $string['choiceactivityname'] = 'Choice: {$a}'; $string['choice:addinstance'] = 'Add a new choice'; $string['choiceclose'] = 'Allow responses until'; -$string['choice:deleteresponses'] = 'Delete responses'; +$string['choice:deleteresponses'] = 'Modify and delete responses'; $string['choice:downloadresponses'] = 'Download responses'; $string['choicefull'] = 'This choice is full and there are no available places.'; $string['choice:choose'] = 'Record a choice'; @@ -67,6 +68,7 @@ $string['choicesaved'] = 'Your choice has been saved'; $string['choicetext'] = 'Choice text'; $string['choice:view'] = 'View choice activity'; $string['chooseaction'] = 'Choose an action ...'; +$string['chooseoption'] = 'Choose: {$a}'; $string['description'] = 'Description'; $string['includeinactive'] = 'Include responses from inactive/suspended users'; $string['limit'] = 'Limit'; diff --git a/mod/choice/lib.php b/mod/choice/lib.php index d7a00b040a2..5f9ded4d090 100644 --- a/mod/choice/lib.php +++ b/mod/choice/lib.php @@ -244,6 +244,61 @@ function choice_prepare_options($choice, $user, $coursemodule, $allresponses) { return $cdisplay; } +/** + * Modifies responses of other users adding the option $newoptionid to them + * + * @param array $userids list of users to add option to (must be users without any answers yet) + * @param array $answerids list of existing attempt ids of users (will be either appended or + * substituted with the newoptionid, depending on $choice->allowmultiple) + * @param int $newoptionid + * @param stdClass $choice choice object, result of {@link choice_get_choice()} + * @param stdClass $cm + * @param stdClass $course + */ +function choice_modify_responses($userids, $answerids, $newoptionid, $choice, $cm, $course) { + // Get all existing responses and the list of non-respondents. + $groupmode = groups_get_activity_groupmode($cm); + $onlyactive = $choice->includeinactive ? false : true; + $allresponses = choice_get_response_data($choice, $cm, $groupmode, $onlyactive); + + // Check that the option value is valid. + if (!$newoptionid || !isset($choice->option[$newoptionid])) { + return; + } + + // First add responses for users who did not make any choice yet. + foreach ($userids as $userid) { + if (isset($allresponses[0][$userid])) { + choice_user_submit_response($newoptionid, $choice, $userid, $course, $cm); + } + } + + // Create the list of all options already selected by each user. + $optionsbyuser = []; // Mapping userid=>array of chosen choice options. + $usersbyanswer = []; // Mapping answerid=>userid (which answer belongs to each user). + foreach ($allresponses as $optionid => $responses) { + if ($optionid > 0) { + foreach ($responses as $userid => $userresponse) { + $optionsbyuser += [$userid => []]; + $optionsbyuser[$userid][] = $optionid; + $usersbyanswer[$userresponse->answerid] = $userid; + } + } + } + + // Go through the list of submitted attemptids and find which users answers need to be updated. + foreach ($answerids as $answerid) { + if (isset($usersbyanswer[$answerid])) { + $userid = $usersbyanswer[$answerid]; + if (!in_array($newoptionid, $optionsbyuser[$userid])) { + $options = $choice->allowmultiple ? + array_merge($optionsbyuser[$userid], [$newoptionid]) : $newoptionid; + choice_user_submit_response($options, $choice, $userid, $course, $cm); + } + } + } +} + /** * Process user submitted answers for a choice, * and either updating them or saving new answers. @@ -256,7 +311,7 @@ function choice_prepare_options($choice, $user, $coursemodule, $allresponses) { * @return void */ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm) { - global $DB, $CFG; + global $DB, $CFG, $USER; require_once($CFG->libdir.'/completionlib.php'); $continueurl = new moodle_url('/mod/choice/view.php', array('id' => $cm->id)); @@ -349,8 +404,9 @@ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm } // Check the user hasn't exceeded the maximum selections for the choice(s) they have selected. + $answersnapshots = array(); + $deletedanswersnapshots = array(); if (!($choice->limitanswers && $choicesexceeded)) { - $answersnapshots = array(); if ($current) { // Update an existing answer. $existingchoices = array(); @@ -358,8 +414,8 @@ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm if (in_array($c->optionid, $formanswers)) { $existingchoices[] = $c->optionid; $DB->set_field('choice_answers', 'timemodified', time(), array('id' => $c->id)); - $answersnapshots[] = $c; } else { + $deletedanswersnapshots[] = $c; $DB->delete_records('choice_answers', array('id' => $c->id)); } } @@ -376,9 +432,6 @@ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm $answersnapshots[] = $newanswer; } } - - // Initialised as true, meaning we updated the answer. - $answerupdated = true; } else { // Add new answer. foreach ($formanswers as $answer) { @@ -396,9 +449,6 @@ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm if ($completion->is_enabled($cm) && $choice->completionsubmit) { $completion->update_state($cm, COMPLETION_COMPLETE); } - - // Initalised as false, meaning we submitted a new answer. - $answerupdated = false; } } else { // Check to see if current choice already selected - if not display error. @@ -416,30 +466,12 @@ function choice_user_submit_response($formanswer, $choice, $userid, $course, $cm $choicelock->release(); } - // Now record completed event. - if (isset($answerupdated)) { - $eventdata = array(); - $eventdata['context'] = $context; - $eventdata['objectid'] = $choice->id; - $eventdata['userid'] = $userid; - $eventdata['courseid'] = $course->id; - $eventdata['other'] = array(); - $eventdata['other']['choiceid'] = $choice->id; - - if ($answerupdated) { - $eventdata['other']['optionid'] = $formanswer; - $event = \mod_choice\event\answer_updated::create($eventdata); - } else { - $eventdata['other']['optionid'] = $formanswers; - $event = \mod_choice\event\answer_submitted::create($eventdata); - } - $event->add_record_snapshot('course', $course); - $event->add_record_snapshot('course_modules', $cm); - $event->add_record_snapshot('choice', $choice); - foreach ($answersnapshots as $record) { - $event->add_record_snapshot('choice_answers', $record); - } - $event->trigger(); + // Trigger events. + foreach ($deletedanswersnapshots as $answer) { + \mod_choice\event\answer_deleted::create_from_object($answer, $choice, $cm, $course)->trigger(); + } + foreach ($answersnapshots as $answer) { + \mod_choice\event\answer_created::create_from_object($answer, $choice, $cm, $course)->trigger(); } } @@ -479,6 +511,17 @@ function prepare_choice_show_results($choice, $course, $cm, $allresponses) { $display->coursemoduleid = $cm->id; $display->courseid = $course->id; + if (!empty($choice->showunanswered)) { + $choice->option[0] = get_string('notanswered', 'choice'); + $choice->maxanswers[0] = 0; + } + + // Remove from the list of non-respondents the users who do not have access to this activity. + if (!empty($display->showunanswered) && $allresponses[0]) { + $info = new \core_availability\info_module(cm_info::create($cm)); + $allresponses[0] = $info->filter_user_list($allresponses[0]); + } + //overwrite options value; $display->options = array(); $allusers = []; @@ -531,27 +574,11 @@ function choice_delete_responses($attemptids, $choice, $cm, $course) { } } - $context = context_module::instance($cm->id); $completion = new completion_info($course); foreach($attemptids as $attemptid) { if ($todelete = $DB->get_record('choice_answers', array('choiceid' => $choice->id, 'id' => $attemptid))) { // Trigger the event answer deleted. - $eventdata = array(); - $eventdata['objectid'] = $todelete->id; - $eventdata['context'] = $context; - $eventdata['userid'] = $USER->id; - $eventdata['courseid'] = $course->id; - $eventdata['relateduserid'] = $todelete->userid; - $eventdata['other'] = array(); - $eventdata['other']['choiceid'] = $choice->id; - $eventdata['other']['optionid'] = $todelete->optionid; - $event = \mod_choice\event\answer_deleted::create($eventdata); - $event->add_record_snapshot('course', $course); - $event->add_record_snapshot('course_modules', $cm); - $event->add_record_snapshot('choice', $choice); - $event->add_record_snapshot('choice_answers', $todelete); - $event->trigger(); - + \mod_choice\event\answer_deleted::create_from_object($todelete, $choice, $cm, $course)->trigger(); $DB->delete_records('choice_answers', array('choiceid' => $choice->id, 'id' => $attemptid)); } } diff --git a/mod/choice/renderer.php b/mod/choice/renderer.php index f40f331e0d5..48f31ba3cb7 100644 --- a/mod/choice/renderer.php +++ b/mod/choice/renderer.php @@ -171,7 +171,7 @@ class mod_choice_renderer extends plugin_renderer_base { $usernumberheader->text = get_string('numberofuser', 'choice'); $columns['usernumber'][] = $usernumberheader; - + $optionsnames = []; foreach ($choices->options as $optionid => $options) { $celloption = clone($celldefault); $cellusernumber = clone($celldefault); @@ -179,7 +179,7 @@ class mod_choice_renderer extends plugin_renderer_base { $celltext = ''; if ($choices->showunanswered && $optionid == 0) { - $celltext = format_string(get_string('notanswered', 'choice')); + $celltext = get_string('notanswered', 'choice'); } else if ($optionid > 0) { $celltext = format_string($choices->options[$optionid]->text); } @@ -189,6 +189,7 @@ class mod_choice_renderer extends plugin_renderer_base { } $celloption->text = $celltext; + $optionsnames[$optionid] = $celltext; $cellusernumber->text = $numberofuser; $columns['options'][] = $celloption; @@ -222,10 +223,17 @@ class mod_choice_renderer extends plugin_renderer_base { } $userfullname = fullname($user, $choices->fullnamecapability); - if ($choices->viewresponsecapability && $choices->deleterepsonsecapability && $optionid > 0) { - $attemptaction = html_writer::label($userfullname, 'attempt-user'.$user->id, false, array('class' => 'accesshide')); - $attemptaction .= html_writer::checkbox('attemptid[]', $user->answerid, '', null, - array('id' => 'attempt-user'.$user->id)); + if ($choices->viewresponsecapability && $choices->deleterepsonsecapability) { + $checkboxid = 'attempt-user'.$user->id.'-option'.$optionid; + $attemptaction = html_writer::label($userfullname . ' ' . $optionsnames[$optionid], + $checkboxid, false, array('class' => 'accesshide')); + if ($optionid > 0) { + $attemptaction .= html_writer::checkbox('attemptid[]', $user->answerid, '', null, + array('id' => $checkboxid)); + } else { + $attemptaction .= html_writer::checkbox('userid[]', $user->id, '', null, + array('id' => $checkboxid)); + } $data .= html_writer::tag('div', $attemptaction, array('class'=>'attemptaction')); } $userimage = $this->output->user_picture($user, array('courseid'=>$choices->courseid)); @@ -252,6 +260,7 @@ class mod_choice_renderer extends plugin_renderer_base { if ($choices->viewresponsecapability && $choices->deleterepsonsecapability) { $selecturl = new moodle_url('#'); + $actiondata .= html_writer::start_div('selectallnone'); $selectallactions = new component_action('click',"checkall"); $selectall = new action_link($selecturl, get_string('selectall'), $selectallactions); $actiondata .= $this->output->render($selectall) . ' / '; @@ -260,11 +269,18 @@ class mod_choice_renderer extends plugin_renderer_base { $deselectall = new action_link($selecturl, get_string('deselectall'), $deselectallactions); $actiondata .= $this->output->render($deselectall); - $actiondata .= html_writer::tag('label', ' ' . get_string('withselected', 'choice') . ' ', array('for'=>'menuaction')); + $actiondata .= html_writer::end_div(); $actionurl = new moodle_url($PAGE->url, array('sesskey'=>sesskey(), 'action'=>'delete_confirmation()')); - $select = new single_select($actionurl, 'action', array('delete'=>get_string('delete')), null, array(''=>get_string('chooseaction', 'choice')), 'attemptsform'); - + $actionoptions = array('delete' => get_string('delete')); + foreach ($choices->options as $optionid => $option) { + if ($optionid > 0) { + $actionoptions['choose_'.$optionid] = get_string('chooseoption', 'choice', $option->text); + } + } + $select = new single_select($actionurl, 'action', $actionoptions, null, + array('' => get_string('chooseaction', 'choice')), 'attemptsform'); + $select->set_label(get_string('withselected', 'choice')); $actiondata .= $this->output->render($select); } $html .= html_writer::tag('div', $actiondata, array('class'=>'responseaction')); diff --git a/mod/choice/report.php b/mod/choice/report.php index 59ac1b65770..3fcde4218c3 100644 --- a/mod/choice/report.php +++ b/mod/choice/report.php @@ -4,15 +4,12 @@ require_once("lib.php"); $id = required_param('id', PARAM_INT); //moduleid - $format = optional_param('format', CHOICE_PUBLISH_NAMES, PARAM_INT); $download = optional_param('download', '', PARAM_ALPHA); - $action = optional_param('action', '', PARAM_ALPHA); - $attemptids = optional_param_array('attemptid', array(), PARAM_INT); //get array of responses to delete. + $action = optional_param('action', '', PARAM_ALPHANUMEXT); + $attemptids = optional_param_array('attemptid', array(), PARAM_INT); // Get array of responses to delete or modify. + $userids = optional_param_array('userid', array(), PARAM_INT); // Get array of users whose choices need to be modified. $url = new moodle_url('/mod/choice/report.php', array('id'=>$id)); - if ($format !== CHOICE_PUBLISH_NAMES) { - $url->param('format', $format); - } if ($download !== '') { $url->param('download', $download); } @@ -52,9 +49,18 @@ $event = \mod_choice\event\report_viewed::create($eventdata); $event->trigger(); - if (data_submitted() && $action == 'delete' && has_capability('mod/choice:deleteresponses',$context) && confirm_sesskey()) { - choice_delete_responses($attemptids, $choice, $cm, $course); //delete responses. - redirect("report.php?id=$cm->id"); + if (data_submitted() && has_capability('mod/choice:deleteresponses', $context) && confirm_sesskey()) { + if ($action === 'delete') { + // Delete responses of other users. + choice_delete_responses($attemptids, $choice, $cm, $course); + redirect("report.php?id=$cm->id"); + } + if (preg_match('/^choose_(\d+)$/', $action, $actionmatch)) { + // Modify responses of other users. + $newoptionid = (int)$actionmatch[1]; + choice_modify_responses($userids, $attemptids, $newoptionid, $choice, $cm, $course); + redirect("report.php?id=$cm->id"); + } } if (!$download) { @@ -238,15 +244,11 @@ } exit; } - // Show those who haven't answered the question. - if (!empty($choice->showunanswered)) { - $choice->option[0] = get_string('notanswered', 'choice'); - $choice->maxanswers[0] = 0; - } - + // Always show those who haven't answered the question. + $choice->showunanswered = 1; $results = prepare_choice_show_results($choice, $course, $cm, $users); $renderer = $PAGE->get_renderer('mod_choice'); - echo $renderer->display_result($results, has_capability('mod/choice:readresponses', $context)); + echo $renderer->display_result($results, true); //now give links for downloading spreadsheets. if (!empty($users) && has_capability('mod/choice:downloadresponses',$context)) { diff --git a/mod/choice/styles.css b/mod/choice/styles.css index 2de72fab815..373f27d2854 100644 --- a/mod/choice/styles.css +++ b/mod/choice/styles.css @@ -17,10 +17,11 @@ } .path-mod-choice .anonymous, + .path-mod-choice .names { margin-left: auto; margin-right: auto; - width: 80%; + width: 100%; } .path-mod-choice .downloadreport { diff --git a/mod/choice/tests/behat/modify_choice.feature b/mod/choice/tests/behat/modify_choice.feature new file mode 100644 index 00000000000..9441b8bfc9c --- /dev/null +++ b/mod/choice/tests/behat/modify_choice.feature @@ -0,0 +1,127 @@ +@mod @mod_choice +Feature: Teacher can modify choices of the students + In order to have all students choices + As a teacher + I need to be able to make choice for studnets + + Background: + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + | student3 | Student | 3 | student3@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 | + | student2 | C1 | student | + | student3 | C1 | student | + And the following "activities" exist: + | activity | name | intro | course | idnumber | option | + | choice | Choice name | Test choice description | C1 | choice1 | Option 1, Option 2, Option 3 | + + @javascript + Scenario: Delete students choice response as a teacher + When I log in as "student1" + And I follow "Course 1" + And I choose "Option 1" from "Choice name" choice activity + Then I should see "Your selection: Option 1" + And I should see "Your choice has been saved" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Choice name" + And I follow "View 1 responses" + And I click on "Student 1 Option 1" "checkbox" + And I select "Delete" from the "With selected" singleselect + And "Student 1 Option 1" "checkbox" should not exist + And "Student 1 Not answered yet" "checkbox" should exist + And I log out + + @javascript + Scenario: Teacher set answers of students who did not respond or change existing answers + When I log in as "student1" + And I follow "Course 1" + And I choose "Option 1" from "Choice name" choice activity + Then I should see "Your selection: Option 1" + And I should see "Your choice has been saved" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Choice name" + And I follow "View 1 responses" + And I click on "Student 1 Option 1" "checkbox" + And I click on "Student 2 Not answered yet" "checkbox" + And I click on "Student 3 Not answered yet" "checkbox" + And I select "Choose: Option 2" from the "With selected" singleselect + And "Student 1 Option 1" "checkbox" should not exist + And "Student 2 Not answered yet" "checkbox" should not exist + And "Student 3 Not answered yet" "checkbox" should not exist + And "Student 1 Option 2" "checkbox" should exist + And "Student 2 Option 2" "checkbox" should exist + And "Student 3 Option 2" "checkbox" should exist + And I log out + + @javascript + Scenario: Teacher can delete answers in the multiple answer choice + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Choice name" + And I follow "Edit settings" + And I set the field "Allow more than one choice to be selected" to "Yes" + And I press "Save and return to course" + And I log out + And I log in as "student1" + And I follow "Course 1" + And I choose options "Option 1","Option 2" from "Choice name" choice activity + And 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 "teacher1" + And I follow "Course 1" + And I follow "Choice name" + And I follow "View 1 responses" + And I click on "Student 1 Option 2" "checkbox" + And I select "Delete" from the "With selected" singleselect + And I click on "Student 1 Option 1" "checkbox" + And I select "Choose: Option 3" from the "With selected" singleselect + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Choice name" + And I should see "Your selection: Option 1; Option 3" + And I log out + + @javascript + Scenario: Teacher can manage answers on view page if the names are displayed + When I log in as "student1" + And I follow "Course 1" + And I choose "Option 1" from "Choice name" choice activity + Then I should see "Your selection: Option 1" + And I should see "Your choice has been saved" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Choice name" + And I follow "Edit settings" + And I set the following fields to these values: + | Publish results | Always show results to students | + | Privacy of results | Publish full results, showing names and their choices | + | Show column for unanswered | Yes | + And I press "Save and display" + And I click on "Student 1 Option 1" "checkbox" + And I click on "Student 2 Not answered yet" "checkbox" + And I select "Choose: Option 3" from the "With selected" singleselect + And "Student 1 Option 1" "checkbox" should not exist + And "Student 1 Option 3" "checkbox" should exist + And "Student 2 Not answered yet" "checkbox" should not exist + And "Student 2 Option 3" "checkbox" should exist + And I click on "Student 1 Option 3" "checkbox" + And I select "Delete" from the "With selected" singleselect + And "Student 1 Option 3" "checkbox" should not exist + And "Student 1 Not answered yet" "checkbox" should exist + And I log out diff --git a/mod/choice/tests/events_test.php b/mod/choice/tests/events_test.php index febe209d46a..41fb362ab11 100644 --- a/mod/choice/tests/events_test.php +++ b/mod/choice/tests/events_test.php @@ -64,26 +64,56 @@ class mod_choice_events_testcase extends advanced_testcase { /** * Test to ensure that event data is being stored correctly. */ - public function test_answer_submitted() { + public function test_answer_created() { global $DB; // Generate user data. $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); + + $optionids = array_keys($DB->get_records('choice_options', array('choiceid' => $this->choice->id))); + // Redirect event. + $sink = $this->redirectEvents(); + choice_user_submit_response($optionids[3], $this->choice, $user->id, $this->course, $this->cm); + $events = $sink->get_events(); + $answer = $DB->get_record('choice_answers', ['userid' => $user->id, 'choiceid' => $this->choice->id]); + + // Data checking. + $this->assertCount(1, $events); + $this->assertInstanceOf('\mod_choice\event\answer_created', $events[0]); + $this->assertEquals($user->id, $events[0]->userid); + $this->assertEquals($user->id, $events[0]->relateduserid); + $this->assertEquals(context_module::instance($this->choice->cmid), $events[0]->get_context()); + $this->assertEquals($answer->id, $events[0]->objectid); + $this->assertEquals($this->choice->id, $events[0]->other['choiceid']); + $this->assertEquals($optionids[3], $events[0]->other['optionid']); + $this->assertEventContextNotUsed($events[0]); + $sink->close(); + } + + /** + * Test to ensure that event data is being stored correctly. + */ + public function test_answer_submitted_by_another_user() { + global $DB, $USER; + // Generate user data. + $user = $this->getDataGenerator()->create_user(); $optionids = array_keys($DB->get_records('choice_options', array('choiceid' => $this->choice->id))); // Redirect event. $sink = $this->redirectEvents(); choice_user_submit_response($optionids[3], $this->choice, $user->id, $this->course, $this->cm); $events = $sink->get_events(); + $answer = $DB->get_record('choice_answers', ['userid' => $user->id, 'choiceid' => $this->choice->id]); // Data checking. $this->assertCount(1, $events); - $this->assertInstanceOf('\mod_choice\event\answer_submitted', $events[0]); - $this->assertEquals($user->id, $events[0]->userid); + $this->assertInstanceOf('\mod_choice\event\answer_created', $events[0]); + $this->assertEquals($USER->id, $events[0]->userid); + $this->assertEquals($user->id, $events[0]->relateduserid); $this->assertEquals(context_module::instance($this->choice->cmid), $events[0]->get_context()); + $this->assertEquals($answer->id, $events[0]->objectid); $this->assertEquals($this->choice->id, $events[0]->other['choiceid']); - $this->assertEquals(array($optionids[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->assertEquals($optionids[3], $events[0]->other['optionid']); $this->assertEventContextNotUsed($events[0]); $sink->close(); } @@ -91,11 +121,12 @@ class mod_choice_events_testcase extends advanced_testcase { /** * Test to ensure that multiple choice data is being stored correctly. */ - public function test_answer_submitted_multiple() { + public function test_answer_created_multiple() { global $DB; // Generate user data. $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); // Create multiple choice. $choice = $this->getDataGenerator()->create_module('choice', array('course' => $this->course->id, @@ -110,17 +141,28 @@ class mod_choice_events_testcase extends advanced_testcase { $sink = $this->redirectEvents(); choice_user_submit_response($submittedoptionids, $choice, $user->id, $this->course, $cm); $events = $sink->get_events(); + $answers = $DB->get_records('choice_answers', ['userid' => $user->id, 'choiceid' => $choice->id], 'id'); + $answers = array_values($answers); // Data checking. - $this->assertCount(1, $events); - $this->assertInstanceOf('\mod_choice\event\answer_submitted', $events[0]); + $this->assertCount(2, $events); + $this->assertInstanceOf('\mod_choice\event\answer_created', $events[0]); $this->assertEquals($user->id, $events[0]->userid); + $this->assertEquals($user->id, $events[0]->relateduserid); $this->assertEquals(context_module::instance($choice->cmid), $events[0]->get_context()); + $this->assertEquals($answers[0]->id, $events[0]->objectid); $this->assertEquals($choice->id, $events[0]->other['choiceid']); - $this->assertEquals($submittedoptionids, $events[0]->other['optionid']); - $expected = array($this->course->id, "choice", "choose", 'view.php?id=' . $cm->id, $choice->id, $cm->id); - $this->assertEventLegacyLogData($expected, $events[0]); + $this->assertEquals($optionids[1], $events[0]->other['optionid']); $this->assertEventContextNotUsed($events[0]); + + $this->assertInstanceOf('\mod_choice\event\answer_created', $events[1]); + $this->assertEquals($user->id, $events[1]->userid); + $this->assertEquals($user->id, $events[1]->relateduserid); + $this->assertEquals(context_module::instance($choice->cmid), $events[1]->get_context()); + $this->assertEquals($answers[1]->id, $events[1]->objectid); + $this->assertEquals($choice->id, $events[1]->other['choiceid']); + $this->assertEquals($optionids[3], $events[1]->other['optionid']); + $this->assertEventContextNotUsed($events[1]); $sink->close(); } @@ -129,7 +171,7 @@ class mod_choice_events_testcase extends advanced_testcase { * * @expectedException coding_exception */ - public function test_answer_submitted_other_exception() { + public function test_answer_created_other_exception() { // Generate user data. $user = $this->getDataGenerator()->create_user(); @@ -141,7 +183,7 @@ class mod_choice_events_testcase extends advanced_testcase { $eventdata['other'] = array(); // Make sure content identifier is always set. - $event = \mod_choice\event\answer_submitted::create($eventdata); + $event = \mod_choice\event\answer_created::create($eventdata); $event->trigger(); $this->assertEventContextNotUsed($event); } @@ -153,55 +195,43 @@ class mod_choice_events_testcase extends advanced_testcase { global $DB; // Generate user data. $user = $this->getDataGenerator()->create_user(); + $this->setUser($user); $optionids = array_keys($DB->get_records('choice_options', array('choiceid' => $this->choice->id))); // Create the first answer. choice_user_submit_response($optionids[2], $this->choice, $user->id, $this->course, $this->cm); + $oldanswer = $DB->get_record('choice_answers', ['userid' => $user->id, 'choiceid' => $this->choice->id]); // Redirect event. $sink = $this->redirectEvents(); // Now choose a different answer. choice_user_submit_response($optionids[3], $this->choice, $user->id, $this->course, $this->cm); + $newanswer = $DB->get_record('choice_answers', ['userid' => $user->id, 'choiceid' => $this->choice->id]); $events = $sink->get_events(); // Data checking. - $this->assertCount(1, $events); - $this->assertInstanceOf('\mod_choice\event\answer_updated', $events[0]); + $this->assertCount(2, $events); + $this->assertInstanceOf('\mod_choice\event\answer_deleted', $events[0]); $this->assertEquals($user->id, $events[0]->userid); $this->assertEquals(context_module::instance($this->choice->cmid), $events[0]->get_context()); + $this->assertEquals($oldanswer->id, $events[0]->objectid); $this->assertEquals($this->choice->id, $events[0]->other['choiceid']); - $this->assertEquals($optionids[3], $events[0]->other['optionid']); - $expected = array($this->course->id, "choice", "choose again", 'view.php?id=' . $this->cm->id, - $this->choice->id, $this->cm->id); - $this->assertEventLegacyLogData($expected, $events[0]); + $this->assertEquals($optionids[2], $events[0]->other['optionid']); $this->assertEventContextNotUsed($events[0]); + + $this->assertInstanceOf('\mod_choice\event\answer_created', $events[1]); + $this->assertEquals($user->id, $events[1]->userid); + $this->assertEquals(context_module::instance($this->choice->cmid), $events[1]->get_context()); + $this->assertEquals($newanswer->id, $events[1]->objectid); + $this->assertEquals($this->choice->id, $events[1]->other['choiceid']); + $this->assertEquals($optionids[3], $events[1]->other['optionid']); + $this->assertEventContextNotUsed($events[1]); + $sink->close(); } - /** - * Test custom validations for answer_updated event. - * - * @expectedException coding_exception - */ - public function test_answer_updated_other_exception() { - // Generate user data. - $user = $this->getDataGenerator()->create_user(); - - $eventdata = array(); - $eventdata['context'] = $this->context; - $eventdata['objectid'] = 2; - $eventdata['userid'] = $user->id; - $eventdata['courseid'] = $this->course->id; - $eventdata['other'] = array(); - - // Make sure content identifier is always set. - $event = \mod_choice\event\answer_updated::create($eventdata); - $event->trigger(); - $this->assertEventContextNotUsed($event); - } - /** * Test to ensure that event data is being stored correctly. */ @@ -216,7 +246,7 @@ class mod_choice_events_testcase extends advanced_testcase { choice_user_submit_response($optionids[2], $this->choice, $user->id, $this->course, $this->cm); // Get the users response. $answer = $DB->get_record('choice_answers', array('userid' => $user->id, 'choiceid' => $this->choice->id), - '*', $strictness = IGNORE_MULTIPLE); + '*', $strictness = IGNORE_MULTIPLE); // Redirect event. $sink = $this->redirectEvents(); @@ -269,7 +299,7 @@ class mod_choice_events_testcase extends advanced_testcase { $this->assertEquals($USER->id, $event[0]->userid); $this->assertEquals(context_module::instance($this->choice->cmid), $event[0]->get_context()); $expected = array($this->course->id, "choice", "report", 'report.php?id=' . $this->context->instanceid, - $this->choice->id, $this->context->instanceid); + $this->choice->id, $this->context->instanceid); $this->assertEventLegacyLogData($expected, $event[0]); $this->assertEventContextNotUsed($event[0]); $sink->close(); @@ -341,7 +371,7 @@ class mod_choice_events_testcase extends advanced_testcase { $this->assertEquals($USER->id, $event[0]->userid); $this->assertEquals(context_module::instance($this->choice->cmid), $event[0]->get_context()); $expected = array($this->course->id, "choice", "view", 'view.php?id=' . $this->context->instanceid, - $this->choice->id, $this->context->instanceid); + $this->choice->id, $this->context->instanceid); $this->assertEventLegacyLogData($expected, $event[0]); $this->assertEventContextNotUsed($event[0]); $sink->close(); diff --git a/mod/choice/tests/generator/lib.php b/mod/choice/tests/generator/lib.php index d737e1585ed..8327fccafe8 100644 --- a/mod/choice/tests/generator/lib.php +++ b/mod/choice/tests/generator/lib.php @@ -47,6 +47,8 @@ class mod_choice_generator extends testing_module_generator { $record->option[] = 'Beer'; $record->option[] = 'Wine'; $record->option[] = 'Spirits'; + } else if (!is_array($record->option)) { + $record->option = preg_split('/\s*,\s*/', trim($record->option), -1, PREG_SPLIT_NO_EMPTY); } return parent::create_instance($record, (array)$options); } diff --git a/mod/choice/upgrade.txt b/mod/choice/upgrade.txt index b43dede06b8..40d80d48d50 100644 --- a/mod/choice/upgrade.txt +++ b/mod/choice/upgrade.txt @@ -1,6 +1,15 @@ This files describes API changes in /mod/choice/*, information provided here is intended especially for developers. +=== 3.2 === + +* Events mod_choice\event\answer_submitted and mod_choice\event\answer_updated + are no longer triggered. Observers listening to these events must instead listen + to mod_choice\event\answer_created and mod_choice\event\answer_deleted that are + triggered for each option that is selected or unselected. User whose choice was + modified can be found in $event->relateduserid (this does not have to be the + user who performs the action). + === 3.0 === * External function mod_choice_external::get_choices_by_courses returned parameter "name" and diff --git a/mod/choice/view.php b/mod/choice/view.php index 59cc9df28f1..332de9d5382 100644 --- a/mod/choice/view.php +++ b/mod/choice/view.php @@ -5,8 +5,9 @@ require_once("lib.php"); require_once($CFG->libdir . '/completionlib.php'); $id = required_param('id', PARAM_INT); // Course Module ID -$action = optional_param('action', '', PARAM_ALPHA); -$attemptids = optional_param_array('attemptid', array(), PARAM_INT); // array of attempt ids for delete action +$action = optional_param('action', '', PARAM_ALPHANUMEXT); +$attemptids = optional_param_array('attemptid', array(), PARAM_INT); // Get array of responses to delete or modify. +$userids = optional_param_array('userid', array(), PARAM_INT); // Get array of users whose choices need to be modified. $notify = optional_param('notify', '', PARAM_ALPHA); $url = new moodle_url('/mod/choice/view.php', array('id'=>$id)); @@ -52,12 +53,20 @@ $PAGE->set_title($choice->name); $PAGE->set_heading($course->fullname); /// Submit any new data if there is any -if (data_submitted() && is_enrolled($context, NULL, 'mod/choice:choose') && confirm_sesskey()) { +if (data_submitted() && !empty($action) && confirm_sesskey()) { $timenow = time(); - 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 (has_capability('mod/choice:deleteresponses', $context)) { + if ($action === 'delete') { + // Some responses need to be deleted. + choice_delete_responses($attemptids, $choice, $cm, $course); + redirect("view.php?id=$cm->id"); + } + if (preg_match('/^choose_(\d+)$/', $action, $actionmatch)) { + // Modify responses of other users. + $newoptionid = (int)$actionmatch[1]; + choice_modify_responses($userids, $attemptids, $newoptionid, $choice, $cm, $course); + redirect("view.php?id=$cm->id"); + } } // Redirection after all POSTs breaks block editing, we need to be more specific! @@ -72,7 +81,7 @@ if (data_submitted() && is_enrolled($context, NULL, 'mod/choice:choose') && conf throw new moodle_exception($reason, 'choice', '', $warnings[$reason]); } - if ($answer) { + if ($answer && is_enrolled($context, null, 'mod/choice:choose')) { choice_user_submit_response($answer, $choice, $USER->id, $course, $cm); redirect(new moodle_url('/mod/choice/view.php', array('id' => $cm->id, 'notify' => 'choicesaved', 'sesskey' => sesskey()))); @@ -192,11 +201,6 @@ if (!$choiceformshown) { // print the results at the bottom of the screen if (choice_can_view_results($choice, $current, $choiceopen)) { - - if (!empty($choice->showunanswered)) { - $choice->option[0] = get_string('notanswered', 'choice'); - $choice->maxanswers[0] = 0; - } $results = prepare_choice_show_results($choice, $course, $cm, $allresponses); $renderer = $PAGE->get_renderer('mod_choice'); echo $renderer->display_result($results);