Skip to content

Commit

Permalink
[FIX] survey: prevent soft lock with conditional questions
Browse files Browse the repository at this point in the history
If multiple questions are conditionally displayed if the user selects an answer
in another multi-choice question, that answer was added multiple times in the
list used to filter mandatory questions that have to be answered when displayed.
This is an issue because the answer was only removed once if the user changes
his choice resulting in a state where a mandatory question that is hidden stays
mandatory even though it is not displayed to the user.

How to reproduce:
- create a new survey with:
      - Question1:
        - multi-choice with one answer
        - 2 answers (A, B)
      - Question2:
        - single line text box
        - mandatory answer
        - conditional display depending on answer B of Question1
      - Question3:
        - single line text box
        - conditional display depending on answer B of Question1
- start the survey
- click on answer B then click on answer A

Current behavior:
- user is not able to submit the survey with answer A selected

Expected behavior:
- user should be able to submit the survey

task-3630079

closes odoo#145392

Signed-off-by: Florian Charlier (flch) <[email protected]>
Co-authored-by: Salvo Rapisarda <[email protected]>
Co-authored-by: Damien Abeloos <[email protected]>
  • Loading branch information
abd-msyukyu-odoo and salvorapi committed Dec 19, 2023
1 parent 5502bec commit 34b6a7a
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 14 deletions.
8 changes: 5 additions & 3 deletions addons/survey/static/src/js/survey_form.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,15 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend({
}
if (Object.keys(this.options.triggeredQuestionsByAnswer).includes($target.val())) {
// Display depending question
const selectedAnswerId = parseInt($target.val());
this.options.triggeredQuestionsByAnswer[$target.val()].forEach(function (questionId) {
if (!treatedQuestionIds.includes(questionId)) {
var dependingQuestion = $('.js_question-wrapper#' + questionId);
dependingQuestion.removeClass('d-none');

// Add answer to selected answer
self.selectedAnswers.push(parseInt($target.val()));
if (!self.selectedAnswers.includes(selectedAnswerId)) {
// Add answer to selected answer
self.selectedAnswers.push(selectedAnswerId);
}
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,46 @@ tour.register('test_survey_chained_conditional_questions', {
{
content: 'Click on Start',
trigger: 'button.btn:contains("Start")',
}, {
content: 'Answer Q1 with Answer 2',
trigger: 'div.js_question-wrapper:contains("Q1") label:contains("Answer 2")',
}, {
content: 'Check that Q4 and Q5 are visible',
trigger: 'div.js_question-wrapper:contains(Q4)',
extra_trigger: 'div.js_question-wrapper:contains(Q5)',
run: () => {
const selector = 'div.js_question-wrapper.d-none';
if (document.querySelectorAll(selector).length !== 2) {
throw new Error('Q2 and Q3 should be hidden.');
}
},
}, {
content: 'Answer Q1 with Answer 1',
trigger: 'div.js_question-wrapper:contains("Q1") label:contains("Answer 1")',
}, {
content: 'Answer Q2 with Answer 1',
trigger: 'div.js_question-wrapper:contains("Q2") label:contains("Answer 1")',
run: function (actions) {
const selector = 'div.js_question-wrapper.d-none';
if (document.querySelectorAll(selector).length !== 3) {
throw new Error('Q3, Q4 and Q5 should be hidden.');
}
// Select Answer 1, in order to trigger the display of Q3.
actions.click(this.$anchor);
}
}, {
content: 'Answer Q3 with Answer 1',
trigger: 'div.js_question-wrapper:contains("Q3") label:contains("Answer 1")',
}, {
content: 'Answer Q1 with Answer 2', // This should hide all remaining questions.
trigger: 'div.js_question-wrapper:contains("Q1") label:contains("Answer 2")',
content: 'Answer Q1 with Answer 3', // This should hide all remaining questions.
trigger: 'div.js_question-wrapper:contains("Q1") label:contains("Answer 3")',
}, {
content: 'Check that only question 1 is now visible',
trigger: 'div.js_question-wrapper:contains("Q1")',
run: () => {
const selector = 'div.js_question-wrapper.d-none';
if (document.querySelectorAll(selector).length !== 2) {
throw new Error('Q2 and Q3 should have been hidden.');
if (document.querySelectorAll(selector).length !== 4) {
throw new Error('Q2, Q3, Q4 and Q5 should have been hidden.');
}
}
}, {
Expand Down
32 changes: 25 additions & 7 deletions addons/survey/tests/test_survey_ui_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,10 @@ def test_04_public_survey_with_triggers(self):
}), (0, 0, {
'value': 'Answer 2',
'sequence': 2,
}),
}), (0, 0, {
'value': 'Answer 3',
'sequence': 3,
})
],
'constr_mandatory': True,
}), (0, 0, {
Expand Down Expand Up @@ -226,22 +229,37 @@ def test_04_public_survey_with_triggers(self):
],
'is_conditional': True,
'constr_mandatory': True,
}),
}), (0, 0, {
'title': 'Q4',
'sequence': 4,
'question_type': 'numerical_box',
'is_conditional': True,
'constr_mandatory': True,
}), (0, 0, {
'title': 'Q5',
'sequence': 5,
'question_type': 'numerical_box',
'is_conditional': True,
})
]
})

q1 = survey_with_triggers.question_ids.filtered(lambda q: q.title == 'Q1')
q1_a1 = q1.suggested_answer_ids.filtered(lambda a: a.value == 'Answer 1')
q2 = survey_with_triggers.question_ids.filtered(lambda q: q.title == 'Q2')
q2_a1 = q2.suggested_answer_ids.filtered(lambda a: a.value == 'Answer 1')
q3 = survey_with_triggers.question_ids.filtered(lambda q: q.title == 'Q3')
q1, q2, q3, q4, q5 = survey_with_triggers.question_and_page_ids
q1_a1, q1_a2, __ = q1.suggested_answer_ids
q2_a1 = q2.suggested_answer_ids[0]

q2.triggering_question_id = q1
q2.triggering_answer_id = q1_a1

q3.triggering_question_id = q2
q3.triggering_answer_id = q2_a1

q4.triggering_question_id = q1
q4.triggering_answer_id = q1_a2

q5.triggering_question_id = q1
q5.triggering_answer_id = q1_a2

access_token = survey_with_triggers.access_token
self.start_tour("/survey/start/%s" % access_token, 'test_survey_chained_conditional_questions')

Expand Down

0 comments on commit 34b6a7a

Please sign in to comment.