[FIX] survey_record_generation: partner_id/email on survey.user_input could be wrong
Some checks failed
pre-commit / pre-commit (pull_request) Failing after 1m27s
Some checks failed
pre-commit / pre-commit (pull_request) Failing after 1m27s
New version : 16.0.1.0.3 _mark_done() only set partner_id/email when it created a *new* res.partner. If the answer already had a partner_id/email (e.g. inherited from the Odoo user logged in when the /survey/start link was opened), that value was kept even though the record creation matched or created a different, correct contact from the participant's own answers. Now we always sync partner_id/email to the res.partner actually matched or created from the participant's answers. Add a migration to relink partner_id/email on existing done submissions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -11,7 +11,7 @@ Allow to create record of any model when sending the form :
|
||||
* Associate question with fields
|
||||
* For x2m fields : Associate values to questions
|
||||
""",
|
||||
"version": "16.0.1.0.2",
|
||||
"version": "16.0.1.0.3",
|
||||
"license": "AGPL-3",
|
||||
"author": "Elabore",
|
||||
"website": "https://www.elabore.coop",
|
||||
|
||||
159
survey_record_generation/migrations/16.0.1.0.3/end-migration.py
Normal file
159
survey_record_generation/migrations/16.0.1.0.3/end-migration.py
Normal file
@@ -0,0 +1,159 @@
|
||||
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
|
||||
"""Relink survey.user_input.partner_id/email to the contact that actually
|
||||
matches each participant's own answers.
|
||||
|
||||
Before this version, survey.user_input.partner_id/email were only updated
|
||||
when _mark_done() created a *new* res.partner. If the answer already had a
|
||||
partner_id/email (e.g. inherited from the Odoo user who was logged in when
|
||||
the /survey/start link was opened, or from some other unidentified cause),
|
||||
_mark_done() silently kept that value even when the survey_record_creation
|
||||
config found or created a different, correct contact from the participant's
|
||||
own answers. This backfill re-runs that resolution for every already-done
|
||||
answer and fixes partner_id/email accordingly.
|
||||
|
||||
It never re-triggers the other survey.user_input._mark_done() overrides
|
||||
(crm lead / event registration / notification modules, ...): it only calls
|
||||
the res.partner-matching helpers directly, so it can't create duplicate
|
||||
leads, registrations, etc. for historical submissions.
|
||||
|
||||
A res.partner is expected to already exist for every done submission (it
|
||||
was necessarily found or created the first time _mark_done() ran), so this
|
||||
backfill never creates one. It first looks for the survey.generated.record
|
||||
row _mark_done() logged when it created that partner, and falls back to
|
||||
re-deriving the match through find_existing_record()/
|
||||
find_duplicate_if_there_are_fields_with_unicity_check() when there is no
|
||||
such row (existing partner matched instead of created) or it points to a
|
||||
partner that has since been deleted (e.g. merged into another contact). If
|
||||
neither approach finds anything, the record is left untouched and logged
|
||||
for manual review instead.
|
||||
"""
|
||||
import logging
|
||||
|
||||
from odoo import SUPERUSER_ID, api
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def migrate(cr, version):
|
||||
env = api.Environment(cr, SUPERUSER_ID, {})
|
||||
user_input_model = env["survey.user_input"]
|
||||
|
||||
surveys_with_partner_creation = env["survey.record.creation"].search(
|
||||
[("model_id.model", "=", "res.partner")]
|
||||
).survey_id
|
||||
|
||||
user_inputs = user_input_model.search(
|
||||
[
|
||||
("survey_id", "in", surveys_with_partner_creation.ids),
|
||||
("state", "=", "done"),
|
||||
]
|
||||
)
|
||||
|
||||
_logger.info(
|
||||
"survey_record_generation: relinking partner_id/email on %d done "
|
||||
"survey.user_input records",
|
||||
len(user_inputs),
|
||||
)
|
||||
|
||||
fixed_count = 0
|
||||
not_found_count = 0
|
||||
|
||||
for user_input in user_inputs:
|
||||
record_creations = user_input.survey_id.survey_record_creation_ids.filtered(
|
||||
lambda rc: rc.model_id.model == "res.partner"
|
||||
).sorted("sequence")
|
||||
|
||||
record_creation = record_creations[:1]
|
||||
if not record_creation:
|
||||
continue
|
||||
|
||||
# 1) Prefer the res.partner this very submission created, if any
|
||||
# (the first one, by id, in the rare case there is more than one):
|
||||
# it's the exact record _mark_done() produced for this answer, no
|
||||
# guessing involved. This is also the only option for surveys whose
|
||||
# record creation has neither update_existing_records nor any
|
||||
# unicity_check field configured, since find_existing_record()/
|
||||
# find_duplicate...() can then never find anything (nothing to
|
||||
# search on).
|
||||
record = False
|
||||
for generated in user_input.generated_record_ids.sorted("id"):
|
||||
if (
|
||||
generated.survey_record_creation_id == record_creation
|
||||
and generated.created_record_id
|
||||
and generated.created_record_id._name == "res.partner"
|
||||
and generated.created_record_id.exists()
|
||||
):
|
||||
# The referenced partner may have since been deleted (e.g.
|
||||
# merged into another contact): in that case it's not usable
|
||||
# and we fall through to the search-based lookup below.
|
||||
record = generated.created_record_id
|
||||
break
|
||||
|
||||
# 2) Otherwise, this submission matched an already-existing partner
|
||||
# instead of creating one (find_existing_record()/find_duplicate...()
|
||||
# branch of _mark_done()): re-derive it the same way.
|
||||
if not record:
|
||||
# Only compute the fields find_existing_record()/find_duplicate...()
|
||||
# actually read (the search field, and any unicity_check field),
|
||||
# not every field of the record creation: other fields (e.g. a
|
||||
# "record" reference to a model defined in a module that depends
|
||||
# on this one) may not be loadable yet at this point of the
|
||||
# upgrade, and are useless here anyway since this backfill never
|
||||
# writes to res.partner.
|
||||
needed_field_names = set()
|
||||
if (
|
||||
record_creation.update_existing_records
|
||||
and record_creation.field_to_retrieve_existing_records
|
||||
):
|
||||
needed_field_names.add(
|
||||
record_creation.field_to_retrieve_existing_records.name
|
||||
)
|
||||
unicity_field_values = record_creation.field_values_ids.filtered(
|
||||
lambda field_value: field_value.unicity_check
|
||||
)
|
||||
needed_field_names.update(unicity_field_values.mapped("field_id.name"))
|
||||
|
||||
vals = {}
|
||||
for field_value in record_creation.field_values_ids:
|
||||
if field_value.field_id.name not in needed_field_names:
|
||||
continue
|
||||
value, __ = user_input_model.get_value_based_on_value_origin(
|
||||
field_value=field_value,
|
||||
user_input=user_input,
|
||||
created_records={},
|
||||
model="res.partner",
|
||||
other_record_fields_to_update=[],
|
||||
)
|
||||
vals[field_value.field_id.name] = value
|
||||
|
||||
existing_record = user_input_model.find_existing_record(
|
||||
record_creation, vals
|
||||
)
|
||||
duplicate = (
|
||||
user_input_model.find_duplicate_if_there_are_fields_with_unicity_check(
|
||||
"res.partner", record_creation, vals
|
||||
)
|
||||
)
|
||||
record = duplicate or existing_record
|
||||
|
||||
if not record:
|
||||
_logger.warning(
|
||||
"survey_record_generation: no existing res.partner found for "
|
||||
"survey.user_input %s while backfilling partner_id/email, "
|
||||
"leaving it untouched",
|
||||
user_input.id,
|
||||
)
|
||||
not_found_count += 1
|
||||
continue
|
||||
|
||||
if user_input.partner_id != record or user_input.email != record.email:
|
||||
user_input.partner_id = record.id
|
||||
user_input.email = record.email
|
||||
fixed_count += 1
|
||||
|
||||
_logger.info(
|
||||
"survey_record_generation: fixed %d survey.user_input records "
|
||||
"(%d without a matching res.partner)",
|
||||
fixed_count,
|
||||
not_found_count,
|
||||
)
|
||||
@@ -39,6 +39,7 @@ class SurveyUserInput(models.Model):
|
||||
for user_input in self:
|
||||
created_records = {}
|
||||
other_record_fields_to_update: list[SurveyRecordCreationFieldValues] = []
|
||||
partner_linked_in_this_run = False
|
||||
|
||||
record_creation: SurveyRecordCreation
|
||||
for (
|
||||
@@ -85,8 +86,6 @@ class SurveyUserInput(models.Model):
|
||||
try:
|
||||
with self.env.cr.savepoint():
|
||||
record = self.env[model].create(vals)
|
||||
if model == "res.partner" and not self.partner_id:
|
||||
self.partner_id = record.id
|
||||
except Exception:
|
||||
# This a broad exception because it could be IntegrityError,
|
||||
# EmptyNamesError in case partner_firstname is installed etc...
|
||||
@@ -103,6 +102,17 @@ class SurveyUserInput(models.Model):
|
||||
}
|
||||
)
|
||||
|
||||
if model == "res.partner" and not partner_linked_in_this_run:
|
||||
# Always reflect the partner actually matched/created from
|
||||
# this participant's own answers, even if partner_id/email
|
||||
# were already set on the answer (e.g. inherited from the
|
||||
# logged-in user when the survey link was opened). Only the
|
||||
# first res.partner record creation of this run wins, in
|
||||
# case several are configured on the same survey.
|
||||
user_input.partner_id = record.id
|
||||
user_input.email = record.email
|
||||
partner_linked_in_this_run = True
|
||||
|
||||
created_records[record_creation.id] = record
|
||||
|
||||
# update linked record
|
||||
|
||||
@@ -1 +1,2 @@
|
||||
from . import test_survey_record_creation
|
||||
from . import test_end_migration_16_0_1_0_3
|
||||
|
||||
156
survey_record_generation/tests/test_end_migration_16_0_1_0_3.py
Normal file
156
survey_record_generation/tests/test_end_migration_16_0_1_0_3.py
Normal file
@@ -0,0 +1,156 @@
|
||||
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).
|
||||
import importlib.util
|
||||
import os
|
||||
|
||||
from odoo.addons.survey.tests.common import SurveyCase
|
||||
|
||||
|
||||
def _load_end_migration():
|
||||
migration_path = os.path.join(
|
||||
os.path.dirname(os.path.dirname(os.path.abspath(__file__))),
|
||||
"migrations",
|
||||
"16.0.1.0.3",
|
||||
"end-migration.py",
|
||||
)
|
||||
spec = importlib.util.spec_from_file_location(
|
||||
"survey_record_generation_end_migration_16_0_1_0_3", migration_path
|
||||
)
|
||||
module = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(module)
|
||||
return module
|
||||
|
||||
|
||||
class TestEndMigration160103(SurveyCase):
|
||||
"""Covers the use cases the 16.0.1.0.3 end-migration backfill must
|
||||
handle: relinking survey.user_input.partner_id/email to the contact
|
||||
that actually matches each participant's own answers, for submissions
|
||||
whose partner_id/email were left wrong before the fix in this version.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.migration = _load_end_migration()
|
||||
|
||||
self.survey = self.env["survey.survey"].create({"title": "Test Survey"})
|
||||
self.question_name = self._add_question(
|
||||
page=None,
|
||||
name="Name",
|
||||
qtype="char_box",
|
||||
survey_id=self.survey.id,
|
||||
sequence=1,
|
||||
)
|
||||
self.res_partner_model = self.env["ir.model"]._get("res.partner")
|
||||
self.survey_record_creation = self.env["survey.record.creation"].create(
|
||||
{
|
||||
"name": "Contact",
|
||||
"survey_id": self.survey.id,
|
||||
"model_id": self.res_partner_model.id,
|
||||
}
|
||||
)
|
||||
self.name_field = self.env["ir.model.fields"].search(
|
||||
[("model", "=", "res.partner"), ("name", "=", "name")]
|
||||
)
|
||||
self.env["survey.record.creation.field.values"].create(
|
||||
{
|
||||
"survey_record_creation_id": self.survey_record_creation.id,
|
||||
"survey_id": self.survey.id,
|
||||
"model_id": self.res_partner_model.id,
|
||||
"field_id": self.name_field.id,
|
||||
"value_origin": "question",
|
||||
"question_id": self.question_name.id,
|
||||
}
|
||||
)
|
||||
|
||||
def _submit_answer(self, name):
|
||||
answer = self._add_answer(survey=self.survey, partner=False, email=False)
|
||||
self._add_answer_line(
|
||||
question=self.question_name, answer=answer, answer_value=name
|
||||
)
|
||||
answer._mark_done()
|
||||
return answer
|
||||
|
||||
def _corrupt_with_wrong_partner(self, answer):
|
||||
# Simulate a pre-fix record: partner_id/email point to someone
|
||||
# unrelated to this participant's own answers.
|
||||
wrong_partner = self.env["res.partner"].create({"name": "Wrong Partner"})
|
||||
answer.write({"partner_id": wrong_partner.id, "email": "wrong@test.fr"})
|
||||
return wrong_partner
|
||||
|
||||
def test_migrate_uses_generated_record_when_available(self):
|
||||
# The submission created its own res.partner: the migration must
|
||||
# relink partner_id/email to that exact contact.
|
||||
answer = self._submit_answer("Jean")
|
||||
jean = self.env["res.partner"].search([("name", "=", "Jean")])
|
||||
self._corrupt_with_wrong_partner(answer)
|
||||
|
||||
self.migration.migrate(self.cr, "16.0.1.0.2")
|
||||
answer.invalidate_recordset()
|
||||
|
||||
self.assertEqual(answer.partner_id, jean)
|
||||
self.assertEqual(answer.email, jean.email)
|
||||
|
||||
def test_migrate_falls_back_to_search_when_no_generated_record(self):
|
||||
# The submission matched an already-existing partner instead of
|
||||
# creating one: no survey.generated.record row exists for it, so the
|
||||
# migration must fall back to re-deriving the match through search.
|
||||
jean = self.env["res.partner"].create(
|
||||
{"name": "Jean", "email": "jean@test.fr"}
|
||||
)
|
||||
self.survey_record_creation.write(
|
||||
{
|
||||
"update_existing_records": True,
|
||||
"field_to_retrieve_existing_records": self.name_field.id,
|
||||
}
|
||||
)
|
||||
|
||||
answer = self._submit_answer("Jean")
|
||||
self.assertFalse(answer.generated_record_ids)
|
||||
self._corrupt_with_wrong_partner(answer)
|
||||
|
||||
self.migration.migrate(self.cr, "16.0.1.0.2")
|
||||
answer.invalidate_recordset()
|
||||
|
||||
self.assertEqual(answer.partner_id, jean)
|
||||
self.assertEqual(answer.email, jean.email)
|
||||
|
||||
def test_migrate_falls_back_to_search_when_generated_partner_was_deleted(self):
|
||||
# The res.partner the submission created has since been deleted
|
||||
# (e.g. merged into another contact): the migration must not crash
|
||||
# on the stale reference and must fall back to search instead.
|
||||
self.survey_record_creation.write(
|
||||
{
|
||||
"update_existing_records": True,
|
||||
"field_to_retrieve_existing_records": self.name_field.id,
|
||||
}
|
||||
)
|
||||
|
||||
answer = self._submit_answer("Jean")
|
||||
first_jean = self.env["res.partner"].search([("name", "=", "Jean")])
|
||||
first_jean.unlink()
|
||||
|
||||
# A new contact with the same name takes over.
|
||||
second_jean = self.env["res.partner"].create(
|
||||
{"name": "Jean", "email": "jean2@test.fr"}
|
||||
)
|
||||
self._corrupt_with_wrong_partner(answer)
|
||||
|
||||
self.migration.migrate(self.cr, "16.0.1.0.2")
|
||||
answer.invalidate_recordset()
|
||||
|
||||
self.assertEqual(answer.partner_id, second_jean)
|
||||
self.assertEqual(answer.email, second_jean.email)
|
||||
|
||||
def test_migrate_leaves_answer_untouched_when_no_partner_can_be_found(self):
|
||||
# No survey.generated.record row (e.g. historical data predating
|
||||
# that log) and no way to search (neither update_existing_records
|
||||
# nor unicity_check configured): the migration must leave
|
||||
# partner_id/email as they are instead of guessing or crashing.
|
||||
answer = self._submit_answer("Jean")
|
||||
answer.generated_record_ids.unlink()
|
||||
wrong_partner = self._corrupt_with_wrong_partner(answer)
|
||||
|
||||
self.migration.migrate(self.cr, "16.0.1.0.2")
|
||||
answer.invalidate_recordset()
|
||||
|
||||
self.assertEqual(answer.partner_id, wrong_partner)
|
||||
self.assertEqual(answer.email, "wrong@test.fr")
|
||||
@@ -930,6 +930,71 @@ class TestSurveyRecordCreation(SurveyCase):
|
||||
partner = self.env["res.partner"].search([("name", "=", "Jean")])
|
||||
self.assertEqual(self.answer.partner_id, partner)
|
||||
|
||||
def test_partner_id_and_email_are_corrected_when_creating_a_new_partner(self):
|
||||
# A wrong partner_id/email is already set on the answer (e.g.
|
||||
# inherited from the Odoo user who was logged in when the
|
||||
# /survey/start link was opened). The record creation must still
|
||||
# fill partner_id/email with the contact actually created from the
|
||||
# participant's own answers, overriding that wrong value.
|
||||
wrong_partner = self.env["res.partner"].create({"name": "Wrong Partner"})
|
||||
|
||||
self.answer = self._add_answer(
|
||||
survey=self.survey, partner=wrong_partner, email="wrong@test.fr"
|
||||
)
|
||||
self._add_answer_line(
|
||||
question=self.question_name, answer=self.answer, answer_value="Jean"
|
||||
)
|
||||
self.answer._mark_done()
|
||||
|
||||
partner = self.env["res.partner"].search([("name", "=", "Jean")])
|
||||
self.assertEqual(self.answer.partner_id, partner)
|
||||
self.assertNotEqual(self.answer.partner_id, wrong_partner)
|
||||
self.assertEqual(self.answer.email, partner.email)
|
||||
|
||||
def test_partner_id_and_email_are_corrected_when_matching_an_existing_partner(self):
|
||||
# Same as above, but this time the record creation matches an
|
||||
# already-existing partner (via update_existing_records) instead of
|
||||
# creating a new one: partner_id/email must still be corrected to
|
||||
# that matched partner, not left as the wrong pre-existing value.
|
||||
jean = self.env["res.partner"].create({"name": "Jean", "email": "jean@test.fr"})
|
||||
wrong_partner = self.env["res.partner"].create({"name": "Wrong Partner"})
|
||||
|
||||
self.survey_record_creation.write(
|
||||
{
|
||||
"update_existing_records": True,
|
||||
"field_to_retrieve_existing_records": self.name_field.id,
|
||||
}
|
||||
)
|
||||
|
||||
self.answer = self._add_answer(
|
||||
survey=self.survey, partner=wrong_partner, email="wrong@test.fr"
|
||||
)
|
||||
self._add_answer_line(
|
||||
question=self.question_name, answer=self.answer, answer_value="Jean"
|
||||
)
|
||||
self.answer._mark_done()
|
||||
|
||||
self.assertEqual(self.answer.partner_id, jean)
|
||||
self.assertEqual(self.answer.email, jean.email)
|
||||
|
||||
def test_partner_id_and_email_are_corrected_when_matching_a_duplicate(self):
|
||||
# Same as above, but through the unicity_check/find_duplicate branch
|
||||
# instead of update_existing_records.
|
||||
self.name_survey_record_creation_field_values.unicity_check = True
|
||||
jean = self.env["res.partner"].create({"name": "Jean", "email": "jean@test.fr"})
|
||||
wrong_partner = self.env["res.partner"].create({"name": "Wrong Partner"})
|
||||
|
||||
self.answer = self._add_answer(
|
||||
survey=self.survey, partner=wrong_partner, email="wrong@test.fr"
|
||||
)
|
||||
self._add_answer_line(
|
||||
question=self.question_name, answer=self.answer, answer_value="Jean"
|
||||
)
|
||||
self.answer._mark_done()
|
||||
|
||||
self.assertEqual(self.answer.partner_id, jean)
|
||||
self.assertEqual(self.answer.email, jean.email)
|
||||
|
||||
def test_partner_id_in_survey_input_is_filled_up_by_first_contact_record_creation(self):
|
||||
# In this test, we verify that when creating several contacts with the same survey,
|
||||
# the 1st created contact is used to fill up survey_input.partner_id
|
||||
@@ -976,3 +1041,58 @@ class TestSurveyRecordCreation(SurveyCase):
|
||||
|
||||
partner = self.env["res.partner"].search([("name", "=", "Jean")])
|
||||
self.assertEqual(self.answer.partner_id, partner)
|
||||
|
||||
def test_partner_id_is_not_overridden_by_second_contact_record_creation(self):
|
||||
# When several res.partner record creations run on the same survey,
|
||||
# partner_id/email must be corrected to match the first contact
|
||||
# created, and the second contact created must not override that
|
||||
# choice, even though the answer started with a wrong
|
||||
# partner_id/email.
|
||||
self.second_question_name = self._add_question(
|
||||
page=None,
|
||||
name="Name of second person",
|
||||
qtype="char_box",
|
||||
survey_id=self.survey.id,
|
||||
sequence=1,
|
||||
)
|
||||
|
||||
self.second_contact_creation = self.env["survey.record.creation"].create(
|
||||
{
|
||||
"name": "Contact 2",
|
||||
"survey_id": self.survey.id,
|
||||
"model_id": self.res_partner_model.id,
|
||||
}
|
||||
)
|
||||
self.env["survey.record.creation.field.values"].create(
|
||||
{
|
||||
"survey_record_creation_id": self.second_contact_creation.id,
|
||||
"survey_id": self.survey.id,
|
||||
"model_id": self.res_partner_model.id,
|
||||
"field_id": self.name_field.id,
|
||||
"value_origin": "question",
|
||||
"question_id": self.second_question_name.id,
|
||||
}
|
||||
)
|
||||
|
||||
wrong_partner = self.env["res.partner"].create({"name": "Wrong Partner"})
|
||||
|
||||
self.answer = self._add_answer(
|
||||
survey=self.survey, partner=wrong_partner, email="wrong@test.fr"
|
||||
)
|
||||
self._add_answer_line(
|
||||
question=self.question_name,
|
||||
answer=self.answer,
|
||||
answer_value="Jean",
|
||||
)
|
||||
self._add_answer_line(
|
||||
question=self.second_question_name,
|
||||
answer=self.answer,
|
||||
answer_value="Jeanne",
|
||||
)
|
||||
self.answer._mark_done()
|
||||
|
||||
jean = self.env["res.partner"].search([("name", "=", "Jean")])
|
||||
jeanne = self.env["res.partner"].search([("name", "=", "Jeanne")])
|
||||
self.assertEqual(self.answer.partner_id, jean)
|
||||
self.assertNotEqual(self.answer.partner_id, jeanne)
|
||||
self.assertEqual(self.answer.email, jean.email)
|
||||
|
||||
Reference in New Issue
Block a user