diff --git a/addons/survey/__manifest__.py b/addons/survey/__manifest__.py index dd7cdc98a44..f7dfbf861a3 100644 --- a/addons/survey/__manifest__.py +++ b/addons/survey/__manifest__.py @@ -94,7 +94,8 @@ sent mails with personal token for the invitation of the survey. 'survey/static/src/question_page/*', 'survey/static/src/js/fields_section_one2many.js', 'survey/static/src/js/fields_form_page_description.js', - 'survey/static/src/views/*.js', + 'survey/static/src/views/**/*.js', + 'survey/static/src/views/**/*.xml', 'survey/static/src/scss/survey_survey_views.scss', 'survey/static/src/scss/survey_question_views.scss', 'survey/static/src/scss/survey_templates_results.scss', diff --git a/addons/survey/models/survey_question.py b/addons/survey/models/survey_question.py index 222aa29779c..a23576b3db9 100644 --- a/addons/survey/models/survey_question.py +++ b/addons/survey/models/survey_question.py @@ -124,7 +124,7 @@ class SurveyQuestion(models.Model): validation_max_date = fields.Date('Maximum Date') validation_min_datetime = fields.Datetime('Minimum Datetime') validation_max_datetime = fields.Datetime('Maximum Datetime') - validation_error_msg = fields.Char('Validation Error message', translate=True) + validation_error_msg = fields.Char('Validation Error', translate=True) constr_mandatory = fields.Boolean('Mandatory Answer') constr_error_msg = fields.Char('Error message', translate=True) # answers @@ -134,7 +134,7 @@ class SurveyQuestion(models.Model): # Conditional display is_conditional = fields.Boolean( - string='Conditional Display', copy=True, help="""If checked, this question will be displayed only + 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", @@ -144,6 +144,11 @@ class SurveyQuestion(models.Model): '|', \ ('sequence', '<', sequence), \ '&', ('sequence', '=', sequence), ('id', '<', id)]") + 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?", + 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.", @@ -160,7 +165,13 @@ class SurveyQuestion(models.Model): ('scored_datetime_have_answers', "CHECK (is_scored_question != True OR question_type != 'datetime' OR answer_datetime is not null)", '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') + '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.'), ] # ------------------------------------------------------------------------- @@ -269,6 +280,51 @@ 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') + def _compute_allowed_triggering_question_ids(self): + """ 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']), + ('suggested_answer_ids', '!=', False), + ('survey_id', 'in', self.survey_id.ids) + ]) + # 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.env.cr.execute( + "SELECT id, sequence FROM survey_question WHERE id =ANY(%s)", + [conditional_questions.ids] + ) + conditional_questions_sequences = dict(self.env.cr.fetchall()) # id: sequence mapping + + for question in conditional_questions: + question_id = question._origin.id + if not question_id: # New question + conditional_questions.allowed_triggering_question_ids = possible_trigger_questions + question.is_placed_before_trigger = False + continue + + question_sequence = conditional_questions_sequences[question_id] + + question.allowed_triggering_question_ids = possible_trigger_questions.filtered( + 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) + @api.depends('is_conditional') def _compute_triggering_question_id(self): """ Used as an 'onchange' : Reset the triggering question if user uncheck 'Conditional Display' @@ -312,6 +368,34 @@ class SurveyQuestion(models.Model): else: question.is_scored_question = False + @api.onchange('question_type', 'validation_required') + def _onchange_validation_parameters(self): + """Ensure no value stays set but not visible on form, + preventing saving (+consistency with question type).""" + self.validation_email = False + self.validation_length_min = 0 + self.validation_length_max = 0 + self.validation_min_date = False + self.validation_max_date = False + self.validation_min_datetime = False + self.validation_max_datetime = False + self.validation_min_float_value = 0 + self.validation_max_float_value = 0 + + # ------------------------------------------------------------ + # CRUD + # ------------------------------------------------------------ + + 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() + # ------------------------------------------------------------ # VALIDATION # ------------------------------------------------------------ @@ -612,3 +696,13 @@ class SurveyQuestionAnswer(models.Model): for label in self: if not bool(label.question_id) != bool(label.matrix_question_id): raise ValidationError(_("A label must be attached to only one question.")) + + 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 73729a0495f..1c701575a03 100644 --- a/addons/survey/models/survey_survey.py +++ b/addons/survey/models/survey_survey.py @@ -367,7 +367,8 @@ class Survey(models.Model): This implementation assumes that the order of created questions/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 standard copy. + 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() clone = super(Survey, self).copy(default) @@ -382,6 +383,7 @@ class Survey(models.Model): } for src, dst in zip(self.question_ids, clone.question_ids): 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) return clone 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 93d12ca459b..d460946cf5f 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 @@ -96,4 +96,24 @@ export class QuestionPageListRenderer extends ListRenderer { } return super.onCellKeydownEditMode(...arguments); } + + /** + * Save the survey after a question used as trigger is deleted. This allows + * immediate feedback on the form view as the triggers will be removed + * anyway on the records by the ORM. + * + * @override + * @param record + * @return {Promise} + */ + async onDeleteRecord(record) { + const triggeredRecords = this.props.list.records.filter(rec => rec.data.triggering_question_id[0] === record.data.id); + if (triggeredRecords.length) { + const res = await super.onDeleteRecord(record); + await this.props.list.model.root.save({stayInEdition: true}); + return res; + } else { + return super.onDeleteRecord(record); + } + } } diff --git a/addons/survey/static/src/question_page/question_page_one2many_field.js b/addons/survey/static/src/question_page/question_page_one2many_field.js index cbe0b7ee5dc..16faf8f77ae 100644 --- a/addons/survey/static/src/question_page/question_page_one2many_field.js +++ b/addons/survey/static/src/question_page/question_page_one2many_field.js @@ -2,6 +2,8 @@ import { QuestionPageListRenderer } from "./question_page_list_renderer"; import { registry } from "@web/core/registry"; +import { useOpenX2ManyRecord, useX2ManyCrud } from "@web/views/fields/relational_utils"; +import { useService } from "@web/core/utils/hooks"; import { X2ManyField } from "@web/views/fields/x2many/x2many_field"; const { useSubEnv } = owl; @@ -12,6 +14,88 @@ class QuestionPageOneToManyField extends X2ManyField { useSubEnv({ openRecord: (record) => this.openRecord(record), }); + this.notificationService = useService("notification"); + + // Systematically and automatically save SurveyForm at each question edit/creation + // enables checking validation parameters consistency and using questions as triggers + // immediately during question creation. + // Preparing everything in order to override `this._openRecord` below. + const { saveRecord: superSaveRecord, updateRecord: superUpdateRecord } = useX2ManyCrud( + () => this.list, + this.isMany2Many + ); + + const self = this; + const saveRecord = async (record) => { + const saveResponse = await superSaveRecord(record); + try { + await self.props.record.save({stayInEdition: true, throwOnError: true}); + } catch (error) { + return self.handleSurveySaveError(error, record); + } + return saveResponse; + }; + + const updateRecord = async (record) => { + const updateResponse = await superUpdateRecord(record); + try { + await self.props.record.save({stayInEdition: true, throwOnError: true}); + } catch (error) { + return self.handleSurveySaveError(error); + } + return updateResponse; + }; + + const openRecord = useOpenX2ManyRecord({ + resModel: this.list.resModel, + activeField: this.activeField, + activeActions: this.activeActions, + getList: () => this.list, + saveRecord, + updateRecord, + }); + this._openRecord = async (params) => { + if (!await self.props.record.save({ stayInEdition: true })) { + // do not open question form as it won't be savable either. + return; + } + if (params.record) { + // Force synchronization of fields that depend on sequence + // (allowed_triggering_question_ids, is_placed_before_trigger) + // as records may have been re-ordered before opening this one. + await params.record.load(); + } + openRecord(params); + }; + } + + /** + * For convenience, we'll prevent closing the question form dialog and + * stay in edit mode to make sure only valid records are saved. Therefore, + * two things should be cared for in case of error occurring when saving + * the question: + * * Remove erroneous question row added to the embedded list + * * Replace default error modal with a notification + * + * @param {Error} error Error thrown when saving survey/question. + * @param {Record?} recordToDelete (optional) In case the error is + * thrown when saving a new question, it should be deleted from the + * list. + */ + async handleSurveySaveError(error, recordToDelete) { + error.event.preventDefault(); + if (recordToDelete) { + const listRecord = this.list.records.find(r => r.__bm_handle__ === recordToDelete.__bm_handle__); + await this.list.delete(listRecord.id); + } + this.notificationService.add( + error.message.data.message, { + title: this.env._t("Validation Error"), + type: "danger" + } + ); + // Prevent closing the question form view + return Promise.reject("Impossible to save survey, see error notification."); } } QuestionPageOneToManyField.components = { diff --git a/addons/survey/static/src/scss/survey_question_views.scss b/addons/survey/static/src/scss/survey_question_views.scss index fca016908ef..6c25b808d2f 100644 --- a/addons/survey/static/src/scss/survey_question_views.scss +++ b/addons/survey/static/src/scss/survey_question_views.scss @@ -23,4 +23,11 @@ .o_preview_questions_choice { line-height: 1rem; } + + .o_survey_question_validation_parameters { + @extend .oe_inline; + > div, input.o_survey_question_validation_inline { + width: 45% !important; + } + } } 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 new file mode 100644 index 00000000000..7a2202a3bdd --- /dev/null +++ b/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.js @@ -0,0 +1,116 @@ +/** @odoo-module **/ + +import { _lt } from "@web/core/l10n/translation"; +import { registry } from "@web/core/registry"; +import { sprintf } from '@web/core/utils/strings'; +import { standardWidgetProps} from "@web/views/widgets/standard_widget_props"; + +const { Component, useEffect, useRef, useState } = owl; + +export class SurveyQuestionTriggerWidget extends Component { + setup() { + super.setup(); + this.button = useRef('survey_question_trigger'); + this.state = useState({ + triggeringQuestionTitle: this.props.record.data.triggering_question_id, + surveyQuestionTriggerTooltip: "", + surveyQuestionTriggerIconClass: "", + }); + useEffect(() => { + if (this.button && this.button.el) { + this.state.triggeringQuestionTitle = this.props.record.data.triggering_question_id[1]; + const triggerError = this.surveyQuestionTriggerError; + if (triggerError === "MISPLACED_TRIGGER_WARNING") { + this.state.surveyQuestionTriggerTooltip = sprintf( + '⚠ ' + _lt('This question is positioned before its trigger ("%s") and will be skipped.'), + this.state.triggeringQuestionTitle); + this.state.surveyQuestionTriggerIconClass = 'fa-exclamation-triangle text-warning'; + } else if (triggerError === "WRONG_QUESTIONS_SELECTION_WARNING") { + this.state.surveyQuestionTriggerTooltip = '⚠ ' + _lt( + 'Conditional display is not available when questions are randomly picked.'); + this.state.surveyQuestionTriggerIconClass = 'fa-exclamation-triangle text-warning'; + } else if (triggerError === "MISSING_TRIGGER_ERROR") { + // This case must be handled to not temporarily render the "normal" icon if previously + // on an error state, which would cause a flicker as the trigger itself will be removed + // at next save (auto on survey form and primary list view). + } else { + this.state.surveyQuestionTriggerTooltip = sprintf(_lt('Displayed if "%s: %s"'), + this.state.triggeringQuestionTitle, this.props.record.data.triggering_answer_id[1]); + this.state.surveyQuestionTriggerIconClass = 'fa-code-fork'; + } + } else { + this.state.triggeringQuestionTitle = ""; + this.state.surveyQuestionTriggerTooltip = ""; + this.state.surveyQuestionTriggerIconClass = ""; + } + }); + } + + /** + * `surveyQuestionTriggerError` is computed here and does not rely on + * record data (is_placed_before_trigger) for two linked reasons: + * 1. Performance: we avoid saving the survey each time a line is moved. + * 2. Robustness, as sequences values do not always match between server + * provided values when the records are not saved. + * + * @returns { String } + * * `""`: 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. + * * `"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. + * * `"WRONG_QUESTIONS_SELECTION_WARNING"`: a `triggering_question_id` + * is set but the survey is configured to randomize questions asked + * mode which ignores the triggers. This can happen if the survey mode + * is changed after triggers are set. + */ + get surveyQuestionTriggerError() { + const record = this.props.record; + if (!record.data.triggering_question_id) { + return ""; + } + const triggerId = record.data.triggering_question_id[0]; + let triggerRecord; + if (this.props.isSurveyForm) { // embedded list + triggerRecord = record.model.root.data.question_and_page_ids.records.find(rec => rec.data.id === triggerId); + } else { + triggerRecord = record.model.root.records.find(rec => rec.resId === triggerId); + } + + if (!triggerRecord) { + return "MISSING_TRIGGER_ERROR"; + } + if (record.data.questions_selection === 'random') { + return "WRONG_QUESTIONS_SELECTION_WARNING"; + } + if (record.data.sequence < triggerRecord.data.sequence || + (record.data.sequence === triggerRecord.data.sequence && record.data.id < triggerId)) { + return "MISPLACED_TRIGGER_WARNING"; + } + return ""; + } +} + +SurveyQuestionTriggerWidget.template = "survey.surveyQuestionTrigger"; +SurveyQuestionTriggerWidget.props = { + ...standardWidgetProps, + isSurveyForm: { type: Boolean, optional: true }, +}; + +SurveyQuestionTriggerWidget.defaultProps = { + isSurveyForm: false +}; + +SurveyQuestionTriggerWidget.extractProps = ({ attrs }) => { + return { + isSurveyForm: attrs.options.isSurveyForm + }; +}; + +SurveyQuestionTriggerWidget.displayName = 'Trigger'; +SurveyQuestionTriggerWidget.supportedTypes = ['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 new file mode 100644 index 00000000000..42ca868b041 --- /dev/null +++ b/addons/survey/static/src/views/widgets/survey_question_trigger/survey_question_trigger.xml @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/addons/survey/static/tests/components/question_page_one2many_field_tests.js b/addons/survey/static/tests/components/question_page_one2many_field_tests.js index a1abe443f02..1f10ebd5fd6 100644 --- a/addons/survey/static/tests/components/question_page_one2many_field_tests.js +++ b/addons/survey/static/tests/components/question_page_one2many_field_tests.js @@ -2,6 +2,7 @@ import { click, editInput, getFixture, nextTick, triggerHotkey } from "@web/../tests/helpers/utils"; import { makeView, setupViewRegistries } from "@web/../tests/views/helpers"; +import { RPCError } from "@web/core/network/rpc_service"; QUnit.module("QuestionPageOneToManyField", (hooks) => { let serverData; @@ -12,16 +13,20 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { serverData = { models: { - partner: { - fields: { lines: { type: "one2many", relation: "lines_sections" } }, + survey: { + fields: { + question_and_page_ids: { type: "one2many", relation: "survey_question" }, + favorite_color: { string: "Favorite color", type: "char" } + }, records: [ { id: 1, - lines: [1, 2], + question_and_page_ids: [1, 2], + favorite_color: "" }, ], }, - lines_sections: { + survey_question: { fields: { is_page: { type: "boolean" }, title: { type: "char", string: "Title" }, @@ -44,7 +49,7 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { }, }, views: { - "lines_sections,false,form": ` + "survey_question,false,form": `
@@ -58,12 +63,12 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { QUnit.test("basic rendering", async (assert) => { await makeView({ type: "form", - resModel: "partner", + resModel: "survey", resId: 1, serverData, arch: `
- + @@ -87,12 +92,12 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { QUnit.test("click on section behaves as usual in readonly mode", async (assert) => { await makeView({ type: "form", - resModel: "partner", + resModel: "survey", resId: 1, serverData, arch: ` - + @@ -112,12 +117,12 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { QUnit.test("click on section edit the section in place", async (assert) => { await makeView({ type: "form", - resModel: "partner", + resModel: "survey", resId: 1, serverData, arch: ` - + @@ -131,15 +136,16 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { assert.containsNone(target, ".modal .o_form_view"); }); - QUnit.test("click on real line opens a dialog", async (assert) => { + QUnit.test("click on real line saves form and opens a dialog", async (assert) => { await makeView({ type: "form", - resModel: "partner", + resModel: "survey", resId: 1, serverData, arch: ` - + + @@ -148,21 +154,68 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { `, + mockRPC(route, args) { + if (args.method === "write" && args.model === "survey") { + assert.step("save parent form"); + } + }, + }); + await editInput(target, "[name='favorite_color'] input", "Yellow"); + await click(target.querySelector(".o_data_row:nth-child(2) .o_data_cell")); + // Edit content to trigger the expected actual save at row opening + assert.verifySteps(["save parent form"]); + assert.containsNone(target, ".o_selected_row"); + assert.containsOnce(target, ".modal .o_form_view"); + }); + + QUnit.test("A validation error from saving parent form notifies and prevents dialog from closing", async (assert) => { + await makeView({ + type: "form", + resModel: "survey", + resId: 1, + serverData, + arch: ` +
+ + + + + + + +
+ `, + mockRPC(route, args) { + if (args.method === "write" && args.model === "survey") { + assert.step("save parent form"); + const surveyValidationError = new RPCError(); + Object.assign(surveyValidationError, { + exceptionName: "odoo.exceptions.ValidationError", + data: { + message: "This isn't right!", + }, + }); + return Promise.reject(surveyValidationError); + } + }, }); await click(target.querySelector(".o_data_row:nth-child(2) .o_data_cell")); - assert.containsNone(target, ".o_selected_row"); + await editInput(target, ".o_dialog:not(.o_inactive_modal) .modal-body [name='title'] input", "Invalid RecordTitle"); + await click(target.querySelector(".o_dialog:not(.o_inactive_modal) .o_form_button_save")); + assert.verifySteps(["save parent form"]); + assert.containsOnce(document.body, ".o_notification"); assert.containsOnce(target, ".modal .o_form_view"); }); QUnit.test("can create section inline", async (assert) => { await makeView({ type: "form", - resModel: "partner", + resModel: "survey", resId: 1, serverData, arch: `
- + @@ -187,12 +240,12 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { QUnit.test("creates real record in form dialog", async (assert) => { await makeView({ type: "form", - resModel: "partner", + resModel: "survey", resId: 1, serverData, arch: ` - + @@ -217,12 +270,12 @@ QUnit.module("QuestionPageOneToManyField", (hooks) => { async (assert) => { await makeView({ type: "form", - resModel: "partner", + resModel: "survey", resId: 1, serverData, arch: ` - + diff --git a/addons/survey/static/tests/components/survey_question_trigger_widget_tests.js b/addons/survey/static/tests/components/survey_question_trigger_widget_tests.js new file mode 100644 index 00000000000..0aab60ea4eb --- /dev/null +++ b/addons/survey/static/tests/components/survey_question_trigger_widget_tests.js @@ -0,0 +1,155 @@ +/** @odoo-module */ + +import { dragAndDrop, getFixture, nextTick } from "@web/../tests/helpers/utils"; +import { makeView, setupViewRegistries } from "@web/../tests/views/helpers"; + +QUnit.module("SurveyQuestionTriggerWidget", (hooks) => { + let serverData; + let target; + + hooks.beforeEach(() => { + target = getFixture(); + + serverData = { + models: { + survey: { + fields: { + question_and_page_ids: { type: "one2many", relation: "survey_question" }, + }, + records: [ + { + id: 1, + question_and_page_ids: [1, 2], + }, + ], + }, + survey_question: { + fields: { + sequence: { type: "number" }, + name: { type: "char", string: "name", }, + triggering_question_id: { + type: "many2one", + string: "Triggering question", + relation: "survey_question", + required: false, + searchable: true, + }, + triggering_answer_id: { + type: "many2one", + string: "Triggering answer", + relation: "survey_question_answer", + required: false, + searchable: true, + }, + }, + records: [ + { + id: 1, + sequence: 1, + name: "Question 1", + triggering_question_id: null, + triggering_answer_id: null, + }, { + id: 2, + sequence: 2, + name: "Question 2", + triggering_question_id: 1, + triggering_answer_id: 1, + }, + ], + }, + survey_question_answer: { + fields: { + name: {type: "char", string: "name"}, + }, + records: [ + { + id: 1, + name: "Answer 1" + }, + ] + } + }, + views: { + "survey_question,false,form": ` + + + + + + + + + `, + }, + }; + + setupViewRegistries(); + }); + + QUnit.test("dynamic rendering of surveyQuestionTriggerError rows", async (assert) => { + await makeView({ + type: "form", + resModel: "survey", + resId: 1, + serverData, + arch: ` +
+ + + + + + + + + +
+ `, + }); + + assert.containsOnce(target, ".o_field_x2many .o_list_renderer table.o_list_table"); + assert.containsN(target, ".o_data_row", 2); + let rows = target.querySelectorAll(".o_data_row"); + + assert.strictEqual(rows[0].textContent, "Question 1"); + let q1TriggerDiv = rows[0].querySelector("td.o_data_cell div.o_widget_survey_question_trigger"); + assert.containsNone(q1TriggerDiv, "button"); + + assert.strictEqual(rows[1].textContent, "Question 2"); + let q2TriggerDiv = rows[1].querySelector("td.o_data_cell div.o_widget_survey_question_trigger"); + assert.containsOnce(q2TriggerDiv, "button"); + // Question 2 is correctly placed after Question 1 + let triggerIcon = q2TriggerDiv.querySelector("button i"); + assert.doesNotHaveClass(triggerIcon, "text-warning"); + assert.hasAttrValue(triggerIcon, 'data-tooltip', 'Displayed if "Question 1: Answer 1"', + 'Trigger tooltip should be \'Displayed if "Question 1: Answer 1"\'.'); + + // drag and drop Question 2 (triggered) before Question 1 (trigger) + await dragAndDrop("tbody tr:nth-child(2) .o_handle_cell", "tbody tr:nth-child(1)"); + await nextTick(); + rows = target.querySelectorAll(".o_data_row"); + + assert.strictEqual(rows[0].textContent, "Question 2"); + q2TriggerDiv = rows[0].querySelector("td.o_data_cell div.o_widget_survey_question_trigger"); + assert.containsOnce(q2TriggerDiv, "button"); + triggerIcon = q2TriggerDiv.querySelector("button i"); + assert.hasClass(triggerIcon, "text-warning"); + assert.strictEqual( + triggerIcon.getAttribute('data-tooltip'), + '⚠ This question is positioned before its trigger ("Question 1") and will be skipped.', + 'Trigger tooltip should have been changed to misplacement error message.' + ); + + // drag and drop Question 1 (trigger) back before Question 2 (triggered) + await dragAndDrop("tbody tr:nth-child(2) .o_handle_cell", "tbody tr:nth-child(1)"); + await nextTick(); + + rows = target.querySelectorAll(".o_data_row"); + + assert.strictEqual(rows[1].textContent, "Question 2"); + assert.doesNotHaveClass(rows[1].querySelector("td.o_data_cell div.o_widget_survey_question_trigger button i"), "text-warning"); + assert.hasAttrValue(triggerIcon, 'data-tooltip', 'Displayed if "Question 1: Answer 1"', + 'Trigger tooltip should be back to \'Displayed if "Question 1: Answer 1"\'.'); + }); +}); diff --git a/addons/survey/static/tests/tours/survey_form.js b/addons/survey/static/tests/tours/survey_form.js new file mode 100644 index 00000000000..4537d280460 --- /dev/null +++ b/addons/survey/static/tests/tours/survey_form.js @@ -0,0 +1,276 @@ +/** @odoo-module **/ + +import tour from 'web_tour.tour'; + +tour.register('survey_tour_test_survey_form_triggers', { + test: true, + url: '/web', +}, [ + tour.stepUtils.showAppsMenuItem(), + { + content: 'Go to Survey', + trigger: '.o_app[data-menu-xmlid="survey.menu_surveys"]', + }, { + content: "Create a new survey", + trigger: ".o-kanban-button-new", + }, { + content: "Set the Survey's title", + trigger: ".o_field_widget[name=title] input", + run: "text Test survey", + }, { + content: "Add a first question", + trigger: "td.o_field_x2many_list_row_add a", + }, { + content: "Set the first question's title", + trigger: ".modal-content .o_field_widget[name=title] input", + run: "text Question 1", + }, + ...addTwoAnswers(), + ...saveAndNew(), + { + content: "Set the second question's title", + trigger: ".o_field_widget[name=title] input", + run: "text Question 2", + in_modal: true, + }, + ...addTwoAnswers(), + ...toggleIsConditional(), + { + content: "Set a trigger question for the first question", + trigger: ".o_field_widget[name=triggering_question_id] input", + run: "click", + in_modal: true, + }, { + content: "Set the first question as trigger", + trigger: 'ul.ui-autocomplete a:contains("Question 1")', + run: 'click', + in_modal: true, + }, { + content: "Set a trigger answer for first question", + trigger: ".modal-content .o_field_widget[name=triggering_answer_id] input", + run: "click", + in_modal: true, + }, { + content: "Set the first answer as trigger", + trigger: 'ul.ui-autocomplete a:contains("Answer A")', + run: 'click', + }, + ...changeTab("answers"), + ...saveAndNew(), + { + content: "Set the third question's title", + trigger: ".o_field_widget[name=title] input", + run: "text Question 3", + in_modal: true, + }, + ...addTwoAnswers(), + ...toggleIsConditional(), + { + content: "Set a trigger question for the second question", + trigger: ".o_field_widget[name=triggering_question_id] input", + run: "click", + in_modal: true, + }, { + content: "Set the second question as trigger", + trigger: 'ul.ui-autocomplete a:contains("Question 2")', + run: 'click', + in_modal: true, + }, { + content: "Set a trigger answer for second question", + trigger: ".modal-content .o_field_widget[name=triggering_answer_id] input", + run: "click", + in_modal: true, + }, { + content: "Set the second answer as trigger", + trigger: 'ul.ui-autocomplete a:contains("Answer B")', + run: 'click', + }, + ...changeTab("answers"), + ...tour.stepUtils.saveForm(), + // Q2 and Q3 should have fa-fork icons. Assumes that the Trigger widget's column is 2 places after the title's. + { + content: "Check that Question 2 has 'normal' trigger icon", + trigger: "tr:contains('Question 2') button i.fa-code-fork", + run: () => {}, // it's a check + }, { + content: "Check that Question 3 has 'normal' trigger icon", + trigger: "tr:contains('Question 3') button i.fa-code-fork", + run: () => {}, // it's a check + }, { + content: "Move Question 3 above its trigger (Question 2)", + trigger: "tr.o_data_row:nth-child(3) td[name=sequence]", + run: "drag_and_drop_native div[name=question_and_page_ids] table tbody tr:nth-child(2)", + }, { + content: "Check that Question 3 has 'warning' trigger icon", + trigger: "tr:contains('Question 3') button i.fa-exclamation-triangle", + run: () => {}, // it's a check + }, { + content: "Open that question to check the server's misplacement evaluation agrees", + trigger: "tr.o_data_row td[data-tooltip='Question 3']", + run: "click", + }, { + content: "Check that an alert is shown", + trigger: ".o_form_sheet_bg div:first-child.alert-warning:contains('positioned before its trigger')", + in_modal: true, + }, + ...changeTab("options"), + { + content: "Change trigger question", + trigger: ".o_field_widget[name=triggering_question_id] input", + run: "click", + in_modal: true, + }, { + content: "Set the first question as trigger instead", + trigger: 'ul.ui-autocomplete a:contains("Question 1")', + run: 'click', + in_modal: true, + }, { + content: "Check that the alert is gone", + trigger: `.o_form_sheet_bg div:first-child:not(.alert-warning).o_form_sheet`, + in_modal: true, + run: () => {}, // it's a check + }, { + content: "Change the trigger answer as well", + trigger: ".modal-content .o_field_widget[name=triggering_answer_id] input", + run: "click", + in_modal: true, + }, { + content: "Set the second answer as trigger, then", + trigger: 'ul.ui-autocomplete a:contains("Answer B")', + run: 'click', + }, + ...tour.stepUtils.saveForm(), + { + content: "Check that Question 3 has its 'normal' trigger icon back", + trigger: "tr:contains('Question 3') button i.fa-code-fork", + run: () => {}, // it's a check + }, { + content: "Move Question 3 back below Question 2", + trigger: "tr.o_data_row:nth-child(2) td[name=sequence]", + run: "drag_and_drop_native div[name=question_and_page_ids] table tbody tr:nth-child(3)", + }, { + content: "Open that question again", + trigger: "tr.o_data_row td[data-tooltip='Question 3']", + run: "click", + }, + ...changeTab("options"), + { + content: "Change trigger to confirm we can now use Question 2 again", + trigger: ".modal-content .o_field_widget[name=triggering_question_id] input", + run: "click", + in_modal: true, + }, { + content: "Question 2 is allowed as trigger again", + trigger: '.modal-content ul.ui-autocomplete a:contains("Question 2")', + in_modal: true, + }, { + content: "Change the trigger answer back as well", + trigger: ".modal-content .o_field_widget[name=triggering_answer_id] input", + in_modal: true, + }, { + content: "Set the second answer as trigger, then", + trigger: '.modal-content ul.ui-autocomplete a:contains("Answer B")', + run: "click", + }, + ...tour.stepUtils.saveForm(), + // Deleting trigger answers or whole question gracefully remove the trigger automatically + { + content: "Open Question 2 again", + trigger: "tr.o_data_row td[data-tooltip='Question 2']", + run: "click", + }, { + content: "Delete Answer B", + trigger: "div[name=suggested_answer_ids] tr:contains('Answer B') button[name=delete]", + }, + ...tour.stepUtils.saveForm(), + { + content: "Check that Question 3 no longer has a trigger icon", + trigger: "div[name=question_and_page_ids] tr:contains('Question 3') div.o_widget_survey_question_trigger:not(:has(button))", + run: () => {}, // it's a check + }, { + content: "Check that Question 2 however still has a trigger icon", + trigger: "tr:contains('Question 2') button i.fa-code-fork", + run: () => {}, // it's a check + }, { + content: "Delete Question 1", + trigger: "tr:contains('Question 1') button[name=delete]", + run: "click", + }, { + content: "Check that now Question 2 too does no longer have a trigger icon", + trigger: "tr:contains('Question 2') div.o_widget_survey_question_trigger:not(:has(button))", + run: () => {}, // it's a check + }, { + content: 'Go back to Kanban View', + trigger: 'a[data-menu-xmlid="survey.menu_surveys"]', + }, { + content: "Check that we arrived on the kanban view", + trigger: ".o-kanban-button-new", + run: () => {}, // it's a check + } +]); + +function addTwoAnswers() { + return [ + { + content: "Add the first answer", + trigger: "div[name=suggested_answer_ids] .o_field_x2many_list_row_add a", + in_modal: true, + }, { + trigger: 'tr.o_selected_row div[name=value] input', + run: 'text Answer A', + in_modal: true, + }, { + content: "Add the second answer", + trigger: "div[name=suggested_answer_ids] .o_field_x2many_list_row_add a", + in_modal: true, + }, { + trigger: 'tr.o_selected_row div[name=value] input', + run: 'text Answer B', + in_modal: true, + } + ]; +} + +function saveAndNew() { + return [ + { + content: "Click Save & New", + trigger: "button.o_form_button_save_new", + in_modal: true, + }, { + content: "Wait for the dialog to render new question form", + // suggested_answer_ids required even though in_modal is specified... + trigger: "div[name=suggested_answer_ids] .o_list_table tbody tr:first-child:not(.o_data_row)", // empty answers list + in_modal: true, + run: () => {}, // it's a check + } + ]; +} + +function toggleIsConditional() { + return [ + ...changeTab("options"), + { + content: "Toggle is_conditional", + trigger: "div[name=is_conditional] input", + in_modal: true, + run: "click", + } + ]; +} + +function changeTab(tabName) { + // Currently, .modal-content is required even though "in_modal" + return [ + { + content: `Go to ${tabName} tab`, + trigger: `.modal-content a[name=${tabName}].nav-link`, + in_modal: true + }, { + content: `Wait for tab ${tabName} tab`, + trigger: `.modal-content a[name=${tabName}].nav-link.active`, + in_modal: true, + run: () => {}, // it's a check + } + ]; +} diff --git a/addons/survey/tests/__init__.py b/addons/survey/tests/__init__.py index fa4b64afb27..865529005cb 100644 --- a/addons/survey/tests/__init__.py +++ b/addons/survey/tests/__init__.py @@ -9,6 +9,7 @@ from . import test_certification_flow from . import test_survey_invite from . import test_survey_security from . import test_survey_randomize +from . import test_survey_ui_backend from . import test_survey_ui_certification from . import test_survey_ui_feedback from . import test_survey_ui_session diff --git a/addons/survey/tests/test_survey.py b/addons/survey/tests/test_survey.py index 9ecf4b8574e..6fd52b46960 100644 --- a/addons/survey/tests/test_survey.py +++ b/addons/survey/tests/test_survey.py @@ -296,3 +296,46 @@ class TestSurveyInternals(common.TestSurveyCommon): self.assertNotEqual(q_food_not_vegetarian_cloned.triggering_question_id.id, q_is_vegetarian.id) self.assertNotEqual(q_food_not_vegetarian_cloned.triggering_answer_id.id, q_is_vegetarian.suggested_answer_ids[1].id) + + @users('survey_manager') + def test_unlink_triggers(self): + # Create the survey questions + q_is_vegetarian_text = 'Are you vegetarian ?' + q_is_vegetarian = self._add_question( + self.page_0, q_is_vegetarian_text, 'multiple_choice', survey_id=self.survey.id, + sequence=100, labels=[{'value': 'Yes'}, {'value': 'No'}]) + q_food_vegetarian_text = 'Choose your green meal' + veggie_question = self._add_question( + self.page_0, q_food_vegetarian_text, 'multiple_choice', + is_conditional=True, sequence=101, + triggering_question_id=q_is_vegetarian.id, + triggering_answer_id=q_is_vegetarian.suggested_answer_ids[0].id, + survey_id=self.survey.id, + labels=[{'value': 'Vegetarian pizza'}, {'value': 'Vegetarian burger'}]) + q_food_not_vegetarian_text = 'Choose your meal' + not_veggie_question = self._add_question( + self.page_0, q_food_not_vegetarian_text, 'multiple_choice', + is_conditional=True, sequence=102, + triggering_question_id=q_is_vegetarian.id, + triggering_answer_id=q_is_vegetarian.suggested_answer_ids[1].id, + survey_id=self.survey.id, + labels=[{'value': 'Steak with french fries'}, {'value': 'Fish'}]) + + q_is_vegetarian.suggested_answer_ids[0].unlink() + + # Deleting answer Yes makes the following question always visible + self.assertEqual(veggie_question.is_conditional, False) + self.assertEqual(veggie_question.triggering_question_id.id, False) + self.assertEqual(veggie_question.triggering_answer_id.id, False) + + # But the other is still conditional + self.assertEqual(not_veggie_question.is_conditional, True) + self.assertEqual(not_veggie_question.triggering_question_id.id, q_is_vegetarian.id) + self.assertEqual(not_veggie_question.triggering_answer_id.id, q_is_vegetarian.suggested_answer_ids[0].id) + + q_is_vegetarian.unlink() + + # Now it will also be always visible + self.assertEqual(not_veggie_question.is_conditional, False) + self.assertEqual(not_veggie_question.triggering_question_id.id, False) + self.assertEqual(not_veggie_question.triggering_answer_id.id, False) diff --git a/addons/survey/tests/test_survey_ui_backend.py b/addons/survey/tests/test_survey_ui_backend.py new file mode 100644 index 00000000000..1a56aaacb78 --- /dev/null +++ b/addons/survey/tests/test_survey_ui_backend.py @@ -0,0 +1,11 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.tests import HttpCase, tagged + + +@tagged('post_install', '-at_install') +class TestUi(HttpCase): + + def test_tour_test_survey_form_triggers(self): + self.start_tour('/web', 'survey_tour_test_survey_form_triggers', login='admin') diff --git a/addons/survey/views/survey_question_views.xml b/addons/survey/views/survey_question_views.xml index 94259f8951e..51be3b49b6b 100644 --- a/addons/survey/views/survey_question_views.xml +++ b/addons/survey/views/survey_question_views.xml @@ -7,19 +7,23 @@ survey.question
+ + - +
@@ -115,12 +119,15 @@ ]}"> - - - + + + @@ -171,44 +178,64 @@ - - - - - - - - - - - + attrs="{'invisible': [('question_type', 'not in', ['char_box', 'numerical_box', 'date', 'datetime'])]}"/> +
Min/Max Limits
+
+ + + + + + + + + + + +
+ placeholder="Displayed when the answer entered is not valid."/> - - + attrs="{'invisible': [('question_type', 'not in', ['text_box', 'char_box', 'date', 'datetime', 'numerical_box'])]}" + placeholder="Help Participants know what to write"/> + + +

+ Conditional display is not available when questions are randomly picked. +

- + + + + @@ -271,7 +298,7 @@ survey.question tree,form - {'search_default_group_by_page': True} + {'search_default_group_by_page': True, 'show_survey_field': True} [('is_page', '=', False)]

diff --git a/addons/survey/views/survey_survey_views.xml b/addons/survey/views/survey_survey_views.xml index 7007f564c20..5d90287b0fb 100644 --- a/addons/survey/views/survey_survey_views.xml +++ b/addons/survey/views/survey_survey_views.xml @@ -65,7 +65,7 @@ - + @@ -76,11 +76,14 @@ - -