Skip to content

Commit 7e9b52d

Browse files
Chartman123Chartman123
committed
enh: improve error messages for required questions.
Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>
1 parent a138698 commit 7e9b52d

10 files changed

Lines changed: 148 additions & 65 deletions

‎lib/Service/SubmissionService.php‎

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -485,9 +485,16 @@ public function validateSubmission(array $questions, array $answers, string $for
485485
if ($question['isRequired']
486486
&& (!$questionAnswered
487487
|| !array_filter($answers[$questionId], static function (string|array $value): bool {
488-
// file type
489488
if (is_array($value)) {
490-
return !empty($value['uploadedFileId']);
489+
// file type
490+
if (isset($value['uploadedFileId'])) {
491+
return !empty($value['uploadedFileId']);
492+
}
493+
494+
// Grid questions
495+
return !empty(array_filter($value, static function ($subValue): bool {
496+
return is_array($subValue) ? !empty(array_filter($subValue)) : $subValue !== '';
497+
}));
491498
}
492499

493500
return $value !== '';
@@ -557,16 +564,19 @@ public function validateSubmission(array $questions, array $answers, string $for
557564
}
558565
// Search corresponding option, return false if non-existent
559566
else {
560-
// Accept numeric strings like "46" from JSON payloads reliably (e.g. with hardening extensions enabled)
561-
$answerId = is_int($answer) ? $answer : (is_string($answer) ? intval(trim($answer)) : null);
562-
563-
// Reject non-numeric / malformed values early
564-
if ($answerId === null || (string)$answerId !== (string)intval($answerId)) {
565-
throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', is_scalar($answer) ? (string)$answer : gettype($answer), $question['text']));
566-
}
567+
$subAnswers = is_array($answer) ? $answer : [$answer];
568+
foreach ($subAnswers as $subAnswer) {
569+
// Accept numeric strings like "46" from JSON payloads reliably (e.g. with hardening extensions enabled)
570+
$answerId = is_int($subAnswer) ? $subAnswer : (is_string($subAnswer) ? intval(trim($subAnswer)) : null);
571+
572+
// Reject non-numeric / malformed values early
573+
if ($answerId === null || (string)$answerId !== (string)intval($answerId)) {
574+
throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', is_scalar($subAnswer) ? (string)$subAnswer : gettype($subAnswer), $question['text']));
575+
}
567576

568-
if (!in_array($answerId, $optionIds, true)) {
569-
throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', $answer, $question['text']));
577+
if (!in_array($answerId, $optionIds, true)) {
578+
throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', $subAnswer, $question['text']));
579+
}
570580
}
571581
}
572582
}

‎src/components/Questions/QuestionColor.vue‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
v-bind="questionProps"
99
:titlePlaceholder="answerType.titlePlaceholder"
1010
:warningInvalid="answerType.warningInvalid"
11+
:errorMessage="errorMessage"
1112
v-on="commonListeners">
1213
<div
1314
class="question__content"
@@ -17,6 +18,7 @@
1718
<NcColorPicker
1819
:modelValue="pickedColor"
1920
advancedFields
21+
:aria-required="isRequired"
2022
@update:modelValue="onUpdatePickedColor">
2123
<NcButton :disabled="!readOnly">
2224
{{ colorPickerPlaceholder }}
@@ -87,6 +89,16 @@ export default {
8789
},
8890
8991
methods: {
92+
async validate() {
93+
if (this.isRequired && this.pickedColor === '') {
94+
this.errorMessage = t('forms', 'You must answer this question')
95+
return false
96+
}
97+
98+
this.errorMessage = null
99+
return true
100+
},
101+
90102
onUpdatePickedColor(color) {
91103
this.$emit('update:values', [color])
92104
},

‎src/components/Questions/QuestionDate.vue‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
v-bind="questionProps"
99
:titlePlaceholder="answerType.titlePlaceholder"
1010
:warningInvalid="answerType.warningInvalid"
11+
:errorMessage="errorMessage"
1112
v-on="commonListeners">
1213
<template v-if="answerType.pickerType === 'date'" #actions>
1314
<NcActionCheckbox
@@ -95,7 +96,8 @@
9596
:type="dateTimePickerType"
9697
:disabledDate="disabledDates"
9798
:disabledTime="disabledTimes"
98-
rangeSeparator=" - "
99+
:aria-required="isRequired"
100+
clearable
99101
@update:modelValue="onValueChange" />
100102
</div>
101103
<template #insert>
@@ -146,25 +148,31 @@ export default {
146148
},
147149
148150
computed: {
151+
isRangeQuestion() {
152+
return this.extraSettings?.dateRange || this.extraSettings?.timeRange
153+
? true
154+
: false
155+
},
156+
149157
datetimePickerPlaceholder() {
150158
if (this.readOnly) {
151-
return this.extraSettings?.dateRange || this.extraSettings?.timeRange
159+
return this.isRangeQuestion
152160
? this.answerType.submitPlaceholderRange
153161
: this.answerType.submitPlaceholder
154162
}
155-
return this.extraSettings?.dateRange || this.extraSettings?.timeRange
163+
return this.isRangeQuestion
156164
? this.answerType.createPlaceholderRange
157165
: this.answerType.createPlaceholder
158166
},
159167
160168
dateTimePickerType() {
161-
return this.extraSettings?.dateRange || this.extraSettings?.timeRange
169+
return this.isRangeQuestion
162170
? this.answerType.pickerType + '-range'
163171
: this.answerType.pickerType
164172
},
165173
166174
time() {
167-
if (this.extraSettings?.dateRange || this.extraSettings?.timeRange) {
175+
if (this.isRangeQuestion) {
168176
return this.values?.[0]
169177
? [this.parse(this.values[0]), this.parse(this.values[1])]
170178
: null
@@ -224,14 +232,27 @@ export default {
224232
},
225233
226234
methods: {
235+
async validate() {
236+
if (this.isRequired && this.time === null) {
237+
this.errorMessage = t('forms', 'You must answer this question')
238+
return false
239+
}
240+
241+
this.errorMessage = null
242+
return true
243+
},
244+
227245
/**
228246
* DateTimepicker show text in picker
229247
* Format depends on component-type date/datetime
230248
*
231-
* @param {Date} date the selected datepicker Date
249+
* @param {Date|Date[]} date the selected datepicker Date
232250
* @return {string}
233251
*/
234252
stringify(date) {
253+
if (this.isRangeQuestion && Array.isArray(date)) {
254+
return `${moment(date[0]).format(this.answerType.momentFormat)} - ${moment(date[1]).format(this.answerType.momentFormat)}`
255+
}
235256
return moment(date).format(this.answerType.momentFormat)
236257
},
237258
@@ -335,10 +356,15 @@ export default {
335356
/**
336357
* Store Value
337358
*
338-
* @param {Date|Array<Date>} date The date or date range to store
359+
* @param {Date|Array<Date>|null} date The date or date range to store
339360
*/
340361
onValueChange(date) {
341-
if (this.extraSettings?.dateRange || this.extraSettings?.timeRange) {
362+
if (!date) {
363+
this.$emit('update:values', [])
364+
return
365+
}
366+
367+
if (this.isRangeQuestion) {
342368
this.$emit('update:values', [
343369
moment(date[0]).format(this.answerType.storageFormat),
344370
moment(date[1]).format(this.answerType.storageFormat),

‎src/components/Questions/QuestionDropdown.vue‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
:warningInvalid="answerType.warningInvalid"
1111
:contentValid="contentValid"
1212
:shiftDragHandle="shiftDragHandle"
13+
:errorMessage="errorMessage"
1314
v-on="commonListeners">
1415
<template #actions>
1516
<NcActionCheckbox
@@ -40,6 +41,7 @@
4041
:searchable="false"
4142
label="text"
4243
:aria-label-combobox="selectOptionPlaceholder"
44+
@invalid.prevent="validate"
4345
@update:modelValue="onInput" />
4446
</div>
4547
<template v-else>
@@ -184,6 +186,16 @@ export default {
184186
},
185187
186188
methods: {
189+
async validate() {
190+
if (this.isRequired && this.areNoneChecked) {
191+
this.errorMessage = t('forms', 'You must answer this question')
192+
return false
193+
}
194+
195+
this.errorMessage = null
196+
return true
197+
},
198+
187199
onDragStart() {
188200
this.isDragging = true
189201
},

‎src/components/Questions/QuestionFile.vue‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,12 @@
118118
ref="fileInput"
119119
class="hidden-visually"
120120
type="file"
121+
:required="isRequired && values.length === 0"
121122
:disabled="!readOnly"
122123
:multiple="maxAllowedFilesCount > 1"
123124
:name="name || undefined"
124125
:accept="accept.length ? accept.join(',') : null"
126+
@invalid.prevent="validate"
125127
@input="onFileInput" />
126128
</label>
127129
<NcButton
@@ -422,6 +424,12 @@ export default {
422424
)
423425
return false
424426
}
427+
428+
if (this.isRequired && this.values.length === 0) {
429+
this.errorMessage = t('forms', 'You must answer this question')
430+
return false
431+
}
432+
425433
this.errorMessage = null
426434
return true
427435
},

‎src/components/Questions/QuestionGrid.vue‎

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,19 @@
2323
<th class="first-column"></th>
2424

2525
<th
26-
v-for="column of columns"
26+
v-for="column in columns"
2727
:key="column.local ? 'option-local' : column.id">
2828
{{ column.text }}
2929
</th>
3030
</tr>
3131
</thead>
3232
<tbody>
3333
<tr
34-
v-for="row of rows"
34+
v-for="row in rows"
3535
:key="row.local ? 'option-local' : row.id">
3636
<td class="first-column">{{ row.text }}</td>
3737
<td
38-
v-for="column of columns"
38+
v-for="column in columns"
3939
:key="column.local ? 'option-local' : column.id">
4040
<template v-if="questionType === 'radio'">
4141
<NcCheckboxRadioSwitch
@@ -264,6 +264,14 @@ export default {
264264
265265
methods: {
266266
async validate() {
267+
if (
268+
this.isRequired
269+
&& (this.values.length === 0 || this.values === null)
270+
) {
271+
this.errorMessage = t('forms', 'You must answer this question')
272+
return false
273+
}
274+
267275
if (!this.isUnique) {
268276
// Validate limits
269277
const max = this.extraSettings.optionsLimitMax ?? 0
@@ -315,30 +323,6 @@ export default {
315323
316324
this.$emit('update:values', values)
317325
},
318-
319-
/**
320-
* Is the provided answer required ?
321-
* This is needed for checkboxes as html5
322-
* doesn't allow to require at least ONE checked.
323-
* So we require the one that are checked or all
324-
* if none are checked yet.
325-
*
326-
* @return {boolean}
327-
*/
328-
checkRequired() {
329-
// false, if question not required
330-
if (!this.isRequired) {
331-
return false
332-
}
333-
334-
// true for Radiobuttons
335-
if (this.isUnique) {
336-
return true
337-
}
338-
339-
// For checkboxes, only required if no other is checked
340-
return this.areNoneChecked
341-
},
342326
},
343327
}
344328
</script>

‎src/components/Questions/QuestionLinearScale.vue‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
v-bind="questionProps"
99
:titlePlaceholder="answerType.titlePlaceholder"
1010
:warningInvalid="answerType.warningInvalid"
11+
:errorMessage="errorMessage"
1112
v-on="commonListeners">
1213
<template #actions>
1314
<NcActionInput
@@ -85,7 +86,8 @@
8586
:value="option.toString()"
8687
:name="`${id}-answer`"
8788
type="radio"
88-
:required="checkRequired(option)"
89+
:required="isRequired"
90+
@invalid.prevent="validate"
8991
@update:modelValue="onChange"
9092
@keydown.enter.exact.prevent="onKeydownEnter" />
9193
</div>
@@ -203,6 +205,16 @@ export default {
203205
},
204206
205207
methods: {
208+
async validate() {
209+
if (this.isRequired && this.values.length === 0) {
210+
this.errorMessage = t('forms', 'You must answer this question')
211+
return false
212+
}
213+
214+
this.errorMessage = null
215+
return true
216+
},
217+
206218
onChange(option) {
207219
this.$emit('update:values', [option])
208220
},
@@ -231,24 +243,6 @@ export default {
231243
})
232244
},
233245
234-
/**
235-
* Is the provided answer required ?
236-
* This is needed for checkboxes as html5
237-
* doesn't allow to require at least ONE checked.
238-
* So we require the one that are checked or all
239-
* if none are checked yet.
240-
*
241-
* @return {boolean}
242-
*/
243-
checkRequired() {
244-
// false, if question not required
245-
if (!this.isRequired) {
246-
return false
247-
}
248-
249-
return true
250-
},
251-
252246
/**
253247
* Resizes the given label to fit within the specified constraints.
254248
*

0 commit comments

Comments
 (0)