From c983f8a5343ac623ebc9d6dbddc506db4079ee5e Mon Sep 17 00:00:00 2001 From: Florian Charlier Date: Wed, 2 Aug 2023 15:12:34 +0200 Subject: [PATCH] [IMP] survey: ensure scored can be passed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, it was possible to have users fill-in a scored survey with no way possible to pass it because no answer had any positive score obtainable. We keep here the check on sharing of the survey to avoid errors while configuring the survey, the same way it is already done when trying to share a survey without any question at all. Note that we are also adding this warning and the "This is a test survey" notice for survey_user and not only managers because users can also test their surveys... Task-3374592 closes odoo/odoo#130553 Signed-off-by: Stéphane Debauche (std) --- addons/survey/models/survey_survey.py | 21 ++++- addons/survey/tests/test_survey_invite.py | 79 +++++++++++++++---- .../views/survey_templates_management.xml | 10 ++- 3 files changed, 89 insertions(+), 21 deletions(-) diff --git a/addons/survey/models/survey_survey.py b/addons/survey/models/survey_survey.py index efda3b0e78d..01425202e30 100644 --- a/addons/survey/models/survey_survey.py +++ b/addons/survey/models/survey_survey.py @@ -121,8 +121,9 @@ class Survey(models.Model): ('no_scoring', 'No scoring'), ('scoring_with_answers', 'Scoring with answers at the end'), ('scoring_without_answers', 'Scoring without answers at the end')], - string="Scoring", required=True, store=True, readonly=False, compute="_compute_scoring_type", precompute=True) + string='Scoring', required=True, store=True, readonly=False, compute='_compute_scoring_type', precompute=True) scoring_success_min = fields.Float('Required Score (%)', default=80.0) + scoring_max_obtainable = fields.Float('Maximum obtainable score', compute='_compute_scoring_max_obtainable') # attendees context: attempts and time limitation is_attempts_limited = fields.Boolean('Limited number of attempts', help="Check this option if you want to limit the number of attempts per user", compute="_compute_is_attempts_limited", store=True, readonly=False) @@ -196,6 +197,19 @@ class Survey(models.Model): for survey in self.filtered(lambda survey: survey.background_image and survey.access_token): survey.background_image_url = "/survey/%s/get_background_image" % survey.access_token + @api.depends( + 'question_and_page_ids', + 'question_and_page_ids.suggested_answer_ids', + 'question_and_page_ids.suggested_answer_ids.answer_score', + ) + def _compute_scoring_max_obtainable(self): + for survey in self: + survey.scoring_max_obtainable = sum( + question.answer_score + or sum(answer.answer_score for answer in question.suggested_answer_ids if answer.answer_score > 0) + for question in survey.question_ids + ) + def _compute_users_can_signup(self): signup_allowed = self.env['res.users'].sudo()._get_signup_invitation_scope() == 'b2c' for survey in self: @@ -938,6 +952,11 @@ class Survey(models.Model): if not self.question_ids: raise UserError(_('You cannot send an invitation for a survey that has no questions.')) + # Ensure scored survey have a positive total score obtainable. + if self.scoring_type != 'no_scoring' and self.scoring_max_obtainable <= 0: + raise UserError(_("A scored survey needs at least one question that gives points.\n" + "Please check answers and their scores.")) + # Ensure that this survey has at least one section with question(s), if question layout is 'One page per section'. if self.questions_layout == 'page_per_section': if not self.page_ids: diff --git a/addons/survey/tests/test_survey_invite.py b/addons/survey/tests/test_survey_invite.py index 2d564a19aca..768184a50aa 100644 --- a/addons/survey/tests/test_survey_invite.py +++ b/addons/survey/tests/test_survey_invite.py @@ -5,7 +5,7 @@ from datetime import datetime from dateutil.relativedelta import relativedelta from lxml import etree -from odoo import fields +from odoo import fields, Command from odoo.addons.survey.tests import common from odoo.addons.mail.tests.common import MailCommon from odoo.exceptions import UserError @@ -37,26 +37,74 @@ class TestSurveyInvite(common.TestSurveyCommon, MailCommon): action = self.survey.action_send_survey() self.assertEqual(action['res_model'], 'survey.invite') - # Bad cases - surveys = [ - # 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, 'question_type': False, 'title': 'P0', 'sequence': 1})]}), - # closed - self.env['survey.survey'].with_user(self.survey_manager).create({ - 'title': 'S0', + bad_cases = [ + {}, # empty + { # no question + 'question_and_page_ids': [Command.create({'is_page': True, 'question_type': False, 'title': 'P0', 'sequence': 1})] + }, { + # scored without positive score obtainable + 'scoring_type': 'scoring_with_answers', + 'question_and_page_ids': [Command.create({'question_type': 'numerical_box', 'title': 'Q0', 'sequence': 1})], + }, { + # scored without positive score obtainable from simple choice + 'scoring_type': 'scoring_with_answers', + 'question_and_page_ids': [Command.create({ + 'question_type': 'simple_choice', + 'title': 'Q0', 'sequence': 1, + 'suggested_answer_ids': [ + Command.create({'value': '1', 'answer_score': 0}), + Command.create({'value': '2', 'answer_score': 0}), + ], + })], + }, { + # closed 'active': False, 'question_and_page_ids': [ - (0, 0, {'is_page': True, 'question_type': False, 'title': 'P0', 'sequence': 1}), - (0, 0, {'title': 'Q0', 'sequence': 2, 'question_type': 'text_box'}) + Command.create({'is_page': True, 'question_type': False, 'title': 'P0', 'sequence': 1}), + Command.create({'title': 'Q0', 'sequence': 2, 'question_type': 'text_box'}) ] - }) + }, ] - for survey in surveys: + good_cases = [ + { + # scored with positive score obtainable + 'scoring_type': 'scoring_with_answers', + 'question_and_page_ids': [ + Command.create({'question_type': 'numerical_box', 'title': 'Q0', 'sequence': 1, 'answer_score': 1}), + ], + }, { + # scored with positive score obtainable from simple choice + 'scoring_type': 'scoring_with_answers', + 'question_and_page_ids': [ + Command.create({ # not sufficient + 'question_type': 'simple_choice', + 'title': 'Q0', 'sequence': 1, + 'suggested_answer_ids': [ + Command.create({'value': '1', 'answer_score': 0}), + Command.create({'value': '2', 'answer_score': 0}), + ], + }), + Command.create({ # sufficient even if not 'is_correct' + 'question_type': 'simple_choice', + 'title': 'Q1', 'sequence': 2, + 'suggested_answer_ids': [ + Command.create({'value': '1', 'answer_score': 0}), + Command.create({'value': '2', 'answer_score': 1}), + ], + })], + }, + ] + surveys = self.env['survey.survey'].with_user(self.survey_manager).create([ + {'title': 'Test survey', **case} for case in bad_cases + good_cases + ]) + + for survey in surveys[:len(bad_cases)]: with self.assertRaises(UserError): survey.action_send_survey() + for survey in surveys[len(bad_cases):]: + survey.action_send_survey() + @users('survey_manager') def test_survey_invite(self): Answer = self.env['survey.user_input'] @@ -95,7 +143,6 @@ class TestSurveyInvite(common.TestSurveyCommon, MailCommon): self.assertEqual(invite_form.existing_text, 'The following customers have already received an invite: Caroline Customer.') - @users('survey_manager') def test_survey_invite_authentication_nosignup(self): Answer = self.env['survey.user_input'] @@ -121,7 +168,7 @@ class TestSurveyInvite(common.TestSurveyCommon, MailCommon): self.assertEqual(len(answers), 2) self.assertEqual( set(answers.mapped('email')), - set([self.user_emp.email, self.user_portal.email])) + {self.user_emp.email, self.user_portal.email}) self.assertEqual(answers.mapped('partner_id'), self.user_emp.partner_id | self.user_portal.partner_id) @users('survey_manager') diff --git a/addons/survey/views/survey_templates_management.xml b/addons/survey/views/survey_templates_management.xml index b9a216a4f90..4bcb2864fd8 100644 --- a/addons/survey/views/survey_templates_management.xml +++ b/addons/survey/views/survey_templates_management.xml @@ -96,10 +96,12 @@