From 8ae4544b76665beb23db60b58b9a1cd4a4d9c5fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 30 Jun 2020 12:33:18 +0000 Subject: [PATCH] [REF] website: improve visitor / user synchronization at authenticate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RATIONALE Event will soon gain a major update called Event Online, allowing to better support full-online events. In order to prepare its merge, preparatory merge are done to lessen the final diff and have a smooth integration in a stable version (13.3). PURPOSE Purpose of this merge is to clean visitor synchronization and tests. It prepares further improvements to visitor model linked to Event Online. SPECIFICATIONS Add anchor methods to somehow merge visitors and update partner linked to visitors and their sub records. Main idea would be to be able to * avoid unlinking visitors, notably because we have keys linked to them allowing push notifications. As a given user may be linked to several devices (different keys / different visitors) keeping them in database improves push efficiency; * allow to link sub-records to a main visitor, like tracked pages history, even if multiple visitors are linked to the same identity; In this stable we cannot remove current unlink of duplicate visitors due to constraint of partner_id / visitor_id. However those methods allow to tweak behavior by override. This will be done in future tasks. LINKS Task ID 2290016 (improve visitor synchronization and tests) Prepares Task ID 2252655 (main Online Event task) Prepares Task ID 2284043 (Visitor-based track wishlist) PR #54036 X-original-commit: 8a8d2d4412d1b58545ee4f0240cca5114e541b6a Co-authored-by: Aurélien Warnon Co-authored-by: David Beguin Co-authored-by: Thibault Delavallée --- addons/website/models/res_users.py | 37 ++++++++++++-------- addons/website/models/website_visitor.py | 33 +++++++++++++++++ addons/website/tests/test_website_visitor.py | 3 ++ 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/addons/website/models/res_users.py b/addons/website/models/res_users.py index 060d5e7cc14..d64f94db6b4 100644 --- a/addons/website/models/res_users.py +++ b/addons/website/models/res_users.py @@ -68,26 +68,33 @@ class ResUsers(models.Model): @classmethod def authenticate(cls, db, login, password, user_agent_env): - """ Override to link the logged in user's res.partner to website.visitor """ + """ Override to link the logged in user's res.partner to website.visitor. + If both a request-based visitor and a user-based visitor exist we try + to update them (have same partner_id), and move sub records to the main + visitor (user one). Purpose is to try to keep a main visitor with as + much sub-records (tracked pages, leads, ...) as possible. """ uid = super(ResUsers, cls).authenticate(db, login, password, user_agent_env) if uid: with cls.pool.cursor() as cr: env = api.Environment(cr, uid, {}) visitor_sudo = env['website.visitor']._get_visitor_from_request() if visitor_sudo: - partner = env.user.partner_id - partner_visitor = env['website.visitor'].with_context(active_test=False).sudo().search([('partner_id', '=', partner.id)]) - if partner_visitor and partner_visitor.id != visitor_sudo.id: - # Link history to older Visitor and delete the newest - visitor_sudo.website_track_ids.write({'visitor_id': partner_visitor.id}) - visitor_sudo.unlink() - # If archived (most likely by the cron for inactivity reasons), reactivate the partner's visitor - if not partner_visitor.active: - partner_visitor.write({'active': True}) + user_partner = env.user.partner_id + other_user_visitor_sudo = env['website.visitor'].with_context(active_test=False).sudo().search( + [('partner_id', '=', user_partner.id), ('id', '!=', visitor_sudo.id)], + order='last_connection_datetime DESC', + ) # current 13.3 state: 1 result max as unique visitor / partner + if other_user_visitor_sudo: + visitor_main = other_user_visitor_sudo[0] + other_visitors = other_user_visitor_sudo[1:] # normally void + (visitor_sudo + other_visitors)._link_to_visitor(visitor_main, keep_unique=True) + visitor_main.name = user_partner.name + visitor_main.active = True + visitor_main._update_visitor_last_visit() else: - vals = { - 'partner_id': partner.id, - 'name': partner.name - } - visitor_sudo.write(vals) + if visitor_sudo.partner_id != user_partner: + visitor_sudo._link_to_partner( + user_partner, + update_values={'partner_id': user_partner.id}) + visitor_sudo._update_visitor_last_visit() return uid diff --git a/addons/website/models/website_visitor.py b/addons/website/models/website_visitor.py index 83a47f8554a..ca54d20d883 100644 --- a/addons/website/models/website_visitor.py +++ b/addons/website/models/website_visitor.py @@ -240,6 +240,39 @@ class WebsiteVisitor(models.Model): vals['name'] = self.env.user.partner_id.name return self.sudo().create(vals) + def _link_to_partner(self, partner, update_values=None): + """ Link visitors to a partner. This method is meant to be overridden in + order to propagate, if necessary, partner information to sub records. + + :param partner: partner used to link sub records; + :param update_values: optional values to update visitors to link; + """ + vals = {'name': partner.name} + if update_values: + vals.update(update_values) + self.write(vals) + + def _link_to_visitor(self, target, keep_unique=True): + """ Link visitors to target visitors, because they are linked to the + same identity. Purpose is mainly to propagate partner identity to sub + records to ease database update and decide what to do with "duplicated". + THis method is meant to be overridden in order to implement some specific + behavior linked to sub records of duplicate management. + + :param target: main visitor, target of link process; + :param keep_unique: if True, find a way to make target unique; + """ + # Link sub records of self to target partner + if target.partner_id: + self._link_to_partner(target.partner_id) + # Link sub records of self to target visitor + self.website_track_ids.write({'visitor_id': target.id}) + + if keep_unique: + self.unlink() + + return target + def _cron_archive_visitors(self): delay_days = int(self.env['ir.config_parameter'].sudo().get_param('website.visitor.live.days', 30)) deadline = datetime.now() - timedelta(days=delay_days) diff --git a/addons/website/tests/test_website_visitor.py b/addons/website/tests/test_website_visitor.py index 80f50d76be4..db0af8e5039 100644 --- a/addons/website/tests/test_website_visitor.py +++ b/addons/website/tests/test_website_visitor.py @@ -293,11 +293,14 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): # reconnect with new visitor. self.url_open(self.tracked_page.url) new_visitor = self._get_last_visitor() + self.assertFalse(new_visitor.partner_id) self.assertTrue(new_visitor.id > old_visitor.id, "A new visitor should have been created.") self.assertVisitorTracking(new_visitor, self.tracked_page) with self.mock_visitor_from_request(force_visitor=new_visitor): self.authenticate('demo', 'demo') + (new_visitor | old_visitor).flush() + partner_demo.invalidate_cache(fnames=['visitor_ids']) self.assertEqual(partner_demo.visitor_ids, old_visitor, "The partner visitor should be back to the 'old' visitor.") new_visitor = self.env['website.visitor'].search([('id', '=', new_visitor.id)])