[FIX] mail_plugin: avoid repeating company creation

Purpose
=======
Avoid creating / enriching multiple time the same company.

It can occur in many situation; when IAP do not return an email address
the the company, when the domain is blacklisted...

We also want to remove the field "iap_enrich_info" from the res.partner
model because this field can be heavy (JSON field) and the res.partner
model is used a lot.

Technical
=========
For this purpose, we create a new model <res.partner.iap> which will
store the IAP response and the requested domain.

So we can retrieve the previously enriched company regardless of the
partner's values

Task 2466653
See odoo/odoo/pull/68286
See odoo/upgrade/pull/2301

closes odoo/odoo#68286

Related: odoo/upgrade#2301
Related: odoo/enterprise#17426
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
std-odoo
2021-04-01 14:23:07 +00:00
parent f1ea45c402
commit ab179fb600
12 changed files with 526 additions and 29 deletions
@@ -51,8 +51,8 @@ class MailPluginController(mail_plugin.MailPluginController):
return leads
def _prepare_contact_values(self, partner):
contact_values = super(MailPluginController, self)._prepare_contact_values(partner)
def _get_contact_data(self, partner):
contact_values = super(MailPluginController, self)._get_contact_data(partner)
if not partner:
contact_values['leads'] = []
else:
+2
View File
@@ -15,6 +15,8 @@
],
'data': [
'views/mail_plugin_login.xml',
'views/res_partner_iap_views.xml',
'security/ir.model.access.csv',
],
'installable': True,
'application': False,
+45 -25
View File
@@ -44,12 +44,10 @@ class MailPluginController(http.Controller):
response = {'error': _('Contact has no valid email')}
return response
company_domain = tools.email_domain_extract(normalized_email)
company, enrichment_info = self._create_company_from_iap(company_domain)
company, enrichment_info = self._create_company_from_iap(normalized_email)
response['enrichment_info'] = enrichment_info
response['company'] = self._prepare_company_values(company)
response['company'] = self._get_company_data(company)
if company:
partner.write({'parent_id': company})
@@ -73,7 +71,7 @@ class MailPluginController(http.Controller):
if partner_id:
partner = request.env['res.partner'].browse(partner_id)
return self._prepare_contact_values(partner)
return self._get_contact_data(partner)
normalized_email = tools.email_normalize(email)
if not normalized_email:
@@ -84,12 +82,11 @@ class MailPluginController(http.Controller):
partner = request.env['res.partner'].search(['|', ('email', 'in', [normalized_email, email]),
('email_normalized', '=', normalized_email)], limit=1)
response = self._prepare_contact_values(partner)
response = self._get_contact_data(partner)
# if no partner is found in the database, we should also return an empty one having id = -1, otherwise older versions of
# plugin won't work
if not response['partner']:
sender_domain = tools.email_domain_extract(email)
response['partner'] = {
'id': -1,
'email': email,
@@ -98,9 +95,9 @@ class MailPluginController(http.Controller):
}
company = self._find_existing_company(normalized_email)
if not company: # create and enrich company
company, enrichment_info = self._create_company_from_iap(sender_domain)
company, enrichment_info = self._create_company_from_iap(normalized_email)
response['partner']['enrichment_info'] = enrichment_info
response['partner']['company'] = self._prepare_company_values(company)
response['partner']['company'] = self._get_company_data(company)
return response
@@ -132,7 +129,7 @@ class MailPluginController(http.Controller):
partners = request.env['res.partner'].search(filter_domain, limit=limit)
partners = [
self._prepare_partner_values(partner)
self._get_partner_data(partner)
for partner in partners
]
return {"partners": partners}
@@ -182,14 +179,22 @@ class MailPluginController(http.Controller):
enriched_data = {'enrichment_info': {'type': 'no_data', 'info': 'The enrichment API found no data for the email provided.'}}
return enriched_data
def _find_existing_company(self, normalized_email):
sender_domain = tools.email_domain_extract(normalized_email)
search = sender_domain if sender_domain not in iap_tools._MAIL_DOMAIN_BLACKLIST else normalized_email
return request.env['res.partner'].search([('is_company', '=', True), ('email_normalized', '=ilike', '%' + search)],
limit=1)
def _find_existing_company(self, email):
"""Find the company corresponding to the given domain and its IAP cache.
def _prepare_company_values(self, company):
:param email: Email of the company we search
:return: The partner corresponding to the company
"""
search = self._get_iap_search_term(email)
partner_iap = request.env["res.partner.iap"].sudo().search([("iap_search_domain", "=", search)], limit=1)
if partner_iap:
return partner_iap.partner_id
return request.env["res.partner"].search([("is_company", "=", True), ("email_normalized", "=ilike", "%" + search)], limit=1)
def _get_company_data(self, company):
if not company:
return {'id': -1}
@@ -205,8 +210,8 @@ class MailPluginController(http.Controller):
return company_values
def _create_company_from_iap(self, domain):
def _create_company_from_iap(self, email):
domain = tools.email_domain_extract(email)
iap_data = self._iap_enrich(domain)
if 'enrichment_info' in iap_data:
return None, iap_data['enrichment_info']
@@ -245,8 +250,13 @@ class MailPluginController(http.Controller):
if state:
new_company_info['state_id'] = state.id
new_company_info['iap_enrich_info'] = json.dumps(iap_data)
new_company_info.update({
'iap_search_domain': self._get_iap_search_term(email),
'iap_enrich_info': json.dumps(iap_data),
})
new_company = request.env['res.partner'].create(new_company_info)
new_company.message_post_with_view(
'iap_mail.enrich_company',
values=iap_data,
@@ -255,7 +265,7 @@ class MailPluginController(http.Controller):
return new_company, {'type': 'company_created'}
def _prepare_partner_values(self, partner):
def _get_partner_data(self, partner):
fields_list = ['id', 'name', 'email', 'phone', 'mobile']
@@ -267,19 +277,19 @@ class MailPluginController(http.Controller):
return partner_values
def _prepare_contact_values(self, partner):
def _get_contact_data(self, partner):
"""
method used to return partner related values, it can be overridden by other modules if extra information have to
be returned with the partner (e.g., leads, ...)
"""
if partner:
partner_response = self._prepare_partner_values(partner)
partner_response = self._get_partner_data(partner)
if partner.company_type == 'company':
partner_response['company'] = self._prepare_company_values(partner)
partner_response['company'] = self._get_company_data(partner)
elif partner.parent_id:
partner_response['company'] = self._prepare_company_values(partner.parent_id)
partner_response['company'] = self._get_company_data(partner.parent_id)
else:
partner_response['company'] = self._prepare_company_values(None)
partner_response['company'] = self._get_company_data(None)
else: # no partner found
partner_response = {}
@@ -291,3 +301,13 @@ class MailPluginController(http.Controller):
it can be overridden by sub modules in order to whitelist more models
"""
return ['res.partner']
def _get_iap_search_term(self, email):
"""Return the domain or the email depending if the domain is blacklisted or not.
So if the domain is blacklisted, we search based on the entire email address
(e.g. asbl@gmail.com). But if the domain is not blacklisted, we search based on
the domain (e.g. bob@sncb.be -> sncb.be)
"""
domain = tools.email_domain_extract(email)
return ("@" + domain) if domain not in iap_tools._MAIL_DOMAIN_BLACKLIST else email
+1
View File
@@ -3,3 +3,4 @@
from . import ir_http
from . import res_partner
from . import res_partner_iap
+63 -2
View File
@@ -1,8 +1,69 @@
# -*- coding: utf-8 -*-
from odoo import fields, models
from odoo import api, fields, models
class ResPartner(models.Model):
_inherit = 'res.partner'
iap_enrich_info = fields.Text('IAP Enrich Info', help='Stores additional info retrieved from IAP in JSON')
iap_enrich_info = fields.Text('IAP Enrich Info', help='IAP response stored as a JSON string',
compute='_compute_partner_iap_info')
iap_search_domain = fields.Char('Search Domain / Email',
compute='_compute_partner_iap_info')
def _compute_partner_iap_info(self):
partner_iaps = self.env['res.partner.iap'].sudo().search([('partner_id', 'in', self.ids)])
partner_iaps_per_partner = {
partner_iap.partner_id: partner_iap
for partner_iap in partner_iaps
}
for partner in self:
partner_iap = partner_iaps_per_partner.get(partner)
if partner_iap:
partner.iap_enrich_info = partner_iap.iap_enrich_info
partner.iap_search_domain = partner_iap.iap_search_domain
else:
partner.iap_enrich_info = False
partner.iap_search_domain = False
@api.model
def create(self, vals):
partner = super(ResPartner, self).create(vals)
if vals.get('iap_enrich_info') or vals.get('iap_search_domain'):
# Not done with inverse method so we do not need to search
# for existing <res.partner.iap>
self.env['res.partner.iap'].sudo().create({
'partner_id': partner.id,
'iap_enrich_info': vals.get('iap_enrich_info'),
'iap_search_domain': vals.get('iap_search_domain'),
})
return partner
def write(self, vals):
super(ResPartner, self).write(vals)
if 'iap_enrich_info' in vals or 'iap_search_domain' in vals:
# Not done with inverse method so we do need to search
# for existing <res.partner.iap> only once
partner_iaps = self.env['res.partner.iap'].sudo().search([('partner_id', 'in', self.ids)])
missing_partners = self
for partner_iap in partner_iaps:
if 'iap_enrich_info' in vals:
partner_iap.iap_enrich_info = vals['iap_enrich_info']
if 'iap_search_domain' in vals:
partner_iap.iap_search_domain = vals['iap_search_domain']
missing_partners -= partner_iap.partner_id
if missing_partners:
# Create new <res.partner.iap> for missing records
self.env['res.partner.iap'].sudo().create([
{
'partner_id': partner.id,
'iap_enrich_info': vals.get('iap_enrich_info'),
'iap_search_domain': vals.get('iap_search_domain'),
} for partner in missing_partners
])
@@ -0,0 +1,27 @@
# -*- coding: utf-8 -*-
from odoo import fields, models
class ResPartnerIap(models.Model):
"""Technical model which stores the response returned by IAP.
The goal of this model is to not enrich 2 times the same company. We do it in a
separate model to not add heavy field (iap_enrich_info) on the <res.partner>
model.
We also save the requested domain, so whatever the values are on the <res.partner>,
we will always retrieve the already enriched <res.partner> and the corresponding
IAP information.
"""
_name = 'res.partner.iap'
_description = 'Partner IAP'
partner_id = fields.Many2one('res.partner', string='Partner', help='Corresponding partner',
ondelete='cascade', required=True)
iap_search_domain = fields.Char('Search Domain / Email', help='Domain used to find the company')
iap_enrich_info = fields.Text('IAP Enrich Info', help='IAP response stored as a JSON string', readonly=True)
_sql_constraints = [('unique_partner_id', 'UNIQUE(partner_id)', 'Only one partner IAP is allowed for one partner')]
@@ -0,0 +1,2 @@
id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
mail_plugin.access_res_partner_iap,access_res_partner_iap,mail_plugin.model_res_partner_iap,base.group_system,1,1,1,1
1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
2 mail_plugin.access_res_partner_iap access_res_partner_iap mail_plugin.model_res_partner_iap base.group_system 1 1 1 1
+5
View File
@@ -0,0 +1,5 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from . import test_controller
from . import test_res_partner_iap
+92
View File
@@ -0,0 +1,92 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import json
from unittest.mock import patch
from odoo.addons.mail.tests.common import mail_new_test_user
from odoo.http import request
from odoo.tests.common import HttpCase
class TestMailPluginControllerCommon(HttpCase):
def setUp(self):
super(TestMailPluginControllerCommon, self).setUp()
self.user_test = mail_new_test_user(
self.env,
login="employee",
groups="base.group_user,base.group_partner_manager",
)
def mock_plugin_partner_get(self, name, email, patched_iap_enrich):
"""Simulate a HTTP call to /partner/get with the given email and name.
The authentication process is patched to allow all queries.
The third argument "patched_iap_enrich" allow you to mock the IAP request and
to return the response you want.
"""
def patched_auth_method_outlook(*args, **kwargs):
request.uid = self.user_test.id
data = {
"id": 0,
"jsonrpc": "2.0",
"method": "call",
"params": {"email": email, "name": name},
}
with patch(
"odoo.addons.mail_plugin.models.ir_http.IrHttp"
"._auth_method_outlook",
new=patched_auth_method_outlook,
), patch(
"odoo.addons.mail_plugin.controllers.mail_plugin.MailPluginController"
"._iap_enrich",
new=patched_iap_enrich,
):
result = self.url_open(
"/mail_plugin/partner/get",
data=json.dumps(data).encode(),
headers={"Content-Type": "application/json"},
)
if not result.ok:
return {}
return result.json().get("result", {})
def mock_enrich_and_create_company(self, partner_id, patched_iap_enrich):
"""Simulate a HTTP call to /partner/enrich_and_create_company on the given partner.
The third argument "patched_iap_enrich" allow you to mock the IAP request and
to return the response you want.
"""
def patched_auth_method_outlook(*args, **kwargs):
request.uid = self.user_test.id
data = {
"id": 0,
"jsonrpc": "2.0",
"method": "call",
"params": {"partner_id": partner_id},
}
with patch(
"odoo.addons.mail_plugin.models.ir_http.IrHttp"
"._auth_method_outlook",
new=patched_auth_method_outlook,
), patch(
"odoo.addons.mail_plugin.controllers.mail_plugin.MailPluginController"
"._iap_enrich",
new=patched_iap_enrich,
):
result = self.url_open(
"/mail_plugin/partner/enrich_and_create_company",
data=json.dumps(data).encode(),
headers={"Content-Type": "application/json"},
)
if not result.ok:
return {}
return result.json().get("result", {})
+168
View File
@@ -0,0 +1,168 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from unittest.mock import Mock
from odoo.addons.iap.tools import iap_tools
from odoo.addons.mail_plugin.tests.common import TestMailPluginControllerCommon
class TestMailPluginController(TestMailPluginControllerCommon):
def test_enrich_and_create_company(self):
partner = self.env["res.partner"].create({
"name": "Test partner",
"email": "test@test_domain.xyz",
"is_company": False,
})
result = self.mock_enrich_and_create_company(
partner.id,
lambda _, domain: {"return": domain},
)
self.assertEqual(result["enrichment_info"], {"type": "company_created"})
self.assertEqual(result["company"]["additionalInfo"]["return"], "test_domain.xyz")
company_id = result["company"]["id"]
company = self.env["res.partner"].browse(company_id)
partner.invalidate_cache()
self.assertEqual(partner.parent_id, company, "Should change the company of the partner")
def test_get_partner_blacklisted_domain(self):
"""Test enrichment on a blacklisted domain.
Even is the domain is blacklisted, we should not duplicate the company each
time a request is made.
"""
domain = list(iap_tools._MAIL_DOMAIN_BLACKLIST)[0]
result = self.mock_plugin_partner_get(
"Test", "qsd@" + domain,
lambda _, __: {
"name": "Name",
"email": ["contact@" + domain],
"iap_information": "test",
},
)
first_company_id = result["partner"]["company"]["id"]
self.assertTrue(first_company_id and first_company_id > 0)
self.assertEqual(result["partner"]["company"]["additionalInfo"]["iap_information"], "test")
first_company = self.env["res.partner"].browse(first_company_id)
self.assertEqual(first_company.name, "Name")
self.assertEqual(first_company.email, "contact@" + domain)
# Test that we do not duplicate the company and that we return the previous one
mock_iap_enrich = Mock()
result = self.mock_plugin_partner_get("Test", "qsd@" + domain, mock_iap_enrich)
self.assertFalse(
mock_iap_enrich.called,
"We already enriched this company, should not call IAP a second time")
second_company_id = result["partner"]["company"]["id"]
self.assertEqual(first_company_id, second_company_id, "Should not create a new company")
# But the same blacklisted domain on a different local part
# should create a new company (e.g.: asbl_XXXX@gmail.com VS asbl_YYYY@gmail.com)
result = self.mock_plugin_partner_get(
"Test", "asbl@" + domain,
lambda _, domain: {"name": "Name", "email": ["asbl@" + domain]},
)
second_company_id = result["partner"]["company"]["id"]
self.assertNotEqual(first_company_id, second_company_id, "Should create a new company")
def test_get_partner_company_found(self):
company = self.env["res.partner"].create({
"name": "Test partner",
"email": "test@test_domain.xyz",
"is_company": True,
})
mock_iap_enrich = Mock()
result = self.mock_plugin_partner_get("Test", "qsd@test_domain.xyz", mock_iap_enrich)
self.assertFalse(mock_iap_enrich.called)
self.assertEqual(result["partner"]["id"], -1)
self.assertEqual(result["partner"]["email"], "qsd@test_domain.xyz")
self.assertEqual(result["partner"]["company"]["id"], company.id)
self.assertFalse(result["partner"]["company"]["additionalInfo"])
def test_get_partner_company_not_found(self):
self.env["res.partner"].create({
"name": "Test partner",
"email": "test@test_domain.xyz",
"is_company": False,
})
result = self.mock_plugin_partner_get(
"Test",
"qsd@test_domain.xyz",
lambda _, domain: {"enrichment_info": "missing_data"},
)
self.assertEqual(result["partner"]["id"], -1)
self.assertEqual(result["partner"]["email"], "qsd@test_domain.xyz")
self.assertEqual(result["partner"]["company"]["id"], -1)
def test_get_partner_iap_return_different_domain(self):
"""
Test the case where the domain of the email returned by IAP is not the same as
the domain requested.
"""
result = self.mock_plugin_partner_get(
"Test",
"qsd@test_domain.xyz",
lambda _, domain: {
"name": "Name",
"email": ["contact@gmail.com"],
"iap_information": "test",
},
)
first_company_id = result["partner"]["company"]["id"]
first_company = self.env["res.partner"].browse(first_company_id)
self.assertEqual(result["partner"]["id"], -1)
self.assertEqual(result["partner"]["email"], "qsd@test_domain.xyz")
self.assertTrue(first_company_id, "Should have created the company")
self.assertEqual(result["partner"]["company"]["additionalInfo"]["iap_information"], "test")
self.assertEqual(first_company.name, "Name")
self.assertEqual(first_company.email, "contact@gmail.com")
# Test that we do not duplicate the company and that we return the previous one
mock_iap_enrich = Mock()
result = self.mock_plugin_partner_get("Test", "qsd@test_domain.xyz", mock_iap_enrich)
self.assertFalse(mock_iap_enrich.called, "We already enriched this company, should not call IAP a second time")
second_company_id = result["partner"]["company"]["id"]
self.assertEqual(first_company_id, second_company_id, "Should not create a new company")
self.assertEqual(result["partner"]["company"]["additionalInfo"]["iap_information"], "test")
def test_get_partner_no_email_returned_by_iap(self):
"""Test the case where IAP do not return an email address.
We should not duplicate the previously enriched company and we should be able to
retrieve the first one.
"""
result = self.mock_plugin_partner_get(
"Test", "qsd@domain.com",
lambda _, domain: {"name": "Name", "email": []},
)
first_company_id = result["partner"]["company"]["id"]
self.assertTrue(first_company_id and first_company_id > 0)
first_company = self.env["res.partner"].browse(first_company_id)
self.assertEqual(first_company.name, "Name")
self.assertFalse(first_company.email)
# Test that we do not duplicate the company and that we return the previous one
result = self.mock_plugin_partner_get(
"Test", "qsd@domain.com",
lambda _, domain: {"name": "Name", "email": ["contact@" + domain]},
)
second_company_id = result["partner"]["company"]["id"]
self.assertEqual(first_company_id, second_company_id, "Should not create a new company")
@@ -0,0 +1,77 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import psycopg2
from odoo.addons.mail.tests.common import MailCommon
from odoo.tools import mute_logger
class TestResPartnerIap(MailCommon):
@mute_logger("odoo.sql_db")
def test_res_partner_iap_constraint(self):
partner = self.partner_employee
self.env["res.partner.iap"].search([("partner_id", "=", partner.id)]).unlink()
self.env["res.partner.iap"].create({"partner_id": partner.id, "iap_enrich_info": "test info"})
with self.assertRaises(psycopg2.IntegrityError, msg="Can create only one partner IAP per partner"):
self.env["res.partner.iap"].create({"partner_id": partner.id, "iap_enrich_info": "test info"})
def test_res_partner_iap_compute_iap_enrich_info(self):
partner = self.partner_employee
self.assertFalse(partner.iap_enrich_info)
partner_iap = self.env["res.partner.iap"].create({"partner_id": partner.id, "iap_enrich_info": "test info"})
partner.invalidate_cache()
self.assertEqual(partner.iap_enrich_info, "test info")
partner_iap.unlink()
partner.iap_enrich_info = "test info 2"
partner_iap = self.env["res.partner.iap"].search([("partner_id", "=", partner.id)])
self.assertTrue(partner_iap, "Should have create the <res.partner.iap>")
self.assertEqual(partner_iap.iap_enrich_info, "test info 2")
partner.iap_enrich_info = "test info 3"
partner_iap.invalidate_cache()
new_partner_iap = self.env["res.partner.iap"].search([("partner_id", "=", partner.id)])
self.assertEqual(new_partner_iap, partner_iap, "Should have write on the existing one")
self.assertEqual(partner_iap.iap_enrich_info, "test info 3")
def test_res_partner_iap_creation(self):
partner = self.env['res.partner'].create({
'name': 'Test partner',
'iap_enrich_info': 'enrichment information',
'iap_search_domain': 'qsd@example.com',
})
partner.invalidate_cache()
self.assertEqual(partner.iap_enrich_info, 'enrichment information')
self.assertEqual(partner.iap_search_domain, 'qsd@example.com')
partner_iap = self.env['res.partner.iap'].search([('partner_id', '=', partner.id)])
self.assertEqual(len(partner_iap), 1, 'Should create only one <res.partner.iap>')
self.assertEqual(partner_iap.iap_enrich_info, 'enrichment information')
self.assertEqual(partner_iap.iap_search_domain, 'qsd@example.com')
def test_res_partner_iap_writing(self):
partner = self.env['res.partner'].create({
'name': 'Test partner 2',
})
partner.write({
'iap_enrich_info': 'second information',
'iap_search_domain': 'xyz@example.com',
})
partner_iap = self.env['res.partner.iap'].search([('partner_id', '=', partner.id)])
self.assertEqual(len(partner_iap), 1, 'Should create only one <res.partner.iap>')
self.assertEqual(partner_iap.iap_enrich_info, 'second information')
self.assertEqual(partner_iap.iap_search_domain, 'xyz@example.com')
partner.iap_search_domain = "only write on domain"
partner_iap.invalidate_cache()
self.assertEqual(partner_iap.iap_enrich_info, 'second information')
self.assertEqual(partner_iap.iap_search_domain, 'only write on domain')
@@ -0,0 +1,42 @@
<?xml version="1.0" encoding="utf-8"?>
<odoo>
<record id="res_partner_iap_action" model="ir.actions.act_window">
<field name="name">IAP Partner</field>
<field name="res_model">res.partner.iap</field>
<field name='view_mode'>tree,form</field>
</record>
<record id="res_partner_iap_view_form" model="ir.ui.view">
<field name="name">res.partner.iap.view.form</field>
<field name="model">res.partner.iap</field>
<field name="arch" type="xml">
<form string="IAP Partner">
<sheet>
<h1><field name="partner_id"/></h1>
<group>
<field name="iap_search_domain"/>
<field name="iap_enrich_info"/>
</group>
</sheet>
</form>
</field>
</record>
<record id="res_partner_iap_view_tree" model="ir.ui.view">
<field name="name">res.partner.iap.view.tree</field>
<field name="model">res.partner.iap</field>
<field name="arch" type="xml">
<tree string="IAP Partner">
<field name="partner_id"/>
<field name="iap_search_domain"/>
</tree>
</field>
</record>
<menuitem
id="res_partner_iap_menu"
name="IAP Partners"
parent="iap.iap_root_menu"
action="res_partner_iap_action"
sequence="50"/>
</odoo>