diff --git a/mod/feedback/classes/complete_form.php b/mod/feedback/classes/complete_form.php index 44480a26b97..7bf40ab0953 100644 --- a/mod/feedback/classes/complete_form.php +++ b/mod/feedback/classes/complete_form.php @@ -293,15 +293,22 @@ class mod_feedback_complete_form extends moodleform { */ public function add_form_element($item, $element, $addrequiredrule = true, $setdefaultvalue = true) { global $OUTPUT; - // Add element to the form. - if (is_array($element)) { - if ($this->is_frozen() && $element[0] === 'text') { - // Convert 'text' element to 'static' when freezing for better display. - $element = ['static', $element[1], $element[2]]; + + if (is_array($element) && $element[0] == 'group') { + // For groups, use the mforms addGroup API. + // $element looks like: ['group', $groupinputname, $name, $objects, $separator, $appendname], + $element = $this->_form->addGroup($element[3], $element[1], $element[2], $element[4], $element[5]); + } else { + // Add non-group element to the form. + if (is_array($element)) { + if ($this->is_frozen() && $element[0] === 'text') { + // Convert 'text' element to 'static' when freezing for better display. + $element = ['static', $element[1], $element[2]]; + } + $element = call_user_func_array(array($this->_form, 'createElement'), $element); } - $element = call_user_func_array(array($this->_form, 'createElement'), $element); + $element = $this->_form->addElement($element); } - $element = $this->_form->addElement($element); // Prepend standard CSS classes to the element classes. $attributes = $element->getAttributes(); diff --git a/mod/feedback/item/multichoice/lib.php b/mod/feedback/item/multichoice/lib.php index 9dea3fe0207..0c0286be41c 100644 --- a/mod/feedback/item/multichoice/lib.php +++ b/mod/feedback/item/multichoice/lib.php @@ -316,14 +316,19 @@ class feedback_item_multichoice extends feedback_item_base { $inputname = $item->typ . '_' . $item->id; $options = $this->get_options($item); $separator = !empty($info->horizontal) ? ' ' : '
'; - $tmpvalue = $form->get_item_value($item); + $tmpvalue = $form->get_item_value($item) ?? 0; // Used for element defaults, so must be a valid value (not null). + // Subtypes: + // r = radio + // c = checkbox + // d = dropdown. if ($info->subtype === 'd' || ($info->subtype === 'r' && $form->is_frozen())) { // Display as a dropdown in the complete form or a single value in the response view. $element = $form->add_form_element($item, - ['select', $inputname.'[0]', $name, array(0 => '') + $options, array('class' => $class)], + ['select', $inputname, $name, array(0 => '') + $options, array('class' => $class)], false, false); - $form->set_element_default($inputname.'[0]', $tmpvalue); + $form->set_element_default($inputname, $tmpvalue); + $form->set_element_type($inputname, PARAM_INT); } else if ($info->subtype === 'c' && $form->is_frozen()) { // Display list of checkbox values in the response view. $objs = []; @@ -354,26 +359,26 @@ class feedback_item_multichoice extends feedback_item_base { } else { // Radio. if (!array_key_exists(0, $options)) { - // Always add '0' as hidden element, otherwise form submit data may not have this element. - $objs[] = ['hidden', $inputname.'[0]']; + // Always add a hidden element to the group to guarantee we get a value in the submit data. + $objs[] = ['hidden', $inputname, 0]; } foreach ($options as $idx => $label) { - $objs[] = ['radio', $inputname.'[0]', '', $label, $idx]; + $objs[] = ['radio', $inputname, '', $label, $idx]; } // Span to hold the element id. The id is used for drag and drop reordering. $objs[] = ['static', '', '', html_writer::span('', '', ['id' => 'feedback_item_' . $item->id])]; $element = $form->add_form_group_element($item, 'group_'.$inputname, $name, $objs, $separator, $class); - $form->set_element_default($inputname.'[0]', $tmpvalue); - $form->set_element_type($inputname.'[0]', PARAM_INT); + $form->set_element_default($inputname, $tmpvalue); + $form->set_element_type($inputname, PARAM_INT); } } // Process 'required' rule. if ($item->required) { $elementname = $element->getName(); - $form->add_validation_rule(function($values, $files) use ($elementname, $item) { + $form->add_validation_rule(function($values) use ($elementname, $item) { $inputname = $item->typ . '_' . $item->id; - return empty($values[$inputname]) || !array_filter($values[$inputname]) ? + return empty($values[$inputname]) || (is_array($values[$inputname]) && !array_filter($values[$inputname])) ? array($elementname => get_string('required')) : true; }); } @@ -385,6 +390,9 @@ class feedback_item_multichoice extends feedback_item_base { * @return string */ public function create_value($value) { + // Could be an array (multichoice checkbox) or single value (multichoice radio or dropdown). + $value = is_array($value) ? $value : [$value]; + $value = array_unique(array_filter($value)); return join(FEEDBACK_MULTICHOICE_LINE_SEP, $value); } diff --git a/mod/feedback/tests/external_test.php b/mod/feedback/tests/external_test.php index 3ab5d194938..9aff6c589f8 100644 --- a/mod/feedback/tests/external_test.php +++ b/mod/feedback/tests/external_test.php @@ -518,7 +518,7 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { $this->assertCount(7, $tmpitems); // 2 from the first page + 5 from the second page. // And finally, save everything! We are going to modify one previous recorded value. - $data[2]['value'] = 'b'; + $data[2]['value'] = 2; // 2 is value of the option 'b'. $secondpagedata = [$data[2], $data[3], $data[4], $data[5], $data[6]]; $result = mod_feedback_external::process_page($this->feedback->id, 1, $secondpagedata); $result = external_api::clean_returnvalue(mod_feedback_external::process_page_returns(), $result); @@ -530,7 +530,9 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { // Check if the one we modified was correctly saved. $itemid = $itemscreated[4]->id; $itemsaved = $DB->get_field('feedback_value', 'value', array('item' => $itemid)); - $this->assertEquals('b', $itemsaved); + $mcitem = new feedback_item_multichoice(); + $itemval = $mcitem->get_printval($itemscreated[4], (object) ['value' => $itemsaved]); + $this->assertEquals('b', $itemval); // Check that the answers are saved for course 0. foreach ($items as $item) { @@ -593,7 +595,7 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { $this->assertFalse($result['completed']); // Process second page. - $data[2]['value'] = 'b'; + $data[2]['value'] = 2; // 2 is value of the option 'b'; $secondpagedata = [$data[2], $data[3], $data[4], $data[5], $data[6]]; $result = mod_feedback_external::process_page($this->feedback->id, 1, $secondpagedata, false, $this->course->id); $result = external_api::clean_returnvalue(mod_feedback_external::process_page_returns(), $result); @@ -605,7 +607,9 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { // Check if the one we modified was correctly saved. $itemid = $itemscreated[4]->id; $itemsaved = $DB->get_field('feedback_value', 'value', array('item' => $itemid)); - $this->assertEquals('b', $itemsaved); + $mcitem = new feedback_item_multichoice(); + $itemval = $mcitem->get_printval($itemscreated[4], (object) ['value' => $itemsaved]); + $this->assertEquals('b', $itemval); // Check that the answers are saved for the correct course. foreach ($items as $item) {