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

[IMP] base,test_new_api,website:prevent merge of partners with more than 1 user

Browse Source 
We prevent to merge partner linked to more than one user (by raising an
exception) to avoid having multiple user pointing to the same partner as a
consequence of a merge.

We want to avoid this situation because it generates weird behaviors. As
res.user inherits from res.partner, having multiple user pointing to the same
partner makes the fields of that partner shared with all those users. This was
decided following the tentative to improve partner merge in website_slides
(odoo/odoo#114840), where we also noticed strange behavior like completing a
lesson with one user were adding karma to all the users linked to a same
partner.

We have also adapted WebsiteVisitorTests tests in website that were failing
because they were merging partners with more than one user: simply by merging
partners with only one user.

Task-3167160

closes odoo/odoo#125322

Signed-off-by: Stéphane Debauche (std) <std@odoo.com>
This commit is contained in:
Pierre-Yves Dufays
2023-08-02 15:59:24 +02:00
parent ebfb7112c5
commit 169eaec793
3 changed files with 75 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/website/tests/test_website_visitor.py
+50 -41
View File
@@ -101,6 +101,8 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
'partner_id': self.partner_portal.id,
'groups_id': [(6, 0, [self.env.ref('base.group_portal').id])],
})
# Partner with no user associated, to test partner merge that forbids merging partners with more than 1 user
self.partner_admin_duplicate = self.env['res.partner'].create({'name': 'Mitchell'})
def _get_last_visitor(self):
return self.env['website.visitor'].search([], limit=1, order="id DESC")
@@ -393,38 +395,40 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
def test_merge_partner_with_visitor_both(self):
""" See :meth:`test_merge_partner_with_visitor_single` """
# Setup a visitor for demo and none for admin
# Setup a visitor for admin_duplicate and none for admin
Visitor = self.env['website.visitor']
(self.partner_demo + self.partner_admin).visitor_ids.unlink()
[visitor_demo, visitor_admin] = Visitor.create([{
'partner_id': self.partner_demo.id,
'access_token': self.partner_demo.id,
(self.partner_admin_duplicate + self.partner_admin).visitor_ids.unlink()
[visitor_admin_duplicate, visitor_admin] = Visitor.create([{
'partner_id': self.partner_admin_duplicate.id,
'access_token': self.partner_admin_duplicate.id,
}, {
'partner_id': self.partner_admin.id,
'access_token': self.partner_admin.id,
}])
# | id | access_token | partner_id |
# | -- | ------------ | ---------- |
# | 1 | demo_id | demo_id |
# | | 1062141 | 1062141 |
# | 2 | admin_id | admin_id |
# | | 5013266 | 5013266 |
self.assertTrue(visitor_demo.partner_id.id == int(visitor_demo.access_token) == self.partner_demo.id)
# | id | access_token | partner_id |
# | -- | ---------------------- | --------------------- |
# | 1 | admin_duplicate_id | admin_duplicate_id |
# | | 1062141 | 1062141 |
# | 2 | admin_id | admin_id |
# | | 5013266 | 5013266 |
self.assertTrue(visitor_admin_duplicate.partner_id.id ==
int(visitor_admin_duplicate.access_token) ==
self.partner_admin_duplicate.id)
self.assertTrue(visitor_admin.partner_id.id == int(visitor_admin.access_token) == self.partner_admin.id)
self.env['website.track'].create([{
'visitor_id': visitor_demo.id,
'url': '/demo'
'visitor_id': visitor_admin_duplicate.id,
'url': '/admin/about-duplicate'
}, {
'visitor_id': visitor_admin.id,
'url': '/admin'
}])
self.assertEqual(visitor_demo.website_track_ids.url, '/demo')
self.assertEqual(visitor_admin_duplicate.website_track_ids.url, '/admin/about-duplicate')
self.assertEqual(visitor_admin.website_track_ids.url, '/admin')
# Merge demo partner in admin partner
# Merge admin_duplicate partner (no user associated) in admin partner
self.env['base.partner.merge.automatic.wizard']._merge(
(self.partner_admin + self.partner_demo).ids,
(self.partner_admin + self.partner_admin_duplicate).ids,
self.partner_admin
)
# Should be
@@ -433,11 +437,12 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
# | 2 | admin_id | admin_id |
# | | 5013266 | 5013266 |
self.assertTrue(visitor_admin.exists())
self.assertFalse(visitor_demo.exists())
self.assertFalse(Visitor.search_count([('partner_id', '=', self.partner_demo.id)]),
"The demo visitor should've been merged (and deleted) with the admin one.")
self.assertFalse(visitor_admin_duplicate.exists())
self.assertFalse(Visitor.search_count([('partner_id', '=', self.partner_admin_duplicate.id)]),
"The admin_duplicate visitor should've been merged (and deleted) with the admin one.")
# Track check
self.assertEqual(visitor_admin.website_track_ids.sorted('url').mapped('url'), ['/admin', '/demo'])
self.assertEqual(visitor_admin.website_track_ids.sorted('url').mapped('url'),
['/admin', '/admin/about-duplicate'])
def test_merge_partner_with_visitor_single(self):
""" The partner merge feature of Odoo is auto discovering relations to
@@ -464,35 +469,39 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
Case 1 is tested here.
Cade 2 is tested in :meth:`test_merge_partner_with_visitor_both`.
"""
# Setup a visitor for demo and none for admin
# Setup a visitor for admin_duplicate and none for admin
Visitor = self.env['website.visitor']
(self.partner_demo + self.partner_admin).visitor_ids.unlink()
visitor_demo = Visitor.create({
'partner_id': self.partner_demo.id,
'access_token': self.partner_demo.id,
(self.partner_admin_duplicate + self.partner_admin).visitor_ids.unlink()
visitor_admin_duplicate = Visitor.create({
'partner_id': self.partner_admin_duplicate.id,
'access_token': self.partner_admin_duplicate.id,
})
# | id | access_token | partner_id |
# | -- | ------------ | ---------- |
# | 1 | demo_id | demo_id |
# | | 1062141 | 1062141 |
self.assertTrue(visitor_demo.partner_id.id == int(visitor_demo.access_token) == self.partner_demo.id)
# | id | access_token | partner_id |
# | -- | ---------------------- | --------------------- |
# | 1 | admin_duplicate_id | admin_duplicate_id |
# | | 1062141 | 1062141 |
self.assertTrue(visitor_admin_duplicate.partner_id.id ==
int(visitor_admin_duplicate.access_token) ==
self.partner_admin_duplicate.id)
# Merge demo partner in admin partner
# Merge admin_duplicate partner (no user associated) in admin partner
self.env['base.partner.merge.automatic.wizard']._merge(
(self.partner_admin + self.partner_demo).ids,
(self.partner_admin + self.partner_admin_duplicate).ids,
self.partner_admin
)
# This should not happen..
# | id | access_token | partner_id |
# | -- | ------------ | ---------- |
# | 1 | demo_id | admin_id | <-- Mismatch
# | | 1062141 | 5013266 |
# | id | access_token | partner_id |
# | -- | ---------------------- | ---------- |
# | 1 | admin_duplicate_id | admin_id | <-- Mismatch
# | | 1062141 | 5013266 |
# .. it should be:
# | id | access_token | partner_id |
# | -- | ------------ | ---------- |
# | 1 | admin_id | admin_id | <-- No mismatch, became admin_id
# | | 5013266 | 5013266 |
self.assertTrue(visitor_demo.partner_id.id == int(visitor_demo.access_token) == self.partner_admin.id,
"The demo visitor should now be linked to the admin partner.")
self.assertFalse(Visitor.search_count([('partner_id', '=', self.partner_demo.id)]),
"The demo visitor should've been merged (and deleted) with the admin one.")
self.assertTrue(visitor_admin_duplicate.partner_id.id ==
int(visitor_admin_duplicate.access_token) ==
self.partner_admin.id,
"The admin_duplicate visitor should now be linked to the admin partner.")
self.assertFalse(Visitor.search_count([('partner_id', '=', self.partner_admin_duplicate.id)]),
"The admin_duplicate visitor should've been merged (and deleted) with the admin one.")
odoo/addons/base/wizard/base_partner_merge.py
+4
View File
@@ -310,6 +310,10 @@ class MergePartnerAutomatic(models.TransientModel):
if partner_ids & child_ids:
raise UserError(_("You cannot merge a contact with one of his parent."))
# check if the list of partners to merge are linked to more than one user
if len(partner_ids.with_context(active_test=False).user_ids) > 1:
raise UserError(_("You cannot merge contacts linked to more than one user even if only one is active."))
if extra_checks and len(set(partner.email for partner in partner_ids)) > 1:
raise UserError(_("All contacts must have the same email. Only the Administrator can merge contacts with different emails."))
odoo/addons/test_new_api/tests/test_one2many.py
+21 -1
View File
@@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-
from odoo.tests.common import TransactionCase
from odoo.exceptions import MissingError
from odoo.exceptions import MissingError, UserError
from odoo import Command
from odoo.tools import mute_logger
@@ -189,6 +189,26 @@ class One2manyCase(TransactionCase):
self.assertTrue(p2.exists())
self.assertFalse(p3.exists())
def test_partner_merge_wizard_more_than_one_user_error(self):
""" Test that partners cannot be merged if linked to more than one user even if only one is active. """
p1, p2, dst_partner = self.env['res.partner'].create([{'name': f'test{idx + 1}'} for idx in range(3)])
u1, u2 = self.env['res.users'].create([{'name': 'test1', 'login': 'test1', 'partner_id': p1.id},
{'name': 'test2', 'login': 'test2', 'partner_id': p2.id}])
MergeWizard_with_context = self.env['base.partner.merge.automatic.wizard'].with_context(
active_ids=(u1.partner_id + u2.partner_id + dst_partner).ids, active_model='res.partner')
with self.assertRaises(UserError):
MergeWizard_with_context.create({}).action_merge()
u2.action_archive()
with self.assertRaises(UserError):
MergeWizard_with_context.create({}).action_merge()
u2.unlink()
MergeWizard_with_context.create({}).action_merge()
self.assertTrue(dst_partner.exists())
self.assertEqual(u1.partner_id.id, dst_partner.id)
def test_cache_invalidation(self):
""" Cache invalidation for one2many with integer inverse. """
record0 = self.env['test_new_api.attachment.host'].create({})