From 78c0257ae46b27be576c0b98fc143e2a51ff8cd1 Mon Sep 17 00:00:00 2001 From: std-odoo Date: Fri, 14 Jan 2022 12:59:19 +0000 Subject: [PATCH] [FIX] crm: sync the email / phone with the partner Bug === If we 1. Open a lead with an email but without partner 2. Set a partner without email on the lead 3. The warning "The email will be propagated" is visible 4. When saving the form, the email is not propagated even if the warning message was visible Solution ======== The reason is that, as the email was not changed, the inverse method of this field was not called and so the email was not propagated. The best solution would be to use "force_save" on those fields. But this feature only works on readonly fields. So, we simulate a real "force_save" on the email / phone, directly in JS. That way the inverse will be called, and if necessary, the email / phone will be propagated. Task-2704904 closes odoo/odoo#84618 X-original-commit: 1d548c7eadcd25b97856c4769759358f86e00640 Signed-off-by: Thibault Delavallee (tde) Co-authored-by: flch-odoo --- addons/crm/static/src/js/crm_form.js | 34 ++++++++- .../tours/crm_email_and_phone_propagation.js | 68 +++++++++++++++++ addons/crm/tests/test_crm_ui.py | 75 ++++++++++++++++++- addons/crm/views/assets.xml | 23 ++++++ 4 files changed, 195 insertions(+), 5 deletions(-) create mode 100644 addons/crm/static/tests/tours/crm_email_and_phone_propagation.js create mode 100644 addons/crm/views/assets.xml diff --git a/addons/crm/static/src/js/crm_form.js b/addons/crm/static/src/js/crm_form.js index 57f3eef9b03..e1c6c237dd1 100644 --- a/addons/crm/static/src/js/crm_form.js +++ b/addons/crm/static/src/js/crm_form.js @@ -18,9 +18,41 @@ * We check if the stage_id field was altered and if we need to display a rainbowman * message. * + * This method will also simulate a real "force_save" on the email and phone + * when needed. The "force_save" attribute only works on readonly field. For our + * use case, we need to write the email and the phone even if the user didn't + * change them, to synchronize those values with the partner (so the email / phone + * inverse method can be called). + * + * We base this synchronization on the value of "partner_phone_update" + * and "partner_phone_update", which are computed fields that hold a value + * whenever we need to synch. + * * @override */ - saveRecord: function () { + saveRecord: function (recordID, options) { + recordID = recordID || this.handle; + const localData = this.model.localData[recordID]; + const changes = localData._changes || {}; + + const needsSynchronizationEmail = changes.partner_email_update === undefined + ? localData.data.partner_email_update // original value + : changes.partner_email_update; // new value + + const needsSynchronizationPhone = changes.partner_phone_update === undefined + ? localData.data.partner_phone_update // original value + : changes.partner_phone_update; // new value + + if (needsSynchronizationEmail && changes.email_from === undefined && localData.data.email_from) { + changes.email_from = localData.data.email_from; + } + if (needsSynchronizationPhone && changes.phone === undefined && localData.data.phone) { + changes.phone = localData.data.phone; + } + if (!localData._changes && Object.keys(changes).length) { + localData._changes = changes; + } + return this._super(...arguments).then((modifiedFields) => { if (modifiedFields.indexOf('stage_id') !== -1) { this._checkRainbowmanMessage(this.renderer.state.res_id) diff --git a/addons/crm/static/tests/tours/crm_email_and_phone_propagation.js b/addons/crm/static/tests/tours/crm_email_and_phone_propagation.js new file mode 100644 index 00000000000..6c448787973 --- /dev/null +++ b/addons/crm/static/tests/tours/crm_email_and_phone_propagation.js @@ -0,0 +1,68 @@ +odoo.define('crm.crm_email_and_phone_propagation', function (require) { + 'use strict'; + + const tour = require('web_tour.tour'); + + tour.register('crm_email_and_phone_propagation_edit_save', { + test: true, + url: '/web', + }, [ + tour.stepUtils.showAppsMenuItem(), + { + trigger: '.o_app[data-menu-xmlid="crm.crm_menu_root"]', + content: 'open crm app', + }, { + trigger: '.o_kanban_record .o_kanban_record_title span:contains(Test Lead Propagation)', + content: 'Open the first lead', + run: 'click', + }, { + trigger: '.o_form_button_edit', + extra_trigger: '.o_lead_opportunity_form.o_form_readonly', + content: 'Edit the lead', + run: 'click', + }, { + trigger: '.o_form_button_save', + extra_trigger: '.o_form_editable input[name="email_from"]', + content: 'Save the lead', + run: 'click', + }, { + trigger: '.o_form_readonly', + }, + ]); + + tour.register('crm_email_and_phone_propagation_remove_email_and_phone', { + test: true, + url: '/web', + }, [ + tour.stepUtils.showAppsMenuItem(), + { + trigger: '.o_app[data-menu-xmlid="crm.crm_menu_root"]', + content: 'open crm app', + }, { + trigger: '.o_kanban_record .o_kanban_record_title span:contains(Test Lead Propagation)', + content: 'Open the first lead', + run: 'click', + }, { + trigger: '.o_form_button_edit', + content: 'Edit the lead', + run: 'click', + }, { + trigger: '.o_form_editable input[name="email_from"]', + extra_trigger: '.o_form_editable input[name="phone"]', + content: 'Remove the email and the phone', + run: function () { + $('input[name="email_from"]').val('').trigger("change"); + $('input[name="phone"]').val('').trigger("change"); + }, + }, { + trigger: '.o_form_button_save', + // wait the the warning message to be visible + extra_trigger: '.o_form_sheet_bg .fa-exclamation-triangle:not(.o_invisible_modifier)', + content: 'Save the lead', + run: 'click', + }, { + trigger: '.o_form_readonly', + }, + ]); + +}); diff --git a/addons/crm/tests/test_crm_ui.py b/addons/crm/tests/test_crm_ui.py index 4cca0e9fb73..a67336a6bdd 100644 --- a/addons/crm/tests/test_crm_ui.py +++ b/addons/crm/tests/test_crm_ui.py @@ -28,8 +28,75 @@ class TestUi(HttpCase): def test_03_crm_tour_forecast(self): self.start_tour("/web", 'crm_forecast', login="admin") -class TestCRMLeadMisc(TestCrmCommon): + def test_email_and_phone_propagation_edit_save(self): + """Test the propagation of the email / phone on the partner. - @users('user_sales_leads') - def test_team_my_pipeline(self): - action = self.env['crm.team'].action_your_pipeline() + If the partner has no email but the lead has one, it should be propagated + if we edit and save the lead form. + """ + self.env['crm.lead'].search([]).unlink() + user_admin = self.env['res.users'].search([('login', '=', 'admin')]) + + partner = self.env['res.partner'].create({'name': 'Test Partner'}) + lead = self.env['crm.lead'].create({ + 'name': 'Test Lead Propagation', + 'type': 'opportunity', + 'user_id': user_admin.id, + 'partner_id': partner.id, + 'email_from': 'test@example.com', + 'phone': '+32 494 44 44 44', + }) + partner.email = False + partner.phone = False + + # Check initial state + self.assertFalse(partner.email) + self.assertFalse(partner.phone) + self.assertEqual(lead.email_from, 'test@example.com') + self.assertEqual(lead.phone, '+32 494 44 44 44') + + self.assertTrue(lead.partner_email_update) + self.assertTrue(lead.partner_phone_update) + + self.start_tour('/web', 'crm_email_and_phone_propagation_edit_save', login='admin') + + self.assertEqual(lead.email_from, 'test@example.com', 'Should not have changed the lead email') + self.assertEqual(lead.phone, '+32 494 44 44 44', 'Should not have changed the lead phone') + self.assertEqual(partner.email, 'test@example.com', 'Should have propagated the lead email on the partner') + self.assertEqual(partner.phone, '+32 494 44 44 44', 'Should have propagated the lead phone on the partner') + + def test_email_and_phone_propagation_remove_email_and_phone(self): + """Test the propagation of the email / phone on the partner. + + If we remove the email and phone on the lead, it should be removed on the + partner. This test check that we correctly detect field values changes in JS + (aka undefined VS falsy). + """ + self.env['crm.lead'].search([]).unlink() + user_admin = self.env['res.users'].search([('login', '=', 'admin')]) + + partner = self.env['res.partner'].create({'name': 'Test Partner'}) + lead = self.env['crm.lead'].create({ + 'name': 'Test Lead Propagation', + 'type': 'opportunity', + 'user_id': user_admin.id, + 'partner_id': partner.id, + 'email_from': 'test@example.com', + 'phone': '+32 494 44 44 44', + }) + + # Check initial state + self.assertEqual(partner.email, 'test@example.com') + self.assertEqual(lead.phone, '+32 494 44 44 44') + self.assertEqual(lead.email_from, 'test@example.com') + self.assertEqual(lead.phone, '+32 494 44 44 44') + + self.assertFalse(lead.partner_email_update) + self.assertFalse(lead.partner_phone_update) + + self.start_tour('/web', 'crm_email_and_phone_propagation_remove_email_and_phone', login='admin') + + self.assertFalse(lead.email_from, 'Should have removed the email') + self.assertFalse(lead.phone, 'Should have removed the phone') + self.assertFalse(partner.email, 'Should have removed the email') + self.assertFalse(partner.phone, 'Should have removed the phone') diff --git a/addons/crm/views/assets.xml b/addons/crm/views/assets.xml new file mode 100644 index 00000000000..182676289b7 --- /dev/null +++ b/addons/crm/views/assets.xml @@ -0,0 +1,23 @@ + + + + + +