diff --git a/survey_record_generation/__manifest__.py b/survey_record_generation/__manifest__.py index 04c6cd0..32bee37 100644 --- a/survey_record_generation/__manifest__.py +++ b/survey_record_generation/__manifest__.py @@ -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", diff --git a/survey_record_generation/migrations/16.0.1.0.3/end-migration.py b/survey_record_generation/migrations/16.0.1.0.3/end-migration.py new file mode 100644 index 0000000..332860c --- /dev/null +++ b/survey_record_generation/migrations/16.0.1.0.3/end-migration.py @@ -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, + ) diff --git a/survey_record_generation/models/survey_user_input.py b/survey_record_generation/models/survey_user_input.py index f8e3930..9dff3f4 100644 --- a/survey_record_generation/models/survey_user_input.py +++ b/survey_record_generation/models/survey_user_input.py @@ -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 diff --git a/survey_record_generation/tests/__init__.py b/survey_record_generation/tests/__init__.py index 5b794a3..7982aab 100644 --- a/survey_record_generation/tests/__init__.py +++ b/survey_record_generation/tests/__init__.py @@ -1 +1,2 @@ from . import test_survey_record_creation +from . import test_end_migration_16_0_1_0_3 diff --git a/survey_record_generation/tests/test_end_migration_16_0_1_0_3.py b/survey_record_generation/tests/test_end_migration_16_0_1_0_3.py new file mode 100644 index 0000000..b0ba11b --- /dev/null +++ b/survey_record_generation/tests/test_end_migration_16_0_1_0_3.py @@ -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") diff --git a/survey_record_generation/tests/test_survey_record_creation.py b/survey_record_generation/tests/test_survey_record_creation.py index 37a4150..bcb304e 100644 --- a/survey_record_generation/tests/test_survey_record_creation.py +++ b/survey_record_generation/tests/test_survey_record_creation.py @@ -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)