Skip to content

Commit a3425b3

Browse files
ccaillyccailly
authored
feat(forms): Server side validation for mandatory questions
1 parent dec5092 commit a3425b3

File tree

9 files changed

+552
-47
lines changed

9 files changed

+552
-47
lines changed

‎css/includes/components/form/_form-renderer.scss

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,27 @@
111111
}
112112
}
113113
}
114+
115+
// Handle invalid states for specific inputs (select2 and tinymce)
116+
[data-glpi-form-renderer-question] {
117+
&:has(select.is-invalid) {
118+
.select2-container--default .select2-selection {
119+
border-color: var(--tblr-form-invalid-border-color) !important;
120+
}
121+
}
122+
123+
&:has(textarea.is-invalid) {
124+
.tox.tox-tinymce {
125+
border-color: var(--tblr-form-invalid-border-color) !important;
126+
}
127+
}
128+
129+
.invalid-tooltip {
130+
position: relative;
131+
background-color: unset;
132+
color: var(--tblr-form-invalid-color);
133+
padding: unset;
134+
margin-top: .25rem;
135+
}
136+
}
114137
}

‎js/modules/Forms/RendererController.js

Lines changed: 57 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -134,39 +134,67 @@ export class GlpiFormRendererController
134134
$(this.#target).removeClass('pointer-events-none');
135135
}
136136

