From 48737a8ffc6ddc8a763d162851088bdb323314c1 Mon Sep 17 00:00:00 2001 From: Umesh Gupta Date: Mon, 13 Jun 2022 22:33:47 +0000 Subject: [PATCH] [FIX] survey*: avoid traceback for data/demo data surveys Currently, in demo data (or data) of some surveys, there are records of 'survey.question' having 'is_page' field value as true and 'question_type' is not defined. When question type is not defined, default value 'simple_choice' is assigned to the 'question_type' field. Now when we create a live session of that survey and go to the page, it tries to read the data for bar chart but data is not available (undefined) and the traceback is thrown. This happens because question types 'simple_choice' and 'multiple_choice') are expected to load a bar chart. It used to work before commit[1] where default 'question_type' value was being set to 'text_box' (so attempt to load bar chart was not made). This commit improves the behavior by adapting data/demo data/test cases and by adding a python constraint that prevents question creation of page type if there is a question type set, and compute `question_type` based on `is_page` field. Now that we have python constraint and a compute method, we no longer need the `default_get` method, so it is also removed. commit[1] - https://github.com/odoo/odoo/commit/1fb99d79b1567d4c150dc07f309ce54a3beea8e3 taskID-2841582 closes odoo/odoo#93414 Related: odoo/enterprise#28305 Related: odoo/upgrade#3600 Signed-off-by: Thibault Delavallee (tde) --- .../data/survey_demo.xml | 1 + addons/survey/models/survey_question.py | 27 ++++++++++++------- addons/survey/tests/common.py | 1 + .../test_survey_compute_pages_questions.py | 2 ++ addons/survey/tests/test_survey_flow.py | 2 ++ .../tests/test_survey_flow_with_conditions.py | 1 + addons/survey/tests/test_survey_invite.py | 4 +-- addons/survey/tests/test_survey_randomize.py | 3 +++ addons/survey/tests/test_survey_security.py | 14 +++++----- .../tests/test_ui_wslides.py | 1 + .../data/survey_demo.xml | 1 + 11 files changed, 38 insertions(+), 19 deletions(-) diff --git a/addons/hr_recruitment_survey/data/survey_demo.xml b/addons/hr_recruitment_survey/data/survey_demo.xml index 83a76d62f61..515ee74ccd5 100644 --- a/addons/hr_recruitment_survey/data/survey_demo.xml +++ b/addons/hr_recruitment_survey/data/survey_demo.xml @@ -19,6 +19,7 @@ About you + 1

