[FIX] website: correctly handle website_visitor with merge partners

Since the refactoring of website.visitor with [1] (upsert), the
`access_token` is supposed to be holding the same value as the
`partner_id` when the visitor is linked to a partner:
- Anonymous visitor: no `partner_id`, `access_token` is a hash value
- Partner visitor: `partner_id` set, `access_token` should be sync with
                   `partner_id`.

`partner_id` is just a stored computed field holding the `access_token`
value if it is an integer value.
There should never be a case where there is a `partner_id` set and the
`access_token` is not equal to the `partner_id`.

For instance, having a visitor with `partner_id` = 4 and `access_token`
 = `e4r3ejkj4` is supposed to be impossible.
It would lead to crash, because the visitor is only searched based on
his `access_token`, meaning that when searching for the visitor of
partner 4, none would be found and a new one would try to be created,
raising the `uniq_access_token_id` SQL constraint.

While the `partner_id`/`access_token` sync might seems weird, it is done
to allow the `upsert` use in SQL to improve perfs of this low level
behavior.
It's actually not as weak as it seems as there is only a single entry
point to update the `partner_id` and `access_token`: the authenticate
override of website.
Those fields are not supposed to be changed elsewhere.

Note that modifying the `access_token` would not be an issue as the
`partner_id` is just a stored compute based on the `access_token`.
But it's only true when modifying through the ORM as if you do that in
raw SQL, it won't go through the `api.depends` which is supposed to
recompute the stored computed `partner_id` field.

But something was forgotten during the initial dev: the partner merge
behavior: it does (on top of other thing) auto discover the m2o field
relations that points to a `res.partner` and modify those values in raw
SQL to the new value.
This is obviously wrong regarding the `website.visitor`'s `partner_id`
field, the `access_token` should also be updated, or when possible
visitors should be merged too.

Note that for DB upgrated from previous version to Odoo 16, this is
ensured through the following upgrade script [2]:
```sql
UPDATE website_visitor
    SET access_token = partner_id::text
WHERE partner_id IS NOT NULL
```

[1]: https://github.com/odoo/odoo/commit/d348bed1ad9d3d16b295f013f015706be6c07820
[2]: https://github.com/odoo/upgrade/commit/0cedcbf70494dfeddeb5a97c13bf875cb6a86886

task-3148111

closes odoo/odoo#111002

X-original-commit: a87b4142dd4a2c05e3e1885b2c54f5e0d3c7ac47
Signed-off-by: Romain Derie (rde) <rde@odoo.com>

addons/website/models/__init__.py

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from . import assets
+from . import base_partner_merge
 from . import ir_actions_server
 from . import ir_asset
 from . import ir_attachment

addons/website/models/base_partner_merge.py

@@ -0,0 +1,32 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+
+from odoo import api, models
+
+
+class MergePartnerAutomatic(models.TransientModel):
+    _inherit = 'base.partner.merge.automatic.wizard'
+
+    @api.model
+    def _update_foreign_keys(self, src_partners, dst_partner):
+        # Case 1: there is a visitor for both src and dst partners.
+        # Need to merge visitors before `super` to avoid SQL partner_id unique
+        # constraint to raise as it will change partner_id of the visitor
+        # record(s) to the `dst_partner` which already exists.
+        dst_visitor = dst_partner.visitor_ids and dst_partner.visitor_ids[0]
+        if dst_visitor:
+            for visitor in src_partners.visitor_ids:
+                visitor._merge_visitor(dst_visitor)
+
+        super()._update_foreign_keys(src_partners, dst_partner)
+
+        # Case 2: there is a visitor only for src_partners.
+        # Need to fix the "de-sync" values between `access_token` and
+        # `partner_id`.
+        self.env.cr.execute("""
+            UPDATE website_visitor
+               SET access_token = partner_id
+             WHERE partner_id::int != access_token::int
+               AND partner_id = %s;
+        """, (dst_partner.id,))

addons/website/tests/test_website_visitor.py

@@ -390,3 +390,109 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
                 'url': self.tracked_page_2.url
             })]
         }
+
+    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
+        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,
+        }, {
+            '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)
+        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.id,
+            'url': '/admin'
+        }])
+        self.assertEqual(visitor_demo.website_track_ids.url, '/demo')
+        self.assertEqual(visitor_admin.website_track_ids.url, '/admin')
+
+        # Merge demo partner in admin partner
+        self.env['base.partner.merge.automatic.wizard']._merge(
+            (self.partner_admin + self.partner_demo).ids,
+            self.partner_admin
+        )
+        # Should be
+        # | id | access_token | partner_id |
+        # | -- | ------------ | ---------- |
+        # |  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.")
+        # Track check
+        self.assertEqual(visitor_admin.website_track_ids.mapped('url'), ['/admin', '/demo'])
+
+    def test_merge_partner_with_visitor_single(self):
+        """ The partner merge feature of Odoo is auto discovering relations to
+        ``res_partner`` to change the field value, in raw SQL.
+        It will change the ``partner_id`` field of visitor without changing the
+        ``access_token``, which is supposed to be the same value (``partner_id``
+        is just a stored computed field holding the ``access_token`` value if it
+        is an integer value).
+        This partner_id/access_token "de-sync" need to be handled, this is done
+        in ``_update_foreign_keys()`` website override.
+        This test is ensuring that it works as it should.
+
+        There is 2 possible cases:
+
+        1. There is a visitor for partner 1, none for partner 2. Partner 1 is
+           merged into partner 2, making partner_id of visitor from partner 1
+           becoming partner 2.
+           -> The ``access_token`` value should also be updated from 1 to 2.
+        2. There is a visitor for both partners and partner 1 is merged into
+           partner 2.
+           -> Both visitor should be merged too, so data are aggregated into a
+              single visitor.
+
+        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
+        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,
+        })
+        # | 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)
+
+        # Merge demo partner in admin partner
+        self.env['base.partner.merge.automatic.wizard']._merge(
+            (self.partner_admin + self.partner_demo).ids,
+            self.partner_admin
+        )
+        # This should not happen..
+        # | id | access_token | partner_id |
+        # | -- | ------------ | ---------- |
+        # |  1 |      demo_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.")