Skip to content

Commit 4f37b86

Browse files
AdrienClairembaultcedric-anne
authored andcommitted
Client side validation for mandatory questions
1 parent 3cd910c commit 4f37b86

File tree

4 files changed

+123
-7
lines changed

4 files changed

+123
-7
lines changed

js/modules/Forms/Condition/Engine.js

-2
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,6 @@ export class GlpiFormConditionEngine
4343

4444
async computeVisiblity(container)
4545
{
46-
container = document.querySelector(container);
47-
4846
try {
4947
// Send data to server for computation and apply results.
5048
return await this.#computeVisibilityOnBackend({

js/modules/Forms/RendererController.js

+37-4
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ import { GlpiFormConditionEngine } from './Condition/Engine.js';
4141
export class GlpiFormRendererController
4242
{
4343
/**
44-
* Target form (jquery selector)
45-
* @type {string}
44+
* Target form
45+
* @type {HTMLFormElement}
4646
*/
4747
#target;
4848

@@ -66,7 +66,7 @@ export class GlpiFormRendererController
6666
*/
6767
constructor(target, form_id) {
6868
// Target must be a valid form
69-
this.#target = target;
69+
this.#target = document.querySelector(target);
7070
if ($(this.#target).prop("tagName") != "FORM") {
7171
throw new Error("Target must be a valid form");
7272
}
@@ -124,10 +124,39 @@ export class GlpiFormRendererController
124124
});
125125
}
126126

127+
#checkCurrentSectionValidity() {
128+
// Find all required inputs that are hidden and not already disabled.
129+
// They must be removed from the check as they are inputs from others
130+
// sections or input hidden by condition (thus they should not be
131+
// evaluated).
132+
// The easiest way to not evaluate these inputs is to disable them.
133+
const inputs = $(this.#target).find('[required]:hidden:not(:disabled)');
134+
for (const input of inputs) {
135+
input.disabled = true;
136+
}
137+
138+
// Check validity and display browser feedback if needed.
139+
const is_valid = this.#target.checkValidity();
140+
if (!is_valid) {
141+
this.#target.reportValidity();
142+
}
143+
144+
// Revert disabled inputs
145+
for (const input of inputs) {
146+
input.disabled = false;
147+
}
148+
149+
return is_valid;
150+
}
151+
127152
/**
128153
* Submit the target form using an AJAX request.
129154
*/
130155
async #submitForm() {
156+
if (!this.#checkCurrentSectionValidity()) {
157+
return;
158+
}
159+
131160
// Form will be sumitted using an AJAX request instead
132161
try {
133162
// Update tinymce values
@@ -178,6 +207,10 @@ export class GlpiFormRendererController
178207
* Go to the next section of the form.
179208
*/
180209
#goToNextSection() {
210+
if (!this.#checkCurrentSectionValidity()) {
211+
return;
212+
}
213+
181214
// Hide current section and its questions
182215
$(this.#target)
183216
.find(`
@@ -319,7 +352,7 @@ export class GlpiFormRendererController
319352

320353
#applyVisibilityResults(results)
321354
{
322-
const container = document.querySelector(this.#target);
355+
const container = this.#target;
323356

324357
// Apply sections visibility
325358
for (const [id, must_be_visible] of Object.entries(

tests/cypress/e2e/form/form_rendering.cy.js

+82
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,88 @@ describe('Form rendering', () => {
8888
cy.getDropdownByLabelText('Urgency').should('have.text', "Very low");
8989
cy.getDropdownByLabelText('Request type').should('have.text', "Request");
9090
});
91+
92+
it('Mandatory questions must be filled', () => {
93+
// Set up a form with two sections, each with a mandatory question
94+
cy.createWithAPI('Glpi\\Form\\Form', {
95+
'name': 'Test mandatory questions',
96+
}).as('form_id');
97+
cy.get('@form_id').then((form_id) => {
98+
// Add mandatory question to default section
99+
cy.addQuestionToDefaultSectionWithAPI(
100+
form_id,
101+
'First question',
102+
'Glpi\\Form\\QuestionType\\QuestionTypeShortText',
103+
0,
104+
null,
105+
null,
106+
null,
107+
true // Mandatory
108+
);
109+
110+
// Add second section
111+
cy.createWithAPI('Glpi\\Form\\Section', {
112+
'name': 'Second section',
113+
'rank': 1,
114+
'forms_forms_id': form_id,
115+
}).as('second_section_id');
116+
117+
cy.get('@second_section_id').then((second_section_id) => {
118+
// Add mandatory question to second section
119+
cy.createWithAPI('Glpi\\Form\\Question', {
120+
'name': 'Second question',
121+
'type': 'Glpi\\Form\\QuestionType\\QuestionTypeShortText',
122+
'vertical_rank': 1,
123+
'forms_sections_id': second_section_id,
124+
'is_mandatory' : true,
125+
}).as('second_section_id');
126+
});
127+
128+
// Preview form
129+
cy.login();
130+
cy.visit(`/Form/Render/${form_id}`);
131+
132+
// Try to submit first section, should fail since we didn't answer
133+
// the mandatory question
134+
cy.findByRole('button', {name: 'Continue'}).click();
135+
// Wait to be sure that the current state will not be used a false
136+
// positive to pass the following assertions in case the current
137+
// section was not yet updated by the JS logic.
138+
// This would not be needed if we could detect and wait for the "Please
139+
// fill this input" popup from the browser but it doesn't seem
140+
// supported by cypress.
141+
// eslint-disable-next-line cypress/no-unnecessary-waiting
142+
cy.wait(600);
143+
cy.findByRole('heading', {name: 'First section'}).should('be.visible');
144+
cy.findByRole('heading', {name: 'Second section'}).should('not.exist');
145+
146+
// Submit again with a value
147+
cy.findByRole('textbox', {name: 'First question'}).type("test");
148+
cy.findByRole('button', {name: 'Continue'}).click();
149+
cy.findByRole('heading', {name: 'Second section'}).should('be.visible');
150+
cy.findByRole('heading', {name: 'First section'}).should('not.exist');
151+
152+
// Try to submit the final section, should fail since we didn't answer
153+
// the mandatory question
154+
cy.findByRole('button', {name: 'Submit'}).click();
155+
// Wait to be sure that the current state will not be used a false
156+
// positive to pass the following assertions in case the current
157+
// section was not yet updated by the JS logic.
158+
// This would not be needed if we could detect and wait for the "Please
159+
// fill this input" popup from the browser but it doesn't seem
160+
// supported by cypress.
161+
// eslint-disable-next-line cypress/no-unnecessary-waiting
162+
cy.wait(600);
163+
cy.findByRole('heading', {name: 'Second section'}).should('be.visible');
164+
cy.findByText('Form submitted').should('not.be.visible');
165+
166+
// Submit again with a value
167+
cy.findByRole('textbox', {name: 'Second question'}).type("test");
168+
cy.findByRole('button', {name: 'Submit'}).click();
169+
cy.findByRole('heading', {name: 'Second section'}).should('not.exist');
170+
cy.findByText('Form submitted').should('be.visible');
171+
});
172+
});
91173
});
92174

93175
function addQuestionAndGetUuuid(name, type = null, subtype = null) {

tests/cypress/support/commands/form.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,16 @@ Cypress.Commands.add('addSection', (name) => {
7676
cy.focused().type(name); // Section name is focused by default
7777
});
7878

79+
// TODO: refactor on playwright; too many args
7980
Cypress.Commands.add('addQuestionToDefaultSectionWithAPI', (
8081
form_id,
8182
name,
8283
type,
8384
vertical_rank,
8485
horizontal_rank = 0,
8586
default_value = null,
86-
extra_data = null
87+
extra_data = null,
88+
is_mandatory = false,
8789
) => {
8890
cy.initApi().doApiRequest('GET', `Glpi\\Form\\Form/${form_id}/Glpi\\Form\\Section`).then((response) => {
8991
const section_id = response.body[0].id;
@@ -95,6 +97,7 @@ Cypress.Commands.add('addQuestionToDefaultSectionWithAPI', (
9597
horizontal_rank : horizontal_rank,
9698
default_value : default_value,
9799
extra_data : extra_data,
100+
is_mandatory : is_mandatory,
98101
};
99102

100103
return cy.createWithAPI('Glpi\\Form\\Question', question).then((question_id) => {

0 commit comments

Comments
 (0)