Please fill information about you: who you are, what are your education, experience, and activities. diff --git a/addons/survey/models/survey_question.py b/addons/survey/models/survey_question.py index 6b368527170..0d74c108285 100644 --- a/addons/survey/models/survey_question.py +++ b/addons/survey/models/survey_question.py @@ -45,13 +45,6 @@ class SurveyQuestion(models.Model): _rec_name = 'title' _order = 'sequence,id' - @api.model - def default_get(self, fields): - defaults = super(SurveyQuestion, self).default_get(fields) - if (not fields or 'question_type' in fields): - defaults['question_type'] = False if defaults.get('is_page') else 'simple_choice' - return defaults - # question generic data title = fields.Char('Title', required=True, translate=True) description = fields.Html( @@ -169,6 +162,20 @@ class SurveyQuestion(models.Model): 'All "Is a scored question = True" and "Question Type: Date" questions need an answer') ] + # ------------------------------------------------------------------------- + # CONSTRAINT METHODS + # ------------------------------------------------------------------------- + + @api.constrains("is_page") + def _check_question_type_for_pages(self): + invalid_pages = self.filtered(lambda question: question.is_page and question.question_type) + if invalid_pages: + raise ValidationError(_("Question type should be empty for these pages: %s", ', '.join(invalid_pages.mapped('title')))) + + # ------------------------------------------------------------------------- + # COMPUTE METHODS + # ------------------------------------------------------------------------- + @api.depends('question_type') def _compute_question_placeholder(self): for question in self: @@ -206,9 +213,9 @@ class SurveyQuestion(models.Model): @api.depends('is_page') def _compute_question_type(self): - for question in self: - if not question.question_type or question.is_page: - question.question_type = False + pages = self.filtered(lambda question: question.is_page) + pages.question_type = False + (self - pages).filtered(lambda question: not question.question_type).question_type = 'simple_choice' @api.depends('survey_id.question_and_page_ids.is_page', 'survey_id.question_and_page_ids.sequence') def _compute_question_ids(self): diff --git a/addons/survey/tests/common.py b/addons/survey/tests/common.py index 01db7d8843e..67687d5d336 100644 --- a/addons/survey/tests/common.py +++ b/addons/survey/tests/common.py @@ -300,6 +300,7 @@ class TestSurveyCommon(SurveyCase): 'survey_id': self.survey.id, 'sequence': 1, 'is_page': True, + 'question_type': False, }) self.question_ft = self.env['survey.question'].with_user(self.survey_manager).create({ 'title': 'Test Free Text', diff --git a/addons/survey/tests/test_survey_compute_pages_questions.py b/addons/survey/tests/test_survey_compute_pages_questions.py index 335683d445f..7ddb3a38f7e 100644 --- a/addons/survey/tests/test_survey_compute_pages_questions.py +++ b/addons/survey/tests/test_survey_compute_pages_questions.py @@ -13,6 +13,7 @@ class TestSurveyComputePagesQuestions(common.TestSurveyCommon): page_0 = self.env['survey.question'].create({ 'is_page': True, + 'question_type': False, 'sequence': 1, 'title': 'P1', 'survey_id': survey.id @@ -25,6 +26,7 @@ class TestSurveyComputePagesQuestions(common.TestSurveyCommon): page_1 = self.env['survey.question'].create({ 'is_page': True, + 'question_type': False, 'sequence': 7, 'title': 'P2', 'survey_id': survey.id, diff --git a/addons/survey/tests/test_survey_flow.py b/addons/survey/tests/test_survey_flow.py index d09f0d74e15..d13c2187fe9 100644 --- a/addons/survey/tests/test_survey_flow.py +++ b/addons/survey/tests/test_survey_flow.py @@ -31,6 +31,7 @@ class TestSurveyFlow(common.TestSurveyCommon, HttpCase): # First page is about customer data page_0 = self.env['survey.question'].create({ 'is_page': True, + 'question_type': False, 'sequence': 1, 'title': 'Page1: Your Data', 'survey_id': survey.id, @@ -47,6 +48,7 @@ class TestSurveyFlow(common.TestSurveyCommon, HttpCase): # Second page is about tarte al djotte page_1 = self.env['survey.question'].create({ 'is_page': True, + 'question_type': False, 'sequence': 4, 'title': 'Page2: Tarte Al Djotte', 'survey_id': survey.id, diff --git a/addons/survey/tests/test_survey_flow_with_conditions.py b/addons/survey/tests/test_survey_flow_with_conditions.py index 494e064e84d..374a80c5693 100644 --- a/addons/survey/tests/test_survey_flow_with_conditions.py +++ b/addons/survey/tests/test_survey_flow_with_conditions.py @@ -23,6 +23,7 @@ class TestSurveyFlowWithConditions(common.TestSurveyCommon, HttpCase): 'survey_id': survey.id, 'sequence': 1, 'is_page': True, + 'question_type': False, }) q01 = self._add_question( diff --git a/addons/survey/tests/test_survey_invite.py b/addons/survey/tests/test_survey_invite.py index 0208a8a5b0b..6f71b2b29e1 100644 --- a/addons/survey/tests/test_survey_invite.py +++ b/addons/survey/tests/test_survey_invite.py @@ -30,13 +30,13 @@ class TestSurveyInvite(common.TestSurveyCommon): # no page self.env['survey.survey'].create({'title': 'Test survey'}), # no questions - self.env['survey.survey'].create({'title': 'Test survey', 'question_and_page_ids': [(0, 0, {'is_page': True, 'title': 'P0', 'sequence': 1})]}), + self.env['survey.survey'].create({'title': 'Test survey', 'question_and_page_ids': [(0, 0, {'is_page': True, 'question_type': False, 'title': 'P0', 'sequence': 1})]}), # closed self.env['survey.survey'].with_user(self.survey_manager).create({ 'title': 'S0', 'active': False, 'question_and_page_ids': [ - (0, 0, {'is_page': True, 'title': 'P0', 'sequence': 1}), + (0, 0, {'is_page': True, 'question_type': False, 'title': 'P0', 'sequence': 1}), (0, 0, {'title': 'Q0', 'sequence': 2, 'question_type': 'text_box'}) ] }) diff --git a/addons/survey/tests/test_survey_randomize.py b/addons/survey/tests/test_survey_randomize.py index fa926b1895c..0af3fde45dc 100644 --- a/addons/survey/tests/test_survey_randomize.py +++ b/addons/survey/tests/test_survey_randomize.py @@ -12,6 +12,7 @@ class TestSurveyRandomize(TransactionCase): page_1 = Question.create({ 'title': 'Page 1', 'is_page': True, + 'question_type': False, 'sequence': 1, 'random_questions_count': 3 }) @@ -21,6 +22,7 @@ class TestSurveyRandomize(TransactionCase): page_2 = Question.create({ 'title': 'Page 2', 'is_page': True, + 'question_type': False, 'sequence': 100, 'random_questions_count': 5 }) @@ -30,6 +32,7 @@ class TestSurveyRandomize(TransactionCase): page_3 = Question.create({ 'title': 'Page 2', 'is_page': True, + 'question_type': False, 'sequence': 1000, 'random_questions_count': 4 }) diff --git a/addons/survey/tests/test_survey_security.py b/addons/survey/tests/test_survey_security.py index 0696d15810e..517d4f43501 100644 --- a/addons/survey/tests/test_survey_security.py +++ b/addons/survey/tests/test_survey_security.py @@ -27,7 +27,7 @@ class TestAccess(common.TestSurveyCommon): with self.assertRaises(AccessError): self.env['survey.survey'].create({'title': 'Test Survey 2'}) with self.assertRaises(AccessError): - self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'survey_id': self.survey.id}) + self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'question_type': False, 'survey_id': self.survey.id}) with self.assertRaises(AccessError): self.env['survey.question'].create({'title': 'My Question', 'sequence': 1, 'page_id': self.page_0.id}) @@ -60,7 +60,7 @@ class TestAccess(common.TestSurveyCommon): with self.assertRaises(AccessError): self.env['survey.survey'].create({'title': 'Test Survey 2'}) with self.assertRaises(AccessError): - self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'survey_id': self.survey.id}) + self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'question_type': False, 'survey_id': self.survey.id}) with self.assertRaises(AccessError): self.env['survey.question'].create({'title': 'My Question', 'sequence': 1, 'page_id': self.page_0.id}) @@ -93,7 +93,7 @@ class TestAccess(common.TestSurveyCommon): with self.assertRaises(AccessError): self.env['survey.survey'].create({'title': 'Test Survey 2'}) with self.assertRaises(AccessError): - self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'survey_id': self.survey.id}) + self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'question_type': False, 'survey_id': self.survey.id}) with self.assertRaises(AccessError): self.env['survey.question'].create({'title': 'My Question', 'sequence': 1, 'page_id': self.page_0.id}) @@ -123,7 +123,7 @@ class TestAccess(common.TestSurveyCommon): def test_access_survey_survey_manager(self): # Create: all survey = self.env['survey.survey'].create({'title': 'Test Survey 2'}) - self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'survey_id': survey.id}) + self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'question_type': False, 'survey_id': survey.id}) self.env['survey.question'].create({'title': 'My Question', 'sequence': 1, 'survey_id': survey.id}) # Read: all @@ -142,7 +142,7 @@ class TestAccess(common.TestSurveyCommon): def test_access_survey_survey_user(self): # Create: own only survey = self.env['survey.survey'].create({'title': 'Test Survey 2'}) - self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'survey_id': survey.id}) + self.env['survey.question'].create({'title': 'My Page', 'sequence': 0, 'is_page': True, 'question_type': False, 'survey_id': survey.id}) self.env['survey.question'].create({'title': 'My Question', 'sequence': 1, 'survey_id': survey.id}) # Read: all @@ -251,7 +251,7 @@ class TestAccess(common.TestSurveyCommon): @users('survey_user') def test_access_answers_survey_user(self): survey_own = self.env['survey.survey'].create({'title': 'Other'}) - self.env['survey.question'].create({'title': 'Other', 'sequence': 0, 'is_page': True, 'survey_id': survey_own.id}) + self.env['survey.question'].create({'title': 'Other', 'sequence': 0, 'is_page': True, 'question_type': False, 'survey_id': survey_own.id}) question_own = self.env['survey.question'].create({'title': 'Other Question', 'sequence': 1, 'survey_id': survey_own.id}) # Create: own survey only @@ -294,7 +294,7 @@ class TestAccess(common.TestSurveyCommon): admin = self.env.ref('base.user_admin') with self.with_user(admin.login): survey_other = self.env['survey.survey'].create({'title': 'Other'}) - self.env['survey.question'].create({'title': 'Other', 'sequence': 0, 'is_page': True, 'survey_id': survey_other.id}) + self.env['survey.question'].create({'title': 'Other', 'sequence': 0, 'is_page': True, 'question_type': False, 'survey_id': survey_other.id}) question_other = self.env['survey.question'].create({'title': 'Other Question', 'sequence': 1, 'survey_id': survey_other.id}) self.assertEqual(survey_other.create_uid, admin) self.assertEqual(question_other.create_uid, admin) diff --git a/addons/test_website_slides_full/tests/test_ui_wslides.py b/addons/test_website_slides_full/tests/test_ui_wslides.py index bd8ba5e21aa..ccb77732f1d 100644 --- a/addons/test_website_slides_full/tests/test_ui_wslides.py +++ b/addons/test_website_slides_full/tests/test_ui_wslides.py @@ -70,6 +70,7 @@ class TestUi(TestUICommon): 'title': 'Furniture', 'sequence': 1, 'is_page': True, + 'question_type': False, 'description': "<p>Test your furniture knowledge!</p>", }), (0, 0, { 'title': 'What type of wood is the best for furniture?', diff --git a/addons/website_slides_survey/data/survey_demo.xml b/addons/website_slides_survey/data/survey_demo.xml index 41598dbedbd..d59ea0e910d 100644 --- a/addons/website_slides_survey/data/survey_demo.xml +++ b/addons/website_slides_survey/data/survey_demo.xml @@ -22,6 +22,7 @@ 1 + <p>Test your furniture knowledge!</p>