pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[IMP] survey: enforce question triggering consistency

Browse Source 
Before this commit, the state of a question triggering fields could be
inconsistent/corrupted:

* If a question used as trigger for another question was deleted,
  `is_conditional` would stay `True` even as `triggering_question_id` would be
   `False`, which would eg. prevent saving the question edit form unless this
   is not manually fixed.
* If an answer used as trigger was deleted, the triggered question
  would never show on a survey session, but still appear as "triggered" and
  causing the same trouble on form edition.

In this commit, we
* update `unlink` methods to remove the triggering fields on triggering
  question/answer deletion
* add SQL constraint to ensure DB consistency (also improving the survey
cloning/copying behavior robustness)
* add a python test checking this

Task-2623482
Part of odoo#78705
See odoo/upgrade#3613

Part-of: odoo/odoo#78705
This commit is contained in:
Florian Charlier
2023-01-10 18:54:27 +01:00
parent 51b4e6d098
commit 91b306b5db
3 changed files with 78 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/survey/models/survey_question.py
+32 -2
View File
@@ -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",
@@ -160,7 +160,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.'),
]
# -------------------------------------------------------------------------
@@ -326,6 +332,20 @@ class SurveyQuestion(models.Model):
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
# ------------------------------------------------------------
@@ -626,3 +646,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()
addons/survey/models/survey_survey.py
+3 -1
View File
@@ -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
addons/survey/tests/test_survey.py
+43
View File
@@ -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)