From 25cb3ac1c0895044c5a69d74ba8285f1b37c40bf Mon Sep 17 00:00:00 2001 From: Patrick Hoste Date: Wed, 15 Feb 2023 11:28:32 +0100 Subject: [PATCH] [IMP] survey: allow user to navigate freely This commit allows the user to navigate freely through a survey with mandatory questions. He will be able to skip questions and come back to them later. If he still hasn't answered them at the final submit they will automatically be displayed with an error message to invite the user to answer them. The user can navigate through the skipped questions with a button within the error box. Task-2605657 closes odoo/odoo#119315 Signed-off-by: Thibault Delavallee (tde) --- addons/survey/controllers/main.py | 46 +++++++++--- addons/survey/models/survey_question.py | 11 +-- addons/survey/models/survey_user_input.py | 50 +++++++++++++ addons/survey/static/src/js/survey_form.js | 41 +++++++--- .../survey_roaming_mandatory_questions.js | 74 +++++++++++++++++++ .../survey/tests/test_survey_ui_feedback.py | 45 +++++++++++ addons/survey/views/survey_survey_views.xml | 2 +- addons/survey/views/survey_templates.xml | 32 +++++--- 8 files changed, 262 insertions(+), 39 deletions(-) create mode 100644 addons/survey/static/tests/tours/survey_roaming_mandatory_questions.js diff --git a/addons/survey/controllers/main.py b/addons/survey/controllers/main.py index e46485a0d50..a6efd4f8981 100644 --- a/addons/survey/controllers/main.py +++ b/addons/survey/controllers/main.py @@ -257,11 +257,13 @@ class Survey(http.Controller): """ This method prepares all the data needed for template rendering, in function of the survey user input state. :param post: - previous_page_id : come from the breadcrumb or the back button and force the next questions to load - to be the previous ones. """ + to be the previous ones. + - next_skipped_page : force the display of next skipped question or page if any.""" data = { 'is_html_empty': is_html_empty, 'survey': survey_sudo, 'answer': answer_sudo, + 'skipped_questions': answer_sudo._get_skipped_questions(), 'breadcrumb_pages': [{ 'id': page.id, 'title': page.title, @@ -306,17 +308,23 @@ class Survey(http.Controller): return data if answer_sudo.state == 'in_progress': + next_page_or_question = None if answer_sudo.is_session_answer: next_page_or_question = survey_sudo.session_question_id else: - next_page_or_question = survey_sudo._get_next_page_or_question( - answer_sudo, - answer_sudo.last_displayed_page_id.id if answer_sudo.last_displayed_page_id else 0) + if 'next_skipped_page' in post: + next_page_or_question = answer_sudo._get_next_skipped_page_or_question() + if not next_page_or_question: + next_page_or_question = survey_sudo._get_next_page_or_question( + answer_sudo, + answer_sudo.last_displayed_page_id.id if answer_sudo.last_displayed_page_id else 0) if next_page_or_question: - data.update({ - 'survey_last': survey_sudo._is_last_page_or_question(answer_sudo, next_page_or_question) - }) + if answer_sudo.survey_first_submitted: + survey_last = answer_sudo._is_last_skipped_page_or_question(next_page_or_question) + else: + survey_last = survey_sudo._is_last_page_or_question(answer_sudo, next_page_or_question) + data.update({'survey_last': survey_last}) if answer_sudo.is_session_answer and next_page_or_question.is_time_limited: data.update({ @@ -377,6 +385,7 @@ class Survey(http.Controller): background_image_url = survey_data['page'].background_image_url return { + 'has_skipped_questions': any(answer_sudo._get_skipped_questions()), 'survey_content': survey_content, 'survey_progress': survey_progress, 'survey_navigation': request.env['ir.qweb']._render('survey.survey_navigation', survey_data), @@ -534,16 +543,31 @@ class Survey(http.Controller): answer_sudo._mark_done() elif 'previous_page_id' in post: # when going back, save the last displayed to reload the survey where the user left it. - answer_sudo.write({'last_displayed_page_id': post['previous_page_id']}) + answer_sudo.last_displayed_page_id = post['previous_page_id'] # Go back to specific page using the breadcrumb. Lines are saved and survey continues return self._prepare_question_html(survey_sudo, answer_sudo, **post) + elif 'next_skipped_page_or_question' in post: + answer_sudo.last_displayed_page_id = page_or_question_id + return self._prepare_question_html(survey_sudo, answer_sudo, next_skipped_page=True) else: if not answer_sudo.is_session_answer: - next_page = survey_sudo._get_next_page_or_question(answer_sudo, page_or_question_id) + page_or_question = request.env['survey.question'].sudo().browse(page_or_question_id) + if answer_sudo.survey_first_submitted and answer_sudo._is_last_skipped_page_or_question(page_or_question): + next_page = request.env['survey.question'] + else: + next_page = survey_sudo._get_next_page_or_question(answer_sudo, page_or_question_id) if not next_page: - answer_sudo._mark_done() + if survey_sudo.users_can_go_back and answer_sudo.user_input_line_ids.filtered( + lambda a: a.skipped and a.question_id.constr_mandatory): + answer_sudo.write({ + 'last_displayed_page_id': page_or_question_id, + 'survey_first_submitted': True, + }) + return self._prepare_question_html(survey_sudo, answer_sudo, next_skipped_page=True) + else: + answer_sudo._mark_done() - answer_sudo.write({'last_displayed_page_id': page_or_question_id}) + answer_sudo.last_displayed_page_id = page_or_question_id return self._prepare_question_html(survey_sudo, answer_sudo) diff --git a/addons/survey/models/survey_question.py b/addons/survey/models/survey_question.py index 5965557397c..de57bd88d1f 100644 --- a/addons/survey/models/survey_question.py +++ b/addons/survey/models/survey_question.py @@ -403,11 +403,11 @@ class SurveyQuestion(models.Model): if isinstance(answer, str): answer = answer.strip() # Empty answer to mandatory question - if self.constr_mandatory and not answer and self.question_type not in ['simple_choice', 'multiple_choice']: - return {self.id: self.constr_error_msg or _('This question requires an answer.')} - # because in choices question types, comment can count as answer - if answer or self.question_type in ['simple_choice', 'multiple_choice']: + if not answer and self.question_type not in ['simple_choice', 'multiple_choice']: + if self.constr_mandatory and not self.survey_id.users_can_go_back: + return {self.id: self.constr_error_msg or _('This question requires an answer.')} + else: if self.question_type == 'char_box': return self._validate_char_box(answer) elif self.question_type == 'numerical_box': @@ -473,7 +473,8 @@ class SurveyQuestion(models.Model): def _validate_choice(self, answer, comment): # Empty comment - if self.constr_mandatory \ + if not self.survey_id.users_can_go_back \ + and self.constr_mandatory \ and not answer \ and not (self.comments_allowed and self.comment_count_as_answer and comment): return {self.id: self.constr_error_msg or _('This question requires an answer.')} diff --git a/addons/survey/models/survey_user_input.py b/addons/survey/models/survey_user_input.py index f4cdc3e4595..4d310387a54 100644 --- a/addons/survey/models/survey_user_input.py +++ b/addons/survey/models/survey_user_input.py @@ -52,6 +52,7 @@ class SurveyUserInput(models.Model): scoring_percentage = fields.Float("Score (%)", compute="_compute_scoring_values", store=True, compute_sudo=True) # stored for perf reasons scoring_total = fields.Float("Total Score", compute="_compute_scoring_values", store=True, compute_sudo=True) # stored for perf reasons scoring_success = fields.Boolean('Quizz Passed', compute='_compute_scoring_success', store=True, compute_sudo=True) # stored for perf reasons + survey_first_submitted = fields.Boolean(string='Survey First Submitted') # live sessions is_session_answer = fields.Boolean('Is in a Session', help="Is that user input part of a survey session or not.") question_time_limit_reached = fields.Boolean("Question Time Limit Reached", compute='_compute_question_time_limit_reached') @@ -610,6 +611,55 @@ class SurveyUserInput(models.Model): inactive_questions = self._get_inactive_conditional_questions() return survey.question_ids - inactive_questions + def _get_next_skipped_page_or_question(self): + """Get next skipped question or page in case the option 'can_go_back' is set on the survey + It loops to the first skipped question or page if 'last_displayed_page_id' is the last + skipped question or page.""" + self.ensure_one() + skipped_mandatory_answer_ids = self.user_input_line_ids.filtered( + lambda answer: answer.skipped and answer.question_id.constr_mandatory) + + if not skipped_mandatory_answer_ids: + return self.env['survey.question'] + + page_or_question_key = 'page_id' if self.survey_id.questions_layout == 'page_per_section' else 'question_id' + page_or_question_ids = skipped_mandatory_answer_ids.mapped(page_or_question_key).sorted() + + if self.last_displayed_page_id not in page_or_question_ids\ + or self.last_displayed_page_id == page_or_question_ids[-1]: + return page_or_question_ids[0] + + current_page_index = page_or_question_ids.ids.index(self.last_displayed_page_id.id) + return page_or_question_ids[current_page_index + 1] + + def _get_skipped_questions(self): + self.ensure_one() + + return self.user_input_line_ids.filtered( + lambda answer: answer.skipped and answer.question_id.constr_mandatory).question_id + + def _is_last_skipped_page_or_question(self, page_or_question): + """In case of a submitted survey tells if the question or page is the last + skipped page or question. + + This is used to : + + - Display a Submit button if the actual question is the last skipped question. + - Avoid displaying a Submit button on the last survey question if there are + still skipped questions before. + - Avoid displaying the next page if submitting the latest skipped question. + + :param page_or_question: page if survey's layout is page_per_section, question if page_per_question. + """ + if self.survey_id.questions_layout == 'one_page': + return True + skipped = self._get_skipped_questions() + if not skipped: + return True + if self.survey_id.questions_layout == 'page_per_section': + skipped = skipped.page_id + return skipped == page_or_question + # ------------------------------------------------------------ # MESSAGING # ------------------------------------------------------------ diff --git a/addons/survey/static/src/js/survey_form.js b/addons/survey/static/src/js/survey_form.js index e72170a92fe..89d596949ed 100644 --- a/addons/survey/static/src/js/survey_form.js +++ b/addons/survey/static/src/js/survey_form.js @@ -130,8 +130,10 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa if (keyCode === 13 || keyCode === 39) { // Enter or arrow-right: go Next event.preventDefault(); if (!this.preventEnterSubmit) { - var isFinish = this.$('button[value="finish"]').length !== 0; - this._submitForm({isFinish: isFinish}); + this._submitForm({ + isFinish: this.el.querySelectorAll('button[value="finish"]').length !== 0, + nextSkipped: this.el.querySelectorAll('button[value="next_skipped"]').length !== 0 ? keyCode === 13 : false, + }); } } else if (keyCode === 37) { // arrow-left: previous (if available) // It's easier to actually click on the button (if in the DOM) as it contains necessary @@ -172,7 +174,7 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa .closest('.o_survey_form_choice') .find('.o_survey_comment').length !== 0; if (!questionHasComment) { - this._submitForm({}); + this._submitForm({'nextSkipped': $choiceItemGroup.data('isSkippedQuestion')}); } } } @@ -243,11 +245,13 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa _onSubmit: function (event) { event.preventDefault(); - var options = {}; - var $target = $(event.currentTarget); - if ($target.val() === 'previous') { - options.previousPageId = $target.data('previousPageId'); - } else if ($target.val() === 'finish') { + const options = {}; + const target = event.currentTarget; + if (target.value === 'previous') { + options.previousPageId = parseInt(target.dataset['previousPageId']); + } else if (target.value === 'next_skipped') { + options.nextSkipped = true; + } else if (target.value === 'finish') { options.isFinish = true; } this._submitForm(options); @@ -365,6 +369,9 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa if (options.previousPageId) { params.previous_page_id = options.previousPageId; } + if (options.nextSkipped) { + params.next_skipped_page_or_question = true; + } var route = "/survey/submit"; if (this.options.isStartScreen) { @@ -417,7 +424,7 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa var fadeOutPromise = new Promise(function (resolve, reject) {resolveFadeOut = resolve;}); var selectorsToFadeout = ['.o_survey_form_content']; - if (options.isFinish) { + if (options.isFinish && !this.nextScreenResult.has_skipped_questions) { selectorsToFadeout.push('.breadcrumb', '.o_survey_timer'); deleteCookie('survey_' + self.options.surveyToken); } @@ -454,7 +461,7 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa var self = this; var result = this.nextScreenResult; - if (!(options && options.isFinish) + if ((!(options && options.isFinish) || result.has_skipped_questions) && !this.options.sessionInProgress) { this.preventEnterSubmit = false; } @@ -489,7 +496,7 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa this.surveyTimerWidget.destroy(); } } - if (options && options.isFinish) { + if (options && options.isFinish && !result.has_skipped_questions) { this._initResultWidget(); if (this.surveyBreadcrumbWidget) { this.$('.o_survey_breadcrumb_container').addClass('d-none'); @@ -518,6 +525,7 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa this.$('button[type="submit"]').removeClass('disabled'); + this._scrollToFirstError(); self._focusOnFirstInput(); } else if (result && result.fields && result.error === 'validation') { this.$('.o_survey_form_content').fadeIn(0); @@ -1126,13 +1134,22 @@ publicWidget.registry.SurveyFormWidget = publicWidget.Widget.extend(SurveyPreloa var self = this; var errorKeys = Object.keys(errors || {}); errorKeys.forEach(key => { - self.$("#" + key + '>.o_survey_question_error').append($('

', {text: errors[key]})).addClass("slide_in"); + self.$("#" + key + '>.o_survey_question_error').append($('', {text: errors[key]})).addClass("slide_in"); if (errorKeys[0] === key) { self._scrollToError(self.$('.js_question-wrapper#' + key)); } }); }, + /** + * This method is used to scroll to error generated in the backend. + * (Those errors are displayed when the user skip mandatory question(s)) + */ + _scrollToFirstError: function() { + const errorElem = this.el.querySelector('.o_survey_question_error :not(:empty)'); + errorElem?.scrollIntoView(); + }, + _scrollToError: function ($target) { var scrollLocation = $target.offset().top; var navbarHeight = $('.o_main_navbar').height(); diff --git a/addons/survey/static/tests/tours/survey_roaming_mandatory_questions.js b/addons/survey/static/tests/tours/survey_roaming_mandatory_questions.js new file mode 100644 index 00000000000..c0f80d7736f --- /dev/null +++ b/addons/survey/static/tests/tours/survey_roaming_mandatory_questions.js @@ -0,0 +1,74 @@ +/** @odoo-module **/ + +import { registry } from '@web/core/registry'; + +registry.category('web_tour.tours').add('test_survey_roaming_mandatory_questions', { + test: true, + url: '/survey/start/853ebb30-40f2-43bf-a95a-bbf0e367a365', + steps: () => [{ + content: 'Click on Start', + trigger: 'button.btn:contains("Start")', + }, { + content: 'Skip question Q1', + trigger: 'button.btn:contains("Continue")', + }, { + content: 'Skip question Q2', + extra_trigger: 'div.js_question-wrapper:contains("Q2")', + trigger: 'button.btn:contains("Continue")', + }, { + content: 'Check if Q3 button is Submit', + trigger: 'button.btn:contains("Submit")', + isCheck: true, + }, { + content: 'Go back to Q2', + trigger: 'button.btn[value="previous"]', + }, { + content: 'Check if the alert box is present', + trigger: 'div.o_survey_question_error span', + isCheck: true, + }, { + content: 'Skip question Q2 again', + trigger: 'button.btn:contains("Continue")', + }, { + content: 'Answer Q3', + trigger: 'div.js_question-wrapper:contains("Q3") label:contains("Answer 1")', + }, { + content: 'Click on Submit', + trigger: 'button.btn:contains("Submit")', + }, { + content: 'Check if question is Q1', + trigger: 'div.js_question-wrapper:contains("Q1")', + isCheck: true, + }, { + content: 'Click on "Next Skipped" button', + trigger: 'button.btn:contains("Next Skipped")', + }, { + content: 'Check if question is Q2', + trigger: 'div.js_question-wrapper:contains("Q2")', + isCheck: true, + }, { + content: 'Click on "Next Skipped" button', + trigger: 'button.btn:contains("Next Skipped")', + }, { + content: 'Check if question is Q1 again (should loop on skipped questions)', + trigger: 'div.js_question-wrapper:contains("Q1")', + isCheck: true, + }, { + content: 'Answer Q1', + trigger: 'div.js_question-wrapper:contains("Q1") label:contains("Answer 2")', + }, { + content: 'Check if the visible question is the skipped question Q2', + trigger: 'div.js_question-wrapper:contains("Q2")', + isCheck: true, + }, { + content: 'Answer Q2', + trigger: 'div.js_question-wrapper:contains("Q2") label:contains("Answer 3")', + }, { + content: 'Click on Submit', + trigger: 'button.btn:contains("Submit")', + }, { + content: 'Check if the survey is done', + trigger: 'div.o_survey_finished h1:contains("Thank you!")', + isCheck: true, + }], +}); diff --git a/addons/survey/tests/test_survey_ui_feedback.py b/addons/survey/tests/test_survey_ui_feedback.py index 51c85f0d3d7..32eeffa7485 100644 --- a/addons/survey/tests/test_survey_ui_feedback.py +++ b/addons/survey/tests/test_survey_ui_feedback.py @@ -292,3 +292,48 @@ class TestUiFeedback(HttpCaseWithUserDemo): def test_06_survey_prefill(self): access_token = self.survey_feedback.access_token self.start_tour("/survey/start/%s" % access_token, 'test_survey_prefill') + + def test_07_survey_roaming_mandatory_questions(self): + survey_with_mandatory_questions = self.env['survey.survey'].create({ + 'title': 'Survey With Mandatory questions', + 'access_token': '853ebb30-40f2-43bf-a95a-bbf0e367a365', + 'access_mode': 'public', + 'users_can_go_back': True, + 'questions_layout': 'page_per_question', + 'description': "

Test survey with roaming freely option and mandatory questions

", + 'question_and_page_ids': [ + Command.create({ + 'title': 'Q1', + 'sequence': 1, + 'question_type': 'simple_choice', + 'constr_mandatory': True, + 'suggested_answer_ids': [ + Command.create({'value': 'Answer 1'}), + Command.create({'value': 'Answer 2'}), + Command.create({'value': 'Answer 3'}), + ], + }), Command.create({ + 'title': 'Q2', + 'sequence': 2, + 'question_type': 'simple_choice', + 'constr_mandatory': True, + 'suggested_answer_ids': [ + Command.create({'value': 'Answer 1'}), + Command.create({'value': 'Answer 2'}), + Command.create({'value': 'Answer 3'}), + ], + }), Command.create({ + 'title': 'Q3', + 'sequence': 3, + 'question_type': 'simple_choice', + 'constr_mandatory': True, + 'suggested_answer_ids': [ + Command.create({'value': 'Answer 1'}), + Command.create({'value': 'Answer 2'}), + ], + }), + ] + }) + + access_token = survey_with_mandatory_questions.access_token + self.start_tour("/survey/start/%s" % access_token, 'test_survey_roaming_mandatory_questions') diff --git a/addons/survey/views/survey_survey_views.xml b/addons/survey/views/survey_survey_views.xml index e5fb985752e..51d17fcadee 100644 --- a/addons/survey/views/survey_survey_views.xml +++ b/addons/survey/views/survey_survey_views.xml @@ -97,7 +97,7 @@ invisible="questions_layout == 'one_page' or survey_type == 'live_session'"/> - diff --git a/addons/survey/views/survey_templates.xml b/addons/survey/views/survey_templates.xml index 1dfcb9664ad..c4e5a7f376a 100644 --- a/addons/survey/views/survey_templates.xml +++ b/addons/survey/views/survey_templates.xml @@ -191,7 +191,7 @@
- + or press Enter @@ -209,9 +209,12 @@
- or press Enter
@@ -247,8 +250,11 @@
- @@ -318,10 +324,12 @@ This question requires an answer. The answer you entered is not valid. If other, please specify: +
+ t-att-id="question.id" + t-att-data-required="bool(question.constr_mandatory and (not survey.users_can_go_back or survey.questions_layout == 'one_page')) or None" + t-att-data-constr-error-msg="question.constr_error_msg or default_constr_error_msg if question.constr_mandatory else None" + t-att-data-validation-error-msg="question.validation_error_msg or default_validation_error_msg if question.validation_required else None">

@@ -337,7 +345,10 @@ - +
+ +

@@ -414,6 +425,7 @@