From d1688b6ee38cf40f3f8c8ddf2ec649e5c2b3dbc0 Mon Sep 17 00:00:00 2001 From: abd-msyukyu-odoo Date: Fri, 8 Dec 2023 13:49:17 +0000 Subject: [PATCH] [FIX] survey: prevent soft lock with conditional questions 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 Note: The issue was already fixed from 17.0 in this [commit], but the test is still adjusted to ensure that displaying/hiding multiple questions at once depending on conditional triggers works as intended without preventing the user to finalize the survey. [commit]: https://github.com/odoo/odoo/commit/55fa52be8a80e6f7ee2a526db29c0d97c24ae8f6 task-3630079 closes odoo/odoo#147008 X-original-commit: f574ec49b730917e8e781a9b47895048a389d585 Signed-off-by: Florian Charlier (flch) Co-authored-by: Salvo Rapisarda Co-authored-by: Damien Abeloos --- .../survey_chained_conditional_questions.js | 17 +++++++++++++---- .../survey/tests/test_survey_ui_feedback.py | 19 ++++++++++--------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/addons/survey/static/tests/tours/survey_chained_conditional_questions.js b/addons/survey/static/tests/tours/survey_chained_conditional_questions.js index c1358c4f1cd..84dece220b5 100644 --- a/addons/survey/static/tests/tours/survey_chained_conditional_questions.js +++ b/addons/survey/static/tests/tours/survey_chained_conditional_questions.js @@ -16,16 +16,20 @@ registry.category("web_tour.tours").add('test_survey_chained_conditional_questio }, { content: 'Answer Q2 with Answer 1', trigger: 'div.js_question-wrapper:contains("Q2") label:contains("Answer 1")', + extra_trigger: 'div.js_question-wrapper:contains("Q4")', }, { content: 'Answer Q3 with Answer 1', trigger: 'div.js_question-wrapper:contains("Q3") label:contains("Answer 1")', }, { - content: 'Answer Q1 with Answer 3', // This should hide Q2 but not Q3. + content: 'Answer Q1 with Answer 3', // This should hide Q2 and Q4 but not Q3. trigger: 'div.js_question-wrapper:contains("Q1") label:contains("Answer 3")', }, { content: 'Check that Q2 was hidden', trigger: 'div.js_question-wrapper:contains("Q3")', - run : () => expectHiddenQuestion("Q2"), + run : () => { + expectHiddenQuestion("Q2"); + expectHiddenQuestion("Q4"); + }, }, { content: 'Answer Q3 with Answer 2', trigger: 'div.js_question-wrapper:contains("Q3") label:contains("Answer 2")', @@ -38,14 +42,18 @@ registry.category("web_tour.tours").add('test_survey_chained_conditional_questio run : () => { expectHiddenQuestion("Q2", "Q2's trigger is gone."); expectHiddenQuestion("Q3", "No reason to show it now."); + expectHiddenQuestion("Q4", "No reason to show it now."); }, }, { content: 'Answer Q1 with Answer 3', // This shows Q3. trigger: 'div.js_question-wrapper:contains("Q1") label:contains("Answer 3")', }, { - content: 'Check that a question (Q2) is hidden', + content: 'Check that questions Q2 and Q4 are hidden', trigger: 'div.js_question-wrapper:contains("Q1")', - run : () => expectHiddenQuestion("Q2", "Q2 should stay hidden."), + run : () => { + expectHiddenQuestion("Q2", "Q2 should stay hidden."); + expectHiddenQuestion("Q4", "Q4 should stay hidden."); + }, }, { content: 'Answer Q3 with Answer 2', trigger: 'div.js_question-wrapper:contains("Q3") label:contains("Answer 2")', @@ -58,6 +66,7 @@ registry.category("web_tour.tours").add('test_survey_chained_conditional_questio run : () => { expectHiddenQuestion("Q2", "Q2's trigger is gone, again."); expectHiddenQuestion("Q3", "As Q2's gone, so should this one."); + expectHiddenQuestion("Q4", "No reason to show it now."); }, }, { content: 'Click Submit and finish the survey', diff --git a/addons/survey/tests/test_survey_ui_feedback.py b/addons/survey/tests/test_survey_ui_feedback.py index 32eeffa7485..b3320ea3950 100644 --- a/addons/survey/tests/test_survey_ui_feedback.py +++ b/addons/survey/tests/test_survey_ui_feedback.py @@ -209,21 +209,22 @@ class TestUiFeedback(HttpCaseWithUserDemo): Command.create({'value': 'Answer 2'}), ], 'constr_mandatory': True, - }), + }), Command.create({ + 'title': 'Q4', + 'sequence': 4, + 'question_type': 'numerical_box', + 'constr_mandatory': 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') - q1_a3 = q1.suggested_answer_ids.filtered(lambda a: a.value == 'Answer 3') - - 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 = survey_with_triggers.question_and_page_ids + q1_a1, __, q1_a3 = q1.suggested_answer_ids + q2_a1 = q2.suggested_answer_ids[0] q2.triggering_answer_ids = q1_a1 q3.triggering_answer_ids = q1_a3 | q2_a1 + q4.triggering_answer_ids = q1_a1 access_token = survey_with_triggers.access_token self.start_tour("/survey/start/%s" % access_token, 'test_survey_chained_conditional_questions')