137-
#checkCurrentSectionValidity() {
138-
// Find all required inputs that are hidden and not already disabled.
139-
// They must be removed from the check as they are inputs from others
140-
// sections or input hidden by condition (thus they should not be
141-
// evaluated).
142-
// The easiest way to not evaluate these inputs is to disable them.
143-
const inputs = $(this.#target).find('[required]:hidden:not(:disabled)');
144-
for (const input of inputs) {
145-
input.disabled = true;
146-
}
137+
async #checkCurrentSectionValidity() {
138+
// Get the UUID of the current section
139+
const currentSectionElement = $(this.#target).find(`[data-glpi-form-renderer-section="${this.#section_index}"]`);
140+
const currentSectionUuid = currentSectionElement.data('glpi-form-renderer-uuid');
141+
142+
const response = await $.ajax({
143+
url: `${CFG_GLPI.root_doc}/Form/ValidateAnswers`,
144+
type: 'POST',
145+
data: `${$(this.#target).serialize() }&section_uuid=${currentSectionUuid}`,
146+
dataType: 'json',
147+
});
147148

148-
// Check validity and display browser feedback if needed.
149-
const is_valid = this.#target.checkValidity();
150-
if (!is_valid) {
151-
this.#target.reportValidity();
152-
}
149+
// Remove previous error messages and aria attributes
150+
$(this.#target)
151+
.find(".invalid-tooltip")
152+
.remove();
153+
$(this.#target)
154+
.find(".is-invalid")
155+
.removeClass("is-invalid")
156+
.removeAttr("aria-invalid")
157+
.removeAttr("aria-errormessage");
158+
159+
if (response.success === false) {
160+
Object.values(response.errors).forEach(error => {
161+
// Highlight the field with error
162+
const question = $(`[data-glpi-form-renderer-id="${error.question_id}"][data-glpi-form-renderer-question]`);
163+
if (!question.length) {
164+
return;
165+
}
166+
167+
// Find the input field within the question
168+
const inputField = question.find('input:not([type=hidden]):not([data-uploader-name]):not(.select2-search__field), select, textarea');
169+
if (!inputField.length) {
170+
return;
171+
}
172+
173+
// Generate a unique ID for the error message
174+
const errorId = `error-${error.question_id}`;
175+
176+
// Add validation classes and accessibility attributes
177+
inputField
178+
.addClass('is-invalid')
179+
.attr('aria-invalid', 'true')
180+
.attr('aria-errormessage', errorId);
181+
182+
// Add a tooltip with the error message
183+
inputField.parent().append(
184+
`<span id="${errorId}" class="invalid-tooltip">${error.message}</span>`
185+
);
186+
});
153187

154-
// Revert disabled inputs
155-
for (const input of inputs) {
156-
input.disabled = false;
188+
return false;
157189
}
158190

159-
return is_valid;
191+
return true;
160192
}
161193

162194
/**
163195
* Submit the target form using an AJAX request.
164196
*/
165197
async #submitForm() {
166-
if (!this.#checkCurrentSectionValidity()) {
167-
return;
168-
}
169-
170198
// Form will be sumitted using an AJAX request instead
171199
try {
172200
// Update tinymce values
@@ -176,6 +204,10 @@ export class GlpiFormRendererController
176204
});
177205
}
178206

207+
if (!await this.#checkCurrentSectionValidity()) {
208+
return;
209+
}
210+
179211
// Submit form using AJAX
180212
const response = await $.post({
181213
url: $(this.#target).prop("action"),
@@ -217,8 +249,8 @@ export class GlpiFormRendererController
217249
/**
218250
* Go to the next section of the form.
219251
*/
220-
#goToNextSection() {
221-
if (!this.#checkCurrentSectionValidity()) {
252+
async #goToNextSection() {
253+
if (!await this.#checkCurrentSectionValidity()) {
222254
return;
223255
}
224256

‎phpunit/functional/Glpi/Form/AnswersHandler/AnswersHandlerTest.php

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use Glpi\Form\Condition\LogicOperator;
4343
use Glpi\Form\Condition\Type;
4444
use Glpi\Form\Condition\ValueOperator;
45+
use Glpi\Form\Condition\VisibilityStrategy;
4546
use Glpi\Form\Destination\FormDestinationChange;
4647
use Glpi\Form\Destination\FormDestinationProblem;
4748
use Glpi\Form\Destination\FormDestinationTicket;
@@ -164,6 +165,117 @@ public function testSaveAnswers(): void
164165
);
165166
}
166167

168+
public function testValidateAnswers(): void
169+
{
170+
self::login();
171+
172+
// Create a form with both mandatory and optional questions
173+
$builder = new FormBuilder("Validation Test Form");
174+
$builder
175+
->addQuestion("Mandatory Name", QuestionTypeShortText::class, is_mandatory: true)
176+
->addQuestion("Mandatory Email", QuestionTypeEmail::class, is_mandatory: true)
177+
->addQuestion("Optional Comment", QuestionTypeLongText::class, is_mandatory: false)
178+
;
179+
$form = self::createForm($builder);
180+
181+
// Get handler instance
182+
$handler = AnswersHandler::getInstance();
183+
184+
// Test 1: All mandatory fields are filled - should be valid
185+
$complete_answers = [
186+
self::getQuestionId($form, "Mandatory Name") => "John Doe",
187+
self::getQuestionId($form, "Mandatory Email") => "john.doe@example.com",
188+
self::getQuestionId($form, "Optional Comment") => "This is an optional comment",
189+
];
190+
$result = $handler->validateAnswers($form, $complete_answers);
191+
$this->assertTrue($result->isValid(), "Validation should pass when all mandatory fields are filled");
192+
$this->assertCount(0, $result->getErrors(), "There should be no errors when all mandatory fields are filled");
193+
194+
// Test 2: Missing one mandatory field - should be invalid
195+
$missing_name_answers = [
196+
self::getQuestionId($form, "Mandatory Email") => "john.doe@example.com",
197+
self::getQuestionId($form, "Optional Comment") => "This is an optional comment",
198+
];
199+
$result = $handler->validateAnswers($form, $missing_name_answers);
200+
$this->assertFalse($result->isValid(), "Validation should fail when a mandatory field is missing");
201+
$this->assertCount(1, $result->getErrors(), "There should be one error when one mandatory field is missing");
202+
203+
// Test 3: Missing all mandatory fields - should be invalid with multiple errors
204+
$only_optional_answers = [
205+
self::getQuestionId($form, "Optional Comment") => "This is an optional comment",
206+
];
207+
$result = $handler->validateAnswers($form, $only_optional_answers);
208+
$this->assertFalse($result->isValid(), "Validation should fail when all mandatory fields are missing");
209+
$this->assertCount(2, $result->getErrors(), "There should be two errors when both mandatory fields are missing");
210+
211+
// Test 4: Empty answers - should be invalid with multiple errors
212+
$empty_answers = [];
213+
$result = $handler->validateAnswers($form, $empty_answers);
214+
$this->assertFalse($result->isValid(), "Validation should fail when answers are empty");
215+
$this->assertCount(2, $result->getErrors(), "There should be two errors when answers are empty");
216+
217+
// Test 5: Empty string in mandatory field - should be invalid
218+
$empty_string_answers = [
219+
self::getQuestionId($form, "Mandatory Name") => "",
220+
self::getQuestionId($form, "Mandatory Email") => "john.doe@example.com",
221+
];
222+
$result = $handler->validateAnswers($form, $empty_string_answers);
223+
$this->assertFalse($result->isValid(), "Validation should fail when a mandatory field contains an empty string");
224+
$this->assertCount(1, $result->getErrors(), "There should be one error when a mandatory field contains an empty string");
225+
}
226+
227+
public function testValidateAnswersWithConditions(): void
228+
{
229+
self::login();
230+
231+
// Create a form with conditional questions
232+
$builder = new FormBuilder("Conditional Validation Test Form");
233+
$builder
234+
->addQuestion("Main Question", QuestionTypeShortText::class, is_mandatory: true)
235+
->addQuestion("Conditional Question", QuestionTypeShortText::class, is_mandatory: true)
236+
->setQuestionVisibility(
237+
"Conditional Question",
238+
VisibilityStrategy::VISIBLE_IF,
239+
[
240+
[
241+
'logic_operator' => LogicOperator::AND,
242+
'item_name' => "Main Question",
243+
'item_type' => Type::QUESTION,
244+
'value_operator' => ValueOperator::EQUALS,
245+
'value' => "Show Conditional",
246+
],
247+
]
248+
)
249+
;
250+
$form = self::createForm($builder);
251+
252+
// Get handler instance
253+
$handler = AnswersHandler::getInstance();
254+
255+
// Test 1: Conditional question is shown - should be valid
256+
$conditional_answers = [
257+
self::getQuestionId($form, "Main Question") => "Show Conditional",
258+
self::getQuestionId($form, "Conditional Question") => "This is a conditional answer",
259+
];
260+
$result = $handler->validateAnswers($form, $conditional_answers);
261+
$this->assertTrue($result->isValid(), "Validation should pass when the conditional question is shown and filled");
262+
263+
// Test 2: Conditional question is not shown - should be valid
264+
$non_conditional_answers = [
265+
self::getQuestionId($form, "Main Question") => "Do not show",
266+
];
267+
$result = $handler->validateAnswers($form, $non_conditional_answers);
268+
$this->assertTrue($result->isValid(), "Validation should pass when the conditional question is not shown");
269+
270+
// Test 3: Conditional question is shown but not filled - should be invalid
271+
$missing_conditional_answers = [
272+
self::getQuestionId($form, "Main Question") => "Show Conditional",
273+
self::getQuestionId($form, "Conditional Question") => "",
274+
];
275+
$result = $handler->validateAnswers($form, $missing_conditional_answers);
276+
$this->assertFalse($result->isValid(), "Validation should fail when the conditional question is shown but not filled");
277+
}
278+
167279
private function validateAnswers(
168280
Form $form,
169281
array $answers,

‎src/Glpi/Controller/Form/SubmitAnswerController.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,12 @@ private function saveSubmittedAnswers(
109109
}
110110

111111
$handler = AnswersHandler::getInstance();
112+
113+
// Check if answers are valid
114+
if (!$handler->validateAnswers($form, $answers)->isValid()) {
115+
throw new BadRequestHttpException();
116+
}
117+
112118
$answers = $handler->saveAnswers(
113119
$form,
114120
$answers,

0 commit comments

Comments
 (0)