From 55fa52be8a80e6f7ee2a526db29c0d97c24ae8f6 Mon Sep 17 00:00:00 2001 From: Florian Charlier Date: Mon, 16 Jan 2023 11:16:41 +0000 Subject: [PATCH] [IMP] survey: enable multiple trigger questions and answers Purpose: allowing users to select multiple answers, even from different questions, as triggers to display a subsequent question. For example, we could ask the question "What qualities do you look for in a desk?" if the participant selected one of the following answers before: "What furniture did you already buy from us?" - "A desk" "What kind of furniture are you looking for?" - "Office furniture" Demo data and tests are adapted and new ones are added. We also take this opportunity to remove `is_conditional` because: 1. This field isn't useful anymore. 2. It could cause inconsistencies as it is not supported to check with a sql constraint that `suggested_answer_ids` is set when this flag is `True`. Task-2937533 Part-of: odoo/odoo#109903 Co-authored-by: Pratik Raval --- addons/survey/__manifest__.py | 2 +- addons/survey/controllers/main.py | 12 +- .../survey/data/survey_demo_conditional.xml | 156 +++++++--- addons/survey/models/survey_question.py | 140 ++++----- addons/survey/models/survey_survey.py | 92 +++--- addons/survey/models/survey_user_input.py | 24 +- addons/survey/static/src/js/survey_form.js | 201 +++++++------ .../question_page_list_renderer.js | 2 +- .../survey_question_trigger.js | 86 +++--- .../survey_question_trigger.xml | 2 +- .../survey_question_trigger_widget_tests.js | 49 ++-- .../survey_chained_conditional_questions.js | 49 +++- ...conditional_questions_on_different_page.js | 52 ++++ .../survey/static/tests/tours/survey_form.js | 157 +++++----- addons/survey/tests/test_survey.py | 274 +++++++++--------- .../tests/test_survey_flow_with_conditions.py | 29 +- .../survey/tests/test_survey_ui_feedback.py | 111 ++++--- addons/survey/views/survey_question_views.xml | 22 +- addons/survey/views/survey_survey_views.xml | 7 +- addons/survey/views/survey_templates.xml | 6 +- 20 files changed, 847 insertions(+), 626 deletions(-) create mode 100644 addons/survey/static/tests/tours/survey_conditional_questions_on_different_page.js diff --git a/addons/survey/__manifest__.py b/addons/survey/__manifest__.py index 7363d5bf0c6..516e5c702e6 100644 --- a/addons/survey/__manifest__.py +++ b/addons/survey/__manifest__.py @@ -2,7 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. { 'name': 'Surveys', - 'version': '3.5', + 'version': '3.6', 'category': 'Marketing/Surveys', 'description': """ Create beautiful surveys and visualize answers diff --git a/addons/survey/controllers/main.py b/addons/survey/controllers/main.py index 9c87101b040..e46485a0d50 100644 --- a/addons/survey/controllers/main.py +++ b/addons/survey/controllers/main.py @@ -270,15 +270,15 @@ class Survey(http.Controller): 'format_date': lambda date: format_date(request.env, date) } if survey_sudo.questions_layout != 'page_per_question': - triggering_answer_by_question, triggered_questions_by_answer, selected_answers = answer_sudo._get_conditional_values() + triggering_answers_by_question, triggered_questions_by_answer, selected_answers = answer_sudo._get_conditional_values() data.update({ - 'triggering_answer_by_question': { - question.id: triggering_answer_by_question[question].id for question in triggering_answer_by_question.keys() - if triggering_answer_by_question[question] + 'triggering_answers_by_question': { + question.id: triggering_answers.ids + for question, triggering_answers in triggering_answers_by_question.items() if triggering_answers }, 'triggered_questions_by_answer': { - answer.id: triggered_questions_by_answer[answer].ids - for answer in triggered_questions_by_answer.keys() + answer.id: triggered_questions.ids + for answer, triggered_questions in triggered_questions_by_answer.items() }, 'selected_answers': selected_answers.ids }) diff --git a/addons/survey/data/survey_demo_conditional.xml b/addons/survey/data/survey_demo_conditional.xml index 5e02aedef5d..ce3b0d7f408 100644 --- a/addons/survey/data/survey_demo_conditional.xml +++ b/addons/survey/data/survey_demo_conditional.xml @@ -67,9 +67,7 @@ How long is the White Nile river? simple_choice - - - + @@ -96,9 +94,7 @@ What is the biggest city in the world? simple_choice - - - + @@ -129,9 +125,7 @@ Which is the highest volcano in Europe? simple_choice - - - + @@ -170,9 +164,7 @@ When did Genghis Khan die? simple_choice - - - + @@ -198,9 +190,7 @@ Who is the architect of the Great Pyramid of Giza? simple_choice - - - + @@ -231,9 +221,7 @@ How many years did the 100 years war last? simple_choice - - - + @@ -272,9 +260,7 @@ Who received a Nobel prize in Physics for the discovery of neutrino oscillations, which shows that neutrinos have mass? multiple_choice - - - + @@ -307,9 +293,7 @@ What is, approximately, the critical mass of plutonium-239? simple_choice - - - + @@ -340,9 +324,7 @@ Can Humans ever directly see a photon? simple_choice - - - + @@ -354,7 +336,7 @@ 2 - No, it's to small for the human eye. + No, it's too small for the human eye. @@ -371,9 +353,7 @@ Which Musician is not in the 27th Club? multiple_choice - - - + @@ -406,9 +386,7 @@ Which painting/drawing was not made by Pablo Picasso? simple_choice - - - + @@ -443,9 +421,7 @@ Which quote is from Jean-Claude Van Damme simple_choice - - - + @@ -470,4 +446,110 @@ I actually don't like thinking. I think people think I like to think a lot. And I don't. I do not like to think at all. + + + Food Preferences + survey + foodpref-eren-ces1-abcd-344ca2tgb31e + + public + one_page + +

Please give us your preferences for this event's dinner!

+ + +

Got it!

+

See you soon!

+ + + + + + 1 + Are you vegetarian? + simple_choice + + + + + 1 + Yes + + + + 2 + No + + + + 3 + It depends + + + + + 2 + Would you prefer a veggie meal if possible? + simple_choice + + + + + + + 1 + Yes + + + + 2 + No + + + + + 3 + Choose your green meal + simple_choice + + + + + + 1 + Vegetarian pizza + + + + 2 + Vegetarian burger + + + + + 4 + Choose your meal + simple_choice + + + + + + 1 + Steak with french fries + + + + 2 + Fish + + diff --git a/addons/survey/models/survey_question.py b/addons/survey/models/survey_question.py index 3c07a249902..2e11eeca1be 100644 --- a/addons/survey/models/survey_question.py +++ b/addons/survey/models/survey_question.py @@ -3,9 +3,10 @@ import collections import contextlib -import json import itertools +import json import operator +from textwrap import shorten from odoo import api, fields, models, tools, _ from odoo.exceptions import UserError, ValidationError @@ -132,27 +133,28 @@ class SurveyQuestion(models.Model): 'survey.user_input.line', 'question_id', string='Answers', domain=[('skipped', '=', False)], groups='survey.group_survey_user') - # Conditional display - is_conditional = fields.Boolean( - string='Conditional Display', copy=False, help="""If checked, this question will be displayed only - if the specified conditional answer have been selected in a previous question""") - triggering_question_id = fields.Many2one( - 'survey.question', string="Triggering Question", copy=False, compute="_compute_triggering_question_id", - store=True, readonly=False, help="Question containing the triggering answer to display the current question.", - domain="[('survey_id', '=', survey_id), \ - '&', ('question_type', 'in', ['simple_choice', 'multiple_choice']), \ - '|', \ - ('sequence', '<', sequence), \ - '&', ('sequence', '=', sequence), ('id', '<', id)]") + # Not stored, convenient for trigger display computation. + triggering_question_ids = fields.Many2many( + 'survey.question', string="Triggering Questions", compute="_compute_triggering_question_ids", + store=False, help="Questions containing the triggering answer(s) to display the current question.") + allowed_triggering_question_ids = fields.Many2many( 'survey.question', string="Allowed Triggering Questions", copy=False, compute="_compute_allowed_triggering_question_ids") is_placed_before_trigger = fields.Boolean( - string='Is misplaced?', help="Is this question placed before its trigger question?", + string='Is misplaced?', help="Is this question placed before any of its trigger questions?", compute="_compute_allowed_triggering_question_ids") - triggering_answer_id = fields.Many2one( - 'survey.question.answer', string="Triggering Answer", copy=False, compute="_compute_triggering_answer_id", - store=True, readonly=False, help="Answer that will trigger the display of the current question.", - domain="[('question_id', '=', triggering_question_id)]") + triggering_answer_ids = fields.Many2many( + 'survey.question.answer', string="Triggering Answers", copy=False, store=True, + readonly=False, help="Picking any of these answers will trigger this question.\n" + "Leave the field empty if the question should always be displayed.", + domain="""[ + ('question_id.survey_id', '=', survey_id), + '&', ('question_id.question_type', 'in', ['simple_choice', 'multiple_choice']), + '|', + ('question_id.sequence', '<', sequence), + '&', ('question_id.sequence', '=', sequence), ('question_id.id', '<', id) + ]""" + ) _sql_constraints = [ ('positive_len_min', 'CHECK (validation_length_min >= 0)', 'A length must be positive!'), @@ -166,12 +168,6 @@ class SurveyQuestion(models.Model): 'All "Is a scored question = True" and "Question Type: Datetime" questions need an answer'), ('scored_date_have_answers', "CHECK (is_scored_question != True OR question_type != 'date' OR answer_date is not null)", 'All "Is a scored question = True" and "Question Type: Date" questions need an answer'), - ('conditional_questions_have_triggering_question', 'CHECK (is_conditional != True OR triggering_question_id is not null)', - 'All conditional display questions need a triggering question.\n' - 'Please disable "Conditional Display" or specify a triggering question.'), - ('triggered_questions_have_triggering_answer', 'CHECK (triggering_question_id is null OR triggering_answer_id is not null)', - 'All questions triggered by another need a triggering answer.\n' - 'Please disable "Conditional Display" or specify a triggering answer.'), ] # ------------------------------------------------------------------------- @@ -280,19 +276,12 @@ class SurveyQuestion(models.Model): if not question.validation_required or question.question_type not in ['char_box', 'numerical_box', 'date', 'datetime']: question.validation_required = False - @api.depends('is_conditional', 'survey_id', 'survey_id.question_ids', 'triggering_question_id') + @api.depends('survey_id', 'survey_id.question_ids', 'triggering_answer_ids') def _compute_allowed_triggering_question_ids(self): - """ Although the question (and possible trigger questions) sequence + """Although the question (and possible trigger questions) sequence is used here, we do not add these fields to the dependency list to avoid cascading rpc calls when reordering questions via the webclient. """ - conditional_questions = self.filtered(lambda q: q.is_conditional) - non_conditional_questions = self - conditional_questions - non_conditional_questions.allowed_triggering_question_ids = False - non_conditional_questions.is_placed_before_trigger = False - if not conditional_questions: - return - possible_trigger_questions = self.search([ ('is_page', '=', False), ('question_type', 'in', ['simple_choice', 'multiple_choice']), @@ -301,14 +290,14 @@ class SurveyQuestion(models.Model): ]) # Using the sequence stored in db is necessary for existing questions that are passed as # NewIds because the sequence provided by the JS client can be incorrect. - (conditional_questions | possible_trigger_questions).flush_recordset() + (self | possible_trigger_questions).flush_recordset() self.env.cr.execute( "SELECT id, sequence FROM survey_question WHERE id =ANY(%s)", - [conditional_questions.ids] + [self.ids] ) conditional_questions_sequences = dict(self.env.cr.fetchall()) # id: sequence mapping - for question in conditional_questions: + for question in self: question_id = question._origin.id if not question_id: # New question question.allowed_triggering_question_ids = possible_trigger_questions.filtered( @@ -322,28 +311,15 @@ class SurveyQuestion(models.Model): lambda q: q.survey_id.id == question.survey_id._origin.id and (q.sequence < question_sequence or q.sequence == question_sequence and q.id < question_id) ) - question.is_placed_before_trigger = ( - question.triggering_question_id - and question.triggering_question_id.id not in question.allowed_triggering_question_ids.ids) + question.is_placed_before_trigger = bool( + set(question.triggering_answer_ids.question_id.ids) + - set(question.allowed_triggering_question_ids.ids) # .ids necessary to match ids with newIds + ) - @api.depends('is_conditional') - def _compute_triggering_question_id(self): - """ Used as an 'onchange' : Reset the triggering question if user uncheck 'Conditional Display' - Avoid CacheMiss : set the value to False if the value is not set yet.""" + @api.depends('triggering_answer_ids') + def _compute_triggering_question_ids(self): for question in self: - if not question.is_conditional or question.triggering_question_id is None: - question.triggering_question_id = False - - @api.depends('triggering_question_id') - def _compute_triggering_answer_id(self): - """ Used as an 'onchange' : Reset the triggering answer if user unset or change the triggering question - or uncheck 'Conditional Display'. - Avoid CacheMiss : set the value to False if the value is not set yet.""" - for question in self: - if not question.triggering_question_id \ - or question.triggering_question_id != question.triggering_answer_id.question_id\ - or question.triggering_answer_id is None: - question.triggering_answer_id = False + question.triggering_question_ids = question.triggering_answer_ids.question_id @api.depends('question_type', 'scoring_type', 'answer_date', 'answer_datetime', 'answer_numerical_box', 'suggested_answer_ids.is_correct') def _compute_is_scored_question(self): @@ -391,22 +367,10 @@ class SurveyQuestion(models.Model): def copy(self, default=None): self.ensure_one() clone = super().copy(default) - if self.is_conditional: - clone.is_conditional = True - clone.triggering_question_id = self.triggering_question_id.id - clone.triggering_answer_id = self.triggering_answer_id.id + if self.triggering_answer_ids: + clone.triggering_answer_ids = self.triggering_answer_ids return clone - def unlink(self): - """ Makes sure no question is left depending on the question we're deleting.""" - depending_questions = self.env['survey.question'].search([('triggering_question_id', 'in', self.ids)]) - depending_questions.write({ - 'is_conditional': False, - 'triggering_question_id': False, - 'triggering_answer_id': False, - }) - return super().unlink() - # ------------------------------------------------------------ # CRUD # ------------------------------------------------------------ @@ -698,9 +662,12 @@ class SurveyQuestionAnswer(models.Model): """ _name = 'survey.question.answer' _rec_name = 'value' - _order = 'sequence, id' + _rec_names_search = ['question_id.title', 'value'] + _order = 'question_id, sequence, id' _description = 'Survey Label' + MAX_ANSWER_NAME_LENGTH = 90 # empirically tested in client dropdown + # question and question related fields question_id = fields.Many2one('survey.question', string='Question', ondelete='cascade') matrix_question_id = fields.Many2one('survey.question', string='Question (as matrix row)', ondelete='cascade') @@ -714,6 +681,27 @@ class SurveyQuestionAnswer(models.Model): is_correct = fields.Boolean('Correct') answer_score = fields.Float('Score', help="A positive score indicates a correct choice; a negative or null score indicates a wrong answer") + @api.depends('value', 'question_id.title') + def _compute_display_name(self): + """Render an answer name as "Question title : Answer value" making sure it is not too long. + + This implementation makes sure we have at least 30 characters for the question title, + then we elide it, leaving the rest of the space for the answer. + """ + for answer in self: + # _origin (or fallback title) is (likely temporarily) needed to support survey snapshot + # during onchange for a deleted answer used as trigger in another question. + title = answer._origin.question_id.title + n_extra_characters = len(title) + len(answer.value) + 3 - self.MAX_ANSWER_NAME_LENGTH # 3 for `" : "` + if n_extra_characters <= 0: + answer.display_name = f'{title} : {answer.value}' + else: + answer.display_name = shorten( + f'{shorten(title, max(30, len(title) - n_extra_characters), placeholder="...")} : {answer.value}', + self.MAX_ANSWER_NAME_LENGTH, + placeholder="..." + ) + @api.constrains('question_id', 'matrix_question_id') def _check_question_not_empty(self): """Ensure that field question_id XOR field matrix_question_id is not null""" @@ -728,13 +716,3 @@ class SurveyQuestionAnswer(models.Model): elif self.question_type in ('multiple_choice', 'simple_choice'): return ['&', ('question_id', '=', self.question_id.id), ('suggested_answer_id', '=', self.id)] return [] - - def unlink(self): - """ Makes sure no question is left depending on the answer we're deleting.""" - depending_questions = self.env['survey.question'].search([('triggering_answer_id', 'in', self.ids)]) - depending_questions.write({ - 'is_conditional': False, - 'triggering_question_id': False, - 'triggering_answer_id': False, - }) - return super().unlink() diff --git a/addons/survey/models/survey_survey.py b/addons/survey/models/survey_survey.py index 4ac7d2d93fd..efda3b0e78d 100644 --- a/addons/survey/models/survey_survey.py +++ b/addons/survey/models/survey_survey.py @@ -4,6 +4,8 @@ import json import random import uuid +from collections import defaultdict + import werkzeug from odoo import api, exceptions, fields, models, _ @@ -253,12 +255,12 @@ class Survey(models.Model): survey.question_ids = survey.question_and_page_ids - survey.page_ids survey.question_count = len(survey.question_ids) - @api.depends('question_and_page_ids.is_conditional', 'users_login_required', 'access_mode') + @api.depends('question_and_page_ids.triggering_answer_ids', 'users_login_required', 'access_mode') def _compute_is_attempts_limited(self): for survey in self: if not survey.is_attempts_limited or \ (survey.access_mode == 'public' and not survey.users_login_required) or \ - any(question.is_conditional for question in survey.question_and_page_ids): + any(question.triggering_answer_ids for question in survey.question_and_page_ids): survey.is_attempts_limited = False @api.depends('session_start_time', 'user_input_ids') @@ -310,10 +312,10 @@ class Survey(models.Model): survey.session_show_leaderboard = survey.scoring_type != 'no_scoring' and \ any(question.save_as_nickname for question in survey.question_and_page_ids) - @api.depends('question_and_page_ids.is_conditional') + @api.depends('question_and_page_ids.triggering_answer_ids') def _compute_has_conditional_questions(self): for survey in self: - survey.has_conditional_questions = any(question.is_conditional for question in survey.question_and_page_ids) + survey.has_conditional_questions = any(question.triggering_answer_ids for question in survey.question_and_page_ids) @api.depends('scoring_type') def _compute_certification(self): @@ -381,14 +383,14 @@ class Survey(models.Model): @api.returns('self', lambda value: value.id) def copy(self, default=None): - """ Correctly copy the 'triggering_question_id' and 'triggering_answer_id' fields from the original - to the clone. - This needs to be done in post-processing to make sure we get references to the newly created - answers/questions from the copy instead of references to the answers/questions of the original. - This implementation assumes that the order of created questions/answers will be kept between + """Correctly copy the 'triggering_answer_ids' field from the original to the clone. + + This needs to be done in post-processing to make sure we get references to the newly + created answers from the copy instead of references to the answers of the original. + This implementation assumes that the order of created answers will be kept between the original and the clone, using 'zip()' to match the records between the two. - Note that when question_ids is provided in the default parameter, it falls back to the + Note that when `question_ids` is provided in the default parameter, it falls back to the standard copy, meaning that triggering logic will not be maintained. """ self.ensure_one() @@ -396,23 +398,18 @@ class Survey(models.Model): if default and 'question_ids' in default: return clone - src_questions = self.question_ids - dst_questions = clone.question_ids.sorted() + cloned_question_ids = clone.question_ids.sorted() - questions_map = {src.id: dst.id for src, dst in zip(src_questions, dst_questions)} answers_map = { src_answer.id: dst_answer.id for src, dst - in zip(src_questions, dst_questions) + in zip(self.question_ids, cloned_question_ids) for src_answer, dst_answer in zip(src.suggested_answer_ids, dst.suggested_answer_ids.sorted()) } - - for src, dst in zip(src_questions, dst_questions): - if src.is_conditional: - dst.is_conditional = True - dst.triggering_question_id = questions_map.get(src.triggering_question_id.id) - dst.triggering_answer_id = answers_map.get(src.triggering_answer_id.id) + for src, dst in zip(self.question_ids, cloned_question_ids): + if src.triggering_answer_ids: + dst.triggering_answer_ids = [answers_map[src_answer_id.id] for src_answer_id in src.triggering_answer_ids] return clone def copy_data(self, default=None): @@ -615,22 +612,31 @@ class Survey(models.Model): return result def _get_pages_and_questions_to_show(self): - """ - :return: survey.question recordset excluding invalid conditional questions and pages without description - """ + """Filter question_and_pages_ids to include only valid pages and questions. + Pages are invalid if they have no description. Questions are invalid if + they are conditional and all their triggers are invalid. + Triggers are invalid if they: + - Are a page (not a question) + - Have the wrong question type (`simple_choice` and `multiple_choice` are supported) + - Are misplaced (positioned after the conditional question) + - They are themselves conditional and were found invalid + """ self.ensure_one() invalid_questions = self.env['survey.question'] questions_and_valid_pages = self.question_and_page_ids.filtered( lambda question: not question.is_page or not is_html_empty(question.description)) - for question in questions_and_valid_pages.filtered(lambda q: q.is_conditional).sorted(): - trigger = question.triggering_question_id - if (trigger in invalid_questions - or trigger.is_page - or trigger.question_type not in ['simple_choice', 'multiple_choice'] - or not trigger.suggested_answer_ids - or trigger.sequence > question.sequence - or (trigger.sequence == question.sequence and trigger.id > question.id)): + + for question in questions_and_valid_pages.filtered(lambda q: q.triggering_answer_ids).sorted(): + for trigger in question.triggering_question_ids: + if (trigger not in invalid_questions + and not trigger.is_page + and trigger.question_type in ['simple_choice', 'multiple_choice'] + and (trigger.sequence < question.sequence + or (trigger.sequence == question.sequence and trigger.id < question.id))): + break + else: + # No valid trigger found invalid_questions |= question return questions_and_valid_pages - invalid_questions @@ -674,7 +680,7 @@ class Survey(models.Model): return Question # Conditional Questions Management - triggering_answer_by_question, triggered_questions_by_answer, selected_answers = user_input._get_conditional_values() + triggering_answers_by_question, _, selected_answers = user_input._get_conditional_values() inactive_questions = user_input._get_inactive_conditional_questions() if survey.questions_layout == 'page_per_question': question_candidates = pages_or_questions[0:current_page_index] if go_back \ @@ -687,8 +693,8 @@ class Survey(models.Model): if contains_active_question or is_description_section: return question else: - triggering_answer = triggering_answer_by_question.get(question) - if not triggering_answer or triggering_answer in selected_answers: + triggering_answers = triggering_answers_by_question.get(question) + if not triggering_answers or triggering_answers & selected_answers: # question is visible because not conditioned or conditioned by a selected answer return question elif survey.questions_layout == 'page_per_section': @@ -726,7 +732,7 @@ class Survey(models.Model): next_page_or_question_candidates = pages_or_questions[current_page_index + 1:] if next_page_or_question_candidates: inactive_questions = user_input._get_inactive_conditional_questions() - triggering_answer_by_question, triggered_questions_by_answer, selected_answers = user_input._get_conditional_values() + _, triggered_questions_by_answer, _ = user_input._get_conditional_values() if self.questions_layout == 'page_per_question': next_active_question = any(next_question not in inactive_questions for next_question in next_page_or_question_candidates) is_triggering_question = any(triggering_answer in triggered_questions_by_answer.keys() for triggering_answer in page_or_question.suggested_answer_ids) @@ -794,17 +800,15 @@ class Survey(models.Model): # ------------------------------------------------------------ def _get_conditional_maps(self): - triggering_answer_by_question = {} - triggered_questions_by_answer = {} + triggering_answers_by_question = defaultdict(lambda: self.env['survey.question.answer']) + triggered_questions_by_answer = defaultdict(lambda: self.env['survey.question']) for question in self.question_ids: - triggering_answer_by_question[question] = question.is_conditional and question.triggering_answer_id + triggering_answers_by_question[question] |= question.triggering_answer_ids - if question.is_conditional: - if question.triggering_answer_id in triggered_questions_by_answer: - triggered_questions_by_answer[question.triggering_answer_id] |= question - else: - triggered_questions_by_answer[question.triggering_answer_id] = question - return triggering_answer_by_question, triggered_questions_by_answer + for triggering_answer_id in question.triggering_answer_ids: + triggered_questions_by_answer[triggering_answer_id] |= question + + return triggering_answers_by_question, triggered_questions_by_answer # ------------------------------------------------------------ # SESSIONS MANAGEMENT diff --git a/addons/survey/models/survey_user_input.py b/addons/survey/models/survey_user_input.py index c61cb9dd119..1fbb480696d 100644 --- a/addons/survey/models/survey_user_input.py +++ b/addons/survey/models/survey_user_input.py @@ -536,20 +536,21 @@ class SurveyUserInput(models.Model): that is the next in sequence and that is either not triggered by another question's answer, or that is triggered by an already selected answer. To do all this, we need to return: - - list of all selected answers: [answer_id1, answer_id2, ...] (for survey reloading, otherwise, this list is - updated at client side) + - triggering_answers_by_question: dict -> for a given question, the answers that triggers it + Used mainly to ease template rendering - triggered_questions_by_answer: dict -> for a given answer, list of questions triggered by this answer; Used mainly for dynamic show/hide behaviour at client side - - triggering_answer_by_question: dict -> for a given question, the answer that triggers it - Used mainly to ease template rendering + - list of all selected answers: [answer_id1, answer_id2, ...] (for survey reloading, otherwise, this list is + updated at client side) """ - triggering_answer_by_question, triggered_questions_by_answer = {}, {} + triggering_answers_by_question = {} + triggered_questions_by_answer = {} # Ignore conditional configuration if randomised questions selection if self.survey_id.questions_selection != 'random': - triggering_answer_by_question, triggered_questions_by_answer = self.survey_id._get_conditional_maps() + triggering_answers_by_question, triggered_questions_by_answer = self.survey_id._get_conditional_maps() selected_answers = self._get_selected_suggested_answers() - return triggering_answer_by_question, triggered_questions_by_answer, selected_answers + return triggering_answers_by_question, triggered_questions_by_answer, selected_answers def _get_selected_suggested_answers(self): """ @@ -585,14 +586,13 @@ class SurveyUserInput(models.Model): answers_to_delete.unlink() def _get_inactive_conditional_questions(self): - triggering_answer_by_question, triggered_questions_by_answer, selected_answers = self._get_conditional_values() + triggering_answers_by_question, _, selected_answers = self._get_conditional_values() # get questions that should not be answered inactive_questions = self.env['survey.question'] - for answer in triggered_questions_by_answer.keys(): - if answer not in selected_answers: - for question in triggered_questions_by_answer[answer]: - inactive_questions |= question + for question, triggering_answers in triggering_answers_by_question.items(): + if triggering_answers and not triggering_answers & selected_answers: + inactive_questions |= question return inactive_questions def _get_print_questions(self): diff --git a/addons/survey/static/src/js/survey_form.js b/addons/survey/static/src/js/survey_form.js index c08a5c43c93..614c3ff0f33 100644 --- a/addons/survey/static/src/js/survey_form.js +++ b/addons/survey/static/src/js/survey_form.js @@ -142,20 +142,43 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa } }, + /** + * Handle visibility of comment area and conditional questions + * The form (page) is then automatically submitted if: + * - Survey is configured with one page per question and participants are allowed to go back, + * - It is not the last question of the survey, + * - The question is not waiting for a comment (with "Other" answer), + * + * @param event + */ + _onChangeChoiceItem: function (event) { + const $target = $(event.currentTarget); + const $choiceItemGroup = $target.closest('.o_survey_form_choice'); + + this._applyCommentAreaVisibility($target); + const isQuestionComplete = this._checkConditionalQuestionsConfiguration($target, $choiceItemGroup); + if (isQuestionComplete && this.options.usersCanGoBack) { + const isLastQuestion = this.$('button[value="finish"]').length !== 0; + if (!isLastQuestion) { + const questionHasComment = $target.hasClass('o_survey_js_form_other_comment') || $target + .closest('.o_survey_form_choice') + .find('.o_survey_comment').length !== 0; + if (!questionHasComment) { + this._submitForm({}); + } + } + } + }, /** * Checks, if the 'other' choice is checked. Applies only if the comment count as answer. * If not checked : Clear the comment textarea, hide and disable it * If checked : enable the comment textarea, show and focus on it * - * @private - * @param {Event} event + * @param {JQuery} $choiceItemGroup */ - _onChangeChoiceItem: function (event) { - var self = this; - var $target = $(event.currentTarget); - var $choiceItemGroup = $target.closest('.o_survey_form_choice'); - var $otherItem = $choiceItemGroup.find('.o_survey_js_form_other_comment'); - var $commentInput = $choiceItemGroup.find('textarea[type="text"]'); + _applyCommentAreaVisibility: function ($choiceItemGroup) { + const $otherItem = $choiceItemGroup.find('.o_survey_js_form_other_comment'); + const $commentInput = $choiceItemGroup.find('textarea[type="text"]'); if ($otherItem.prop('checked') || $commentInput.hasClass('o_survey_comment')) { $commentInput.each((idx, $input) => $input.disabled = false); @@ -168,10 +191,20 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa $commentInput.closest('.o_survey_comment_container').addClass('d-none'); $commentInput.each((idx, $input) => $input.disabled = true); } + }, - var $matrixBtn = $target.closest('.o_survey_matrix_btn'); + /** + * For single and multiple choice questions, propagate questions visibility + * based on conditional questions and (de)selected triggers + * + * @param {JQuery} $target + * @param {JQuery} $choiceItemGroup + * @returns {boolean} Whether the question is considered completed + */ + _checkConditionalQuestionsConfiguration: function ($target, $choiceItemGroup) { + let isQuestionComplete = false; + const $matrixBtn = $target.closest('.o_survey_matrix_btn'); if ($target.attr('type') === 'radio') { - var isQuestionComplete = false; if ($matrixBtn.length > 0) { $matrixBtn.closest('tr').find('td').removeClass('o_survey_selected'); if ($target.is(':checked')) { @@ -181,86 +214,56 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa var subQuestionsIds = $matrixBtn.closest('table').data('subQuestions'); var completedQuestions = []; subQuestionsIds.forEach(function (id) { - if (self.$('tr#' + id).find('input:checked').length !== 0) { + if (this.$('tr#' + id).find('input:checked').length !== 0) { completedQuestions.push(id); } }); isQuestionComplete = completedQuestions.length === subQuestionsIds.length; } } else { - var previouslySelectedAnswer = $choiceItemGroup.find('label.o_survey_selected'); + const previouslySelectedAnswer = $choiceItemGroup.find('label.o_survey_selected'); previouslySelectedAnswer.removeClass('o_survey_selected'); + const previouslySelectedAnswerId = previouslySelectedAnswer.find('input').val(); + if (previouslySelectedAnswerId && this.options.questionsLayout !== 'page_per_question') { + this.selectedAnswers.splice(this.selectedAnswers.indexOf(parseInt(previouslySelectedAnswerId)), 1); + } - var newlySelectedAnswer = $target.closest('label'); - if (newlySelectedAnswer.find('input').val() !== previouslySelectedAnswer.find('input').val()) { + const newlySelectedAnswer = $target.closest('label'); + const newlySelectedAnswerId = $target.val(); + const isNewSelection = newlySelectedAnswerId !== previouslySelectedAnswerId; + if (isNewSelection) { newlySelectedAnswer.addClass('o_survey_selected'); isQuestionComplete = this.options.questionsLayout === 'page_per_question'; + if (!isQuestionComplete) { + this.selectedAnswers.push(parseInt(newlySelectedAnswerId)); + } } - // Conditional display if (this.options.questionsLayout !== 'page_per_question') { - var treatedQuestionIds = []; // Needed to avoid show (1st 'if') then immediately hide (2nd 'if') question during conditional propagation cascade - if (Object.keys(this.options.triggeredQuestionsByAnswer).includes(previouslySelectedAnswer.find('input').val())) { - // Hide and clear depending question - this.options.triggeredQuestionsByAnswer[previouslySelectedAnswer.find('input').val()].forEach(function (questionId) { - var dependingQuestion = $('.js_question-wrapper#' + questionId); - - dependingQuestion.addClass('d-none'); - self._clearQuestionInputs(dependingQuestion); - - treatedQuestionIds.push(questionId); - }); - // Remove answer from selected answer - self.selectedAnswers.splice(self.selectedAnswers.indexOf(parseInt($target.val())), 1); - } - if (Object.keys(this.options.triggeredQuestionsByAnswer).includes($target.val())) { - // Display depending question - 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())); - } - }); - } + const conditionalQuestionsToRecomputeVisibility = new Set( + (this.options.triggeredQuestionsByAnswer[previouslySelectedAnswerId] || []) + .concat(this.options.triggeredQuestionsByAnswer[newlySelectedAnswerId] || []) + ) + this._applyConditionalQuestionsVisibility(conditionalQuestionsToRecomputeVisibility) } } - // Auto Submit Form - var isLastQuestion = this.$('button[value="finish"]').length !== 0; - var questionHasComment = $target.closest('.o_survey_form_choice').find('.o_survey_comment').length !== 0 - || $target.hasClass('o_survey_js_form_other_comment'); - if (!isLastQuestion && this.options.usersCanGoBack && isQuestionComplete && !questionHasComment) { - this._submitForm({}); - } } else { // $target.attr('type') === 'checkbox' if ($matrixBtn.length > 0) { $matrixBtn.toggleClass('o_survey_selected', !$matrixBtn.hasClass('o_survey_selected')); } else { - var $label = $target.closest('label'); + const $label = $target.closest('label'); $label.toggleClass('o_survey_selected', !$label.hasClass('o_survey_selected')); + const answerId = $target.val(); - // Conditional display - if (this.options.questionsLayout !== 'page_per_question' && Object.keys(this.options.triggeredQuestionsByAnswer).includes($target.val())) { - var isInputSelected = $label.hasClass('o_survey_selected'); - // Hide and clear or display depending question - this.options.triggeredQuestionsByAnswer[$target.val()].forEach(function (questionId) { - var dependingQuestion = $('.js_question-wrapper#' + questionId); - dependingQuestion.toggleClass('d-none', !isInputSelected); - if (!isInputSelected) { - self._clearQuestionInputs(dependingQuestion); - } - }); - // Add/remove answer to/from selected answer - if (!isInputSelected) { - self.selectedAnswers.splice(self.selectedAnswers.indexOf(parseInt($target.val())), 1); - } else { - self.selectedAnswers.push(parseInt($target.val())); - } + if (this.options.questionsLayout !== 'page_per_question') { + $label.hasClass('o_survey_selected') + ? this.selectedAnswers.push(parseInt(answerId)) + : this.selectedAnswers.splice(this.selectedAnswers.indexOf(parseInt(answerId)), 1); + this._applyConditionalQuestionsVisibility(this.options.triggeredQuestionsByAnswer[answerId]); } } } + return isQuestionComplete; }, /** @@ -534,7 +537,7 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa * * @param {Object} options see '_submitForm' for details */ - _onNextScreenDone: function (options) { + _onNextScreenDone: function (options) { var self = this; var result = this.nextScreenResult; @@ -1134,23 +1137,6 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa // CONDITIONAL QUESTIONS MANAGEMENT TOOLS // ------------------------------------------------------------------------- - /** - * Clear / Un-select all the input from the given question - * + propagate conditional hierarchy by triggering change on choice inputs. - * - * @private - */ - _clearQuestionInputs: function (question) { - question.find('input').each(function () { - if ($(this).attr('type') === 'text' || $(this).attr('type') === 'number') { - $(this).val(''); - } else if ($(this).prop('checked')) { - $(this).prop('checked', false).change(); - } - }); - question.find('textarea').val(''); - }, - /** * Get questions that are not supposed to be answered by the user. * Those are the ones triggered by answers that the user did not selected. @@ -1158,20 +1144,47 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa * @private */ _getInactiveConditionalQuestionIds: function () { - var self = this; - var inactiveQuestionIds = []; - if (this.options.triggeredQuestionsByAnswer) { - Object.keys(this.options.triggeredQuestionsByAnswer).forEach(function (answerId) { - if (!self.selectedAnswers.includes(parseInt(answerId))) { - self.options.triggeredQuestionsByAnswer[answerId].forEach(function (questionId) { - inactiveQuestionIds.push(questionId); - }); - } - }); + const inactiveQuestionIds = []; + for (const [questionId, answerIds] of Object.entries(this.options.triggeringAnswersByQuestion || {})) { + if (!answerIds.some(answerId => this.selectedAnswers.includes(parseInt(answerId)))) { + inactiveQuestionIds.push(parseInt(questionId)); + } } return inactiveQuestionIds; }, + /** + * Apply visibility rules of conditional questions. + * + * @param {Number[] | String[] | Set | undefined} questionIds Conditional questions ids + */ + _applyConditionalQuestionsVisibility: function(questionIds) { + if (!questionIds || (!questionIds.length && !questionIds.size)) { + return; + } + for (const questionId of questionIds) { + const dependingQuestion = document.querySelector(`.js_question-wrapper[id="${questionId}"]`); + if (!dependingQuestion) { // Could be on different page + continue; + } + const hasNoSelectedTriggers = !this.options.triggeringAnswersByQuestion[questionId] + .some(answerId => this.selectedAnswers.includes(parseInt(answerId))); + dependingQuestion.classList.toggle('d-none', hasNoSelectedTriggers); + if (hasNoSelectedTriggers) { + // Clear / Un-select all the input from the given question + // + propagate conditional hierarchy by triggering change on choice inputs. + $(dependingQuestion).find('input').each(function () { + if ($(this).attr('type') === 'text' || $(this).attr('type') === 'number') { + $(this).val(''); + } else if ($(this).prop('checked')) { + $(this).prop('checked', false).change(); + } + }); + $(dependingQuestion).find('textarea').val(''); + } + } + }, + // ERRORS TOOLS // ------------------------------------------------------------------------- diff --git a/addons/survey/static/src/question_page/question_page_list_renderer.js b/addons/survey/static/src/question_page/question_page_list_renderer.js index 926d6ce456d..25e19fe5b0c 100644 --- a/addons/survey/static/src/question_page/question_page_list_renderer.js +++ b/addons/survey/static/src/question_page/question_page_list_renderer.js @@ -134,7 +134,7 @@ export class QuestionPageListRenderer extends ListRenderer { */ async onDeleteRecord(record) { const triggeredRecords = this.props.list.records.filter( - (rec) => rec.data.triggering_question_id[0] === record.resId + (rec) => rec.data.triggering_question_ids.records.map(a => a.resId).includes(record.resId) ); if (triggeredRecords.length) { const res = await super.onDeleteRecord(record); diff --git a/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.js b/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.js index c24c976a2fd..144dc9ec3b9 100644 --- a/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.js +++ b/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.js @@ -7,6 +7,11 @@ import { standardWidgetProps } from "@web/views/widgets/standard_widget_props"; const { Component, useEffect, useRef, useState } = owl; export class SurveyQuestionTriggerWidget extends Component { + static template = "survey.surveyQuestionTrigger"; + static props = { + ...standardWidgetProps, + }; + setup() { super.setup(); this.button = useRef('survey_question_trigger'); @@ -15,19 +20,20 @@ export class SurveyQuestionTriggerWidget extends Component { triggerTooltip: "", }); useEffect(() => { - if (this.button && this.button.el) { - const triggeringQuestionTitle = this.props.record.data.triggering_question_id[1]; - const triggerError = this.surveyQuestionTriggerError; + if (this.button?.el && this.props.record.data.triggering_question_ids.records?.length !== 0) { + const { triggerError, misplacedTriggerQuestionRecords } = this.surveyQuestionTriggerError; if (triggerError === "MISPLACED_TRIGGER_WARNING") { this.state.surveyIconWarning = true; - this.state.triggerTooltip = _t( - '⚠ This question is positioned before its trigger ("%s") and will be skipped.', - triggeringQuestionTitle + this.state.triggerTooltip = '⚠ ' + _t( + 'Triggers based on the following questions will not work because they are positioned after this question:\n"%s".', + misplacedTriggerQuestionRecords + .map((question) => question.data.title) + .join('", "') ); } else if (triggerError === "WRONG_QUESTIONS_SELECTION_WARNING") { this.state.surveyIconWarning = true; - this.state.triggerTooltip = _t( - "⚠ Conditional display is not available when questions are randomly picked." + this.state.triggerTooltip = '⚠ ' + _t( + "Conditional display is not available when questions are randomly picked." ); } else if (triggerError === "MISSING_TRIGGER_ERROR") { // This case must be handled to not temporarily render the "normal" icon if previously @@ -36,9 +42,10 @@ export class SurveyQuestionTriggerWidget extends Component { } else { this.state.surveyIconWarning = false; this.state.triggerTooltip = _t( - 'Displayed if "%s: %s"', - triggeringQuestionTitle, - this.props.record.data.triggering_answer_id[1] + 'Displayed if "%s".', + this.props.record.data.triggering_answer_ids.records + .map((answer) => answer.data.display_name) + .join('", "'), ); } } else { @@ -55,12 +62,12 @@ export class SurveyQuestionTriggerWidget extends Component { * 2. Robustness, as sequences values do not always match between server * provided values when the records are not saved. * - * @returns { String } + * @returns {{ triggerError: String, misplacedTriggerQuestionRecords: Record[] }} * * `""`: No trigger error (also if `triggering_question_id` * field is not set). - * * `"MISSING_TRIGGER_ERROR"`: `triggering_question_id` field is set - * and trigger record is not found. This can happen when a question - * used as trigger is deleted on the client but not yet saved to DB. + * * `"MISSING_TRIGGER_ERROR"`: `triggering_questions_ids` field is set + * but trigger record is not found. This can happen if all questions + * used as triggers are deleted on the client but not yet saved to DB. * * `"MISPLACED_TRIGGER_WARNING"`: a `triggering_question_id` is set * but is positioned after the current record in the list. This can * happen if the triggering or the triggered question is moved. @@ -71,37 +78,48 @@ export class SurveyQuestionTriggerWidget extends Component { */ get surveyQuestionTriggerError() { const record = this.props.record; - if (!record.data.triggering_question_id) { - return ""; + if (!record.data.triggering_question_ids.records.length) { + return { triggerError: "", misplacedTriggerQuestionRecords: [] }; + } + if (this.props.record.data.questions_selection === 'random') { + return { triggerError: 'WRONG_QUESTIONS_SELECTION_WARNING', misplacedTriggerQuestionRecords: [] }; } - const triggerId = record.data.triggering_question_id[0]; - let triggerRecord = record.model.root.data.question_and_page_ids.records.find(rec => rec.resId === triggerId); - if (!triggerRecord) { - return "MISSING_TRIGGER_ERROR"; + const missingTriggerQuestionsIds = []; + let triggerQuestionsRecords = []; + for (const triggeringQuestion of record.data.triggering_question_ids.records) { + const triggeringQuestionRecord = record.model.root.data.question_and_page_ids.records.find( + rec => rec.resId === triggeringQuestion.resId); + if (triggeringQuestionRecord) { + triggerQuestionsRecords.push(triggeringQuestionRecord); + } else { // Trigger question was deleted from the list + missingTriggerQuestionsIds.push(triggeringQuestion.resId); + } } - if (record.data.questions_selection === 'random') { - return "WRONG_QUESTIONS_SELECTION_WARNING"; + + if (missingTriggerQuestionsIds.length === this.props.record.data.triggering_question_ids.records.length) { + return { triggerError: 'MISSING_TRIGGER_ERROR', misplacedTriggerQuestionRecords: [] }; // only if all are missing } - if (record.data.sequence < triggerRecord.data.sequence || - (record.data.sequence === triggerRecord.data.sequence && record.resId < triggerId)) { - return "MISPLACED_TRIGGER_WARNING"; + const misplacedTriggerQuestionRecords = []; + for (const triggerQuestionRecord of triggerQuestionsRecords) { + if (record.data.sequence < triggerQuestionRecord.data.sequence || + (record.data.sequence === triggerQuestionRecord.data.sequence && record.resId < triggerQuestionRecord.resId)) { + misplacedTriggerQuestionRecords.push(triggerQuestionRecord); + } } - return ""; + return { + triggerError: misplacedTriggerQuestionRecords.length ? "MISPLACED_TRIGGER_WARNING" : "", + misplacedTriggerQuestionRecords: misplacedTriggerQuestionRecords, + }; } } -SurveyQuestionTriggerWidget.template = "survey.surveyQuestionTrigger"; -SurveyQuestionTriggerWidget.props = { - ...standardWidgetProps, -}; - export const surveyQuestionTriggerWidget = { component: SurveyQuestionTriggerWidget, displayName: "Trigger", fieldDependencies: [ - { name: "triggering_question_id", type: "many2one" }, - { name: "triggering_answer_id", type: "many2one" }, + { name: "triggering_question_ids", type: "many2one" }, + { name: "triggering_answer_ids", type: "many2one" }, ], }; registry.category("view_widgets").add("survey_question_trigger", surveyQuestionTriggerWidget); diff --git a/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.xml b/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.xml index 2219649145e..31ce39e0d8f 100644 --- a/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.xml +++ b/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.xml @@ -2,7 +2,7 @@ -