fix: Make sure "other" answers are correctly handled

In the backend it handling the "other" answer setting was not changed after switching from object to array for extra settings. In the frontend the value handling of the checkboxes or radio switches was not correct as the initial values is always an empty array. This lead to issues with radio switch answers. Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
nextcloud · Oct 26, 2023 · 8303476 · 8303476
1 parent 6d74165
commit 8303476
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 26 deletions.
diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php
@@ -961,7 +961,7 @@ public function insertSubmission(int $formId, array $answers, string $shareHash
 					$optionIndex = array_search($answer, array_column($question['options'], 'id'));
 					if ($optionIndex !== false) {
 						$answerText = $question['options'][$optionIndex]['text'];
-					} elseif (!empty($question['extraSettings']->allowOtherAnswer) && strpos($answer, Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX) === 0) {
+					} elseif (!empty($question['extraSettings']['allowOtherAnswer']) && strpos($answer, Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX) === 0) {
 						$answerText = str_replace(Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX, "", $answer);
 					}
 				} else {

diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php
@@ -302,7 +302,7 @@ private function array2csv(array $header, array $records): string {
 	 * @return boolean If the submission is valid
 	 */
 	public function validateSubmission(array $questions, array $answers): bool {
-		
+
 		// Check by questions
 		foreach ($questions as $question) {
 			$questionId = $question['id'];
@@ -312,7 +312,7 @@ public function validateSubmission(array $questions, array $answers): bool {
 			if ($question['isRequired'] &&
 				(!$questionAnswered ||
 				!array_filter($answers[$questionId], 'strlen') ||
-				(!empty($question['extraSettings']->allowOtherAnswer) && !array_filter($answers[$questionId], fn ($value) => $value !== Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX)))
+				(!empty($question['extraSettings']['allowOtherAnswer']) && !array_filter($answers[$questionId], fn ($value) => $value !== Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX)))
 			) {
 				return false;
 			}
@@ -334,7 +334,7 @@ public function validateSubmission(array $questions, array $answers): bool {
 				}
 
 				// Check if all answers are within the possible options
-				if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED) && empty($question['extraSettings']->allowOtherAnswer)) {
+				if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED) && empty($question['extraSettings']['allowOtherAnswer'])) {
 					foreach ($answers[$questionId] as $answer) {
 						// Search corresponding option, return false if non-existent
 						if (array_search($answer, array_column($question['options'], 'id')) === false) {

diff --git a/src/components/Questions/QuestionMultiple.vue b/src/components/Questions/QuestionMultiple.vue
@@ -48,7 +48,7 @@
 			<fieldset :name="name || undefined" :aria-labelledby="titleId">
 				<NcCheckboxRadioSwitch v-for="(answer) in sortedOptions"
 					:key="answer.id"
-					:checked.sync="questionValues"
+					:checked="questionValues"
 					:value="answer.id.toString()"
 					:name="`${id}-answer`"
 					:type="isUnique ? 'radio' : 'checkbox'"
@@ -58,7 +58,7 @@
 					{{ answer.text }}
 				</NcCheckboxRadioSwitch>
 				<div v-if="allowOtherAnswer" class="question__other-answer">
-					<NcCheckboxRadioSwitch :checked.sync="questionValues"
+					<NcCheckboxRadioSwitch :checked="questionValues"
 						:value="valueOtherAnswer"
 						:name="`${id}-answer`"
 						:type="isUnique ? 'radio' : 'checkbox'"
@@ -149,20 +149,12 @@ export default {
 
 	data() {
 		return {
-			questionValues: this.values,
 			inputOtherAnswer: this.valueToInputOtherAnswer(),
 			QUESTION_EXTRASETTINGS_OTHER_PREFIX: 'system-other-answer:',
 		}
 	},
 
 	computed: {
-		placeholderOtherAnswer() {
-			if (this.readOnly) {
-				return this.answerType.submitPlaceholder
-			}
-			return this.answerType.createPlaceholder
-		},
-
 		contentValid() {
 			return this.answerType.validate(this)
 		},
@@ -192,6 +184,17 @@ export default {
 			return this.isUnique ? IconRadioboxBlank : IconCheckboxBlankOutline
 		},
 
+		placeholderOtherAnswer() {
+			if (this.readOnly) {
+				return this.answerType.submitPlaceholder
+			}
+			return this.answerType.createPlaceholder
+		},
+
+		questionValues() {
+			return this.isUnique ? this.values?.[0] : this.values
+		},
+
 		titleId() {
 			return `q${this.$attrs.index}_title`
 		},
@@ -205,7 +208,7 @@ export default {
 		},
 
 		hasRequiredOtherAnswerInput() {
-			const checkedOtherAnswer = this.questionValues.filter(item => item.startsWith(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX))
+			const checkedOtherAnswer = this.values.filter(item => item.startsWith(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX))
 			return checkedOtherAnswer[0] !== undefined
 		},
 	},
@@ -229,24 +232,22 @@ export default {
 
 		inputOtherAnswer() {
 			if (this.isUnique) {
-				this.questionValues = this.valueOtherAnswer
-				this.onChange()
+				this.onChange(this.valueOtherAnswer)
 				return
 			}
 
-			this.questionValues = this.questionValues.filter(item => !item.startsWith(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX))
-
+			const values = this.values.filter(item => !item.startsWith(this.QUESTION_EXTRASETTINGS_OTHER_PREFIX))
 			if (this.inputOtherAnswer !== '') {
-				this.questionValues.push(this.valueOtherAnswer)
+				values.push(this.valueOtherAnswer)
 			}
 
-			this.onChange()
+			this.onChange(values)
 		},
 	},
 
 	methods: {
-		onChange() {
-			this.$emit('update:values', this.isUnique ? [this.questionValues] : this.questionValues)
+		onChange(value) {
+			this.$emit('update:values', this.isUnique ? [value] : value)
 		},
 
 		/**

diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php
@@ -572,7 +572,7 @@ public function testInsertSubmission_answers() {
 			[
 				'id' => 2,
 				'type' => Constants::ANSWER_TYPE_MULTIPLE,
-				'extraSettings' => (object)['allowOtherAnswer' => true],
+				'extraSettings' => ['allowOtherAnswer' => true],
 				'options' => [
 					['id' => 1, 'text' => 'test id 1'],
 					['id' => 2, 'text' => 'test id 2'],

diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php
@@ -564,7 +564,7 @@ public function dataValidateSubmission() {
 			'required-empty-other-answer' => [
 				// Questions
 				[
-					['id' => 1, 'type' => 'multiple_unique', 'isRequired' => true, 'extraSettings' => (object)['allowOtherAnswer' => true], 'options' => [
+					['id' => 1, 'type' => 'multiple_unique', 'isRequired' => true, 'extraSettings' => ['allowOtherAnswer' => true], 'options' => [
 						['id' => 3]
 					]]
 				],
@@ -664,7 +664,7 @@ public function dataValidateSubmission() {
 						['id' => 6]
 					]],
 					['id' => 8, 'type' => 'time', 'isRequired' => false],
-					['id' => 9, 'type' => 'multiple_unique', 'isRequired' => true, 'extraSettings' => (object)['allowOtherAnswer' => true], 'options' => [
+					['id' => 9, 'type' => 'multiple_unique', 'isRequired' => true, 'extraSettings' => ['allowOtherAnswer' => true], 'options' => [
 						['id' => 3]
 					]],
 				],