feat(forms): Server side validation for mandatory questions

ccailly · ccailly · commit 3f7b57d204d7 · 2025-05-07T10:01:30.000+02:00
feat(tests): add validation tests for mandatory and optional form answers

feat(tests): improve E2E tests and enchance accesbility

fix: lint tests
diff --git a/css/includes/components/form/_form-renderer.scss b/css/includes/components/form/_form-renderer.scss
@@ -111,4 +111,27 @@
             }
         }
     }
+
+    // Handle invalid states for specific inputs (select2 and tinymce)
+    [data-glpi-form-renderer-question] {
+        &:has(select.is-invalid) {
+            .select2-container--default .select2-selection {
+                border-color: var(--tblr-form-invalid-border-color) !important;
+            }
+        }
+
+        &:has(textarea.is-invalid) {
+            .tox.tox-tinymce {
+                border-color: var(--tblr-form-invalid-border-color) !important;
+            }
+        }
+
+        .invalid-tooltip {
+            position: relative;
+            background-color: unset;
+            color: var(--tblr-form-invalid-color);
+            padding: unset;
+            margin-top: .25rem;
+        }
+    }
 }
diff --git a/js/modules/Forms/RendererController.js b/js/modules/Forms/RendererController.js
@@ -134,39 +134,61 @@ export class GlpiFormRendererController
         $(this.#target).removeClass('pointer-events-none');
     }
 
-    #checkCurrentSectionValidity() {
-        // Find all required inputs that are hidden and not already disabled.
-        // They must be removed from the check as they are inputs from others
-        // sections or input hidden by condition (thus they should not be
-        // evaluated).
-        // The easiest way to not evaluate these inputs is to disable them.
-        const inputs = $(this.#target).find('[required]:hidden:not(:disabled)');
-        for (const input of inputs) {
-            input.disabled = true;
-        }
+    async #checkCurrentSectionValidity() {
+        // Get the UUID of the current section
+        const currentSectionElement = $(this.#target).find(`[data-glpi-form-renderer-section="${this.#section_index}"]`);
+        const currentSectionUuid = currentSectionElement.data('glpi-form-renderer-uuid');
+
+        const response = await $.ajax({
+            url: `${CFG_GLPI.root_doc}/Form/ValidateAnswers`,
+            type: 'POST',
+            data: `${$(this.#target).serialize()  }&section_uuid=${currentSectionUuid}`,
+            dataType: 'json',
+        });
 
-        // Check validity and display browser feedback if needed.
-        const is_valid = this.#target.checkValidity();
-        if (!is_valid) {
-            this.#target.reportValidity();
-        }
+        if (response.success === false) {
+            // Remove previous error messages
+            $(this.#target)
+                .find(".invalid-tooltip")
+                .remove();
+            $(this.#target)
+                .find(".is-invalid")
+                .removeClass("is-invalid");
+
+            Object.values(response.errors).forEach(error => {
+                // Highlight the field with error
+                const question = $(`[data-glpi-form-renderer-id="${error.question_id}"][data-glpi-form-renderer-question]`);
+                if (question.length) {
+                    // Find the input field within the question
+                    const inputField = question.find('input:not([data-uploader-name]), select, textarea');
+                    if (inputField.length) {
+                        // Generate a unique ID for the error message
+                        const errorId = `error-${error.question_id}`;
+
+                        // Add validation classes and accessibility attributes
+                        inputField
+                            .addClass('is-invalid')
+                            .attr('aria-invalid', 'true')
+                            .attr('aria-errormessage', errorId);
+
+                        // Add a tooltip with the error message
+                        inputField.parent().append(
+                            `<span id="${errorId}" class="invalid-tooltip">${error.message}</span>`
+                        );
+                    }
+                }
+            });
 
-        // Revert disabled inputs
-        for (const input of inputs) {
-            input.disabled = false;
+            return false;
         }
 
-        return is_valid;
+        return true;
     }
 
     /**
      * Submit the target form using an AJAX request.
      */
     async #submitForm() {
-        if (!this.#checkCurrentSectionValidity()) {
-            return;
-        }
-
         // Form will be sumitted using an AJAX request instead
         try {
             // Update tinymce values
@@ -176,6 +198,10 @@ export class GlpiFormRendererController
                 });
             }
 
+            if (!await this.#checkCurrentSectionValidity()) {
+                return;
+            }
+
             // Submit form using AJAX
             const response = await $.post({
                 url: $(this.#target).prop("action"),
@@ -217,8 +243,8 @@ export class GlpiFormRendererController
     /**
      * Go to the next section of the form.
      */
-    #goToNextSection() {
-        if (!this.#checkCurrentSectionValidity()) {
+    async #goToNextSection() {
+        if (!await this.#checkCurrentSectionValidity()) {
             return;
         }
 
diff --git a/phpunit/functional/Glpi/Form/AnswersHandler/AnswersHandlerTest.php b/phpunit/functional/Glpi/Form/AnswersHandler/AnswersHandlerTest.php
@@ -164,6 +164,65 @@ public function testSaveAnswers(): void
         );
     }
 
+    public function testValidateAnswers(): void
+    {
+        self::login();
+
+        // Create a form with both mandatory and optional questions
+        $builder = new FormBuilder("Validation Test Form");
+        $builder
+            ->addQuestion("Mandatory Name", QuestionTypeShortText::class, is_mandatory: true)
+            ->addQuestion("Mandatory Email", QuestionTypeEmail::class, is_mandatory: true)
+            ->addQuestion("Optional Comment", QuestionTypeLongText::class, is_mandatory: false)
+        ;
+        $form = self::createForm($builder);
+
+        // Get handler instance
+        $handler = AnswersHandler::getInstance();
+
+        // Test 1: All mandatory fields are filled - should be valid
+        $complete_answers = [
+            self::getQuestionId($form, "Mandatory Name") => "John Doe",
+            self::getQuestionId($form, "Mandatory Email") => "john.doe@example.com",
+            self::getQuestionId($form, "Optional Comment") => "This is an optional comment",
+        ];
+        $result = $handler->validateAnswers($form, $complete_answers);
+        $this->assertTrue($result->isValid(), "Validation should pass when all mandatory fields are filled");
+        $this->assertCount(0, $result->getErrors(), "There should be no errors when all mandatory fields are filled");
+
+        // Test 2: Missing one mandatory field - should be invalid
+        $missing_name_answers = [
+            self::getQuestionId($form, "Mandatory Email") => "john.doe@example.com",
+            self::getQuestionId($form, "Optional Comment") => "This is an optional comment",
+        ];
+        $result = $handler->validateAnswers($form, $missing_name_answers);
+        $this->assertFalse($result->isValid(), "Validation should fail when a mandatory field is missing");
+        $this->assertCount(1, $result->getErrors(), "There should be one error when one mandatory field is missing");
+
+        // Test 3: Missing all mandatory fields - should be invalid with multiple errors
+        $only_optional_answers = [
+            self::getQuestionId($form, "Optional Comment") => "This is an optional comment",
+        ];
+        $result = $handler->validateAnswers($form, $only_optional_answers);
+        $this->assertFalse($result->isValid(), "Validation should fail when all mandatory fields are missing");
+        $this->assertCount(2, $result->getErrors(), "There should be two errors when both mandatory fields are missing");
+
+        // Test 4: Empty answers - should be invalid with multiple errors
+        $empty_answers = [];
+        $result = $handler->validateAnswers($form, $empty_answers);
+        $this->assertFalse($result->isValid(), "Validation should fail when answers are empty");
+        $this->assertCount(2, $result->getErrors(), "There should be two errors when answers are empty");
+
+        // Test 5: Empty string in mandatory field - should be invalid
+        $empty_string_answers = [
+            self::getQuestionId($form, "Mandatory Name") => "",
+            self::getQuestionId($form, "Mandatory Email") => "john.doe@example.com",
+        ];
+        $result = $handler->validateAnswers($form, $empty_string_answers);
+        $this->assertFalse($result->isValid(), "Validation should fail when a mandatory field contains an empty string");
+        $this->assertCount(1, $result->getErrors(), "There should be one error when a mandatory field contains an empty string");
+    }
+
     private function validateAnswers(
         Form $form,
         array $answers,
diff --git a/src/Glpi/Controller/Form/SubmitAnswerController.php b/src/Glpi/Controller/Form/SubmitAnswerController.php
@@ -109,6 +109,12 @@ private function saveSubmittedAnswers(
         }
 
         $handler = AnswersHandler::getInstance();
+
+        // Check if answers are valid
+        if (!$handler->validateAnswers($form, $answers)->isValid()) {
+            throw new BadRequestHttpException();
+        }
+
         $answers = $handler->saveAnswers(
             $form,
             $answers,
diff --git a/src/Glpi/Controller/Form/ValidateAnswerController.php b/src/Glpi/Controller/Form/ValidateAnswerController.php
@@ -0,0 +1,121 @@
+<?php
+
+/**
+ * ---------------------------------------------------------------------
+ *
+ * GLPI - Gestionnaire Libre de Parc Informatique
+ *
+ * http://glpi-project.org
+ *
+ * @copyright 2015-2025 Teclib' and contributors.
+ * @licence   https://www.gnu.org/licenses/gpl-3.0.html
+ *
+ * ---------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of GLPI.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * ---------------------------------------------------------------------
+ */
+
+namespace Glpi\Controller\Form;
+
+use Glpi\Controller\AbstractController;
+use Glpi\Controller\Form\Utils\CanCheckAccessPolicies;
+use Glpi\Exception\Http\BadRequestHttpException;
+use Glpi\Exception\Http\NotFoundHttpException;
+use Glpi\Form\AnswersHandler\AnswersHandler;
+use Glpi\Form\EndUserInputNameProvider;
+use Glpi\Form\Form;
+use Glpi\Form\Section;
+use Glpi\Form\ValidationResult;
+use Glpi\Http\Firewall;
+use Glpi\Security\Attribute\SecurityStrategy;
+use Symfony\Component\HttpFoundation\JsonResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Routing\Attribute\Route;
+
+final class ValidateAnswerController extends AbstractController
+{
+    use CanCheckAccessPolicies;
+
+    #[SecurityStrategy(Firewall::STRATEGY_NO_CHECK)] // Some forms can be accessed anonymously
+    #[Route(
+        "/Form/ValidateAnswers",
+        name: "glpi_form_validate_answers",
+        methods: "POST"
+    )]
+    public function __invoke(Request $request): Response
+    {
+        $form = $this->loadSubmittedForm($request);
+        $section = $this->loadSubmittedSection($request);
+        $this->checkFormAccessPolicies($form, $request);
+
+        $validation_result = $this->checkSubmittedAnswersValidation($section, $request);
+        return new JsonResponse([
+            'success' => $validation_result->isValid(),
+            'errors' => $validation_result->getErrors(),
+        ]);
+    }
+
+    private function loadSubmittedForm(Request $request): Form
+    {
+        $forms_id = $request->request->getInt("forms_id");
+        if (!$forms_id) {
+            throw new BadRequestHttpException();
+        }
+
+        $form = Form::getById($forms_id);
+        if (!$form) {
+            throw new NotFoundHttpException();
+        }
+
+        return $form;
+    }
+
+    private function loadSubmittedSection(Request $request): Section
+    {
+        $section_uuid = $request->request->getString("section_uuid");
+        if (!$section_uuid) {
+            throw new BadRequestHttpException();
+        }
+
+        $section = Section::getByUuid($section_uuid);
+        if (!$section) {
+            throw new NotFoundHttpException();
+        }
+
+        return $section;
+    }
+
+    private function checkSubmittedAnswersValidation(
+        Form|Section $questions_container,
+        Request $request
+    ): ValidationResult {
+        $post = $request->request->all();
+        $provider = new EndUserInputNameProvider();
+
+        $answers = $provider->getAnswers($post);
+        if (empty($answers)) {
+            throw new BadRequestHttpException();
+        }
+
+        $handler = AnswersHandler::getInstance();
+        return $handler->validateAnswers($questions_container, $answers);
+    }
+}
diff --git a/src/Glpi/Form/AnswersHandler/AnswersHandler.php b/src/Glpi/Form/AnswersHandler/AnswersHandler.php
@@ -44,6 +44,8 @@
 use Glpi\Form\Destination\AnswersSet_FormDestinationItem;
 use Glpi\Form\Destination\FormDestination;
 use Glpi\Form\Form;
+use Glpi\Form\Section;
+use Glpi\Form\ValidationResult;
 
 /**
  * Helper class to handle raw answers data
@@ -75,6 +77,35 @@ public static function getInstance(): AnswersHandler
         return self::$instance;
     }
 
+    /**
+     * Check if the given answers are valid for the given form
+     *
+     * @param Form|Section $questions_container The form or section to check
+     * @param array $answers The answers to check
+     * @return ValidationResult The validation result
+     */
+    public function validateAnswers(
+        Form|Section $questions_container,
+        array $answers
+    ): ValidationResult {
+        $result = new ValidationResult();
+
+        // Check if mandatory questions are answered
+        $mandatory_questions = array_filter(
+            $questions_container->getQuestions(),
+            static fn($question) => $question->fields['is_mandatory']
+        );
+
+        foreach ($mandatory_questions as $question) {
+            // Check if the question is not answered (empty or not set)
+            if (empty($answers[$question->getID()])) {
+                $result->addError($question, __('This field is mandatory'));
+            }
+        }
+
+        return $result;
+    }
+
     /**
      * Saves the given answers of a given form into an AnswersSet object and
      * create destinations objects.
diff --git a/src/Glpi/Form/ValidationResult.php b/src/Glpi/Form/ValidationResult.php
diff --git a/tests/cypress/e2e/form/form_rendering.cy.js b/tests/cypress/e2e/form/form_rendering.cy.js
