From 0641cb20e87123bbb527cc57bdb8fea36d0bbd7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Warnon?= Date: Tue, 15 Feb 2022 14:30:01 +0000 Subject: [PATCH] [REF] website_event: move website.visitor's parent_id to website The concept of 'parent_id' on website.visitors was introduced in the saas-13.3 stable while implementing the "event online" feature: However, it should have been part of the website module from the start since it's a 'global' concept that does not depend on events at all. See #53540 for more details. This commit aims to clean the code by moving the field to the website module, which allows a nice cleaning of associated overridden methods as well. Along with that, we move the website.visitor demo data from the event module to the website module, allowing a fresh install of website to showcase some of our visitors feature. We also took this opportunity to do some minor improvements in the visitors kanban view in order to make relevant information more visible. Task-2429652 Part-of: odoo/odoo#65113 --- addons/website/__manifest__.py | 1 + addons/website/data/website_visitor_demo.xml | 35 +++++++++++++++++ addons/website/models/res_users.py | 2 +- addons/website/models/website_visitor.py | 38 +++++++++++++----- addons/website/tests/test_website_visitor.py | 20 +++------- .../website/views/website_visitor_views.xml | 16 +++++--- .../views/website_visitor_views.xml | 13 +++++-- addons/website_event/__manifest__.py | 1 - .../data/event_registration_demo.xml | 8 ++-- .../data/website_visitor_demo.xml | 26 ------------- .../website_event/models/website_visitor.py | 39 +------------------ .../data/event_track_visitor_demo.xml | 4 +- .../models/website_visitor.py | 4 +- .../data/quiz_demo.xml | 6 +-- .../models/website_visitor.py | 4 +- addons/website_livechat/tests/__init__.py | 1 + .../views/website_visitor_views.xml | 9 +++-- 17 files changed, 114 insertions(+), 113 deletions(-) create mode 100644 addons/website/data/website_visitor_demo.xml delete mode 100644 addons/website_event/data/website_visitor_demo.xml diff --git a/addons/website/__manifest__.py b/addons/website/__manifest__.py index 63a9070c1ee..eb60fa0f3ff 100644 --- a/addons/website/__manifest__.py +++ b/addons/website/__manifest__.py @@ -113,6 +113,7 @@ ], 'demo': [ 'data/website_demo.xml', + 'data/website_visitor_demo.xml', ], 'application': True, 'post_init_hook': 'post_init_hook', diff --git a/addons/website/data/website_visitor_demo.xml b/addons/website/data/website_visitor_demo.xml new file mode 100644 index 00000000000..b03141e5fc1 --- /dev/null +++ b/addons/website/data/website_visitor_demo.xml @@ -0,0 +1,35 @@ + + + + + Edwin Hansen + + + + + + + Soham Palmer + + + + + + + Philipe J. Fry + + + + + + + Philipe J. Fry (old) + + + + + + + + + diff --git a/addons/website/models/res_users.py b/addons/website/models/res_users.py index d64f94db6b4..1173757b10e 100644 --- a/addons/website/models/res_users.py +++ b/addons/website/models/res_users.py @@ -87,7 +87,7 @@ class ResUsers(models.Model): 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_sudo + other_visitors)._link_to_visitor(visitor_main) visitor_main.name = user_partner.name visitor_main.active = True visitor_main._update_visitor_last_visit() diff --git a/addons/website/models/website_visitor.py b/addons/website/models/website_visitor.py index 400afd00a7f..3486fdbaa4b 100644 --- a/addons/website/models/website_visitor.py +++ b/addons/website/models/website_visitor.py @@ -35,6 +35,7 @@ class WebsiteVisitor(models.Model): active = fields.Boolean('Active', default=True) website_id = fields.Many2one('website', "Website", readonly=True) partner_id = fields.Many2one('res.partner', string="Contact", help="Partner of the last logged in user.", index='btree_not_null') + parent_id = fields.Many2one('website.visitor', string="Parent", ondelete='set null', index='btree_not_null', help="Main identity") partner_image = fields.Binary(related='partner_id.image_1920') # localisation and info @@ -161,10 +162,17 @@ class WebsiteVisitor(models.Model): } def _get_visitor_from_request(self, force_create=False): - """ Return the visitor as sudo from the request if there is a visitor_uuid cookie. - It is possible that the partner has changed or has disconnected. - In that case the cookie is still referencing the old visitor and need to be replaced - with the one of the visitor returned !!!. """ + """ Return the visitor as sudo from the request if there is a + visitor_uuid cookie. + + When fetching visitor, now that duplicates are linked to a main visitor + instead of unlinked, you may have more collisions issues with cookie + being set (after a de-connection for example). + + The visitor associated to a partner in case of public user is not taken + into account, it is considered as desynchronized cookie. + In addition, we also discard if the visitor has a main visitor whose + partner is set (aka wrong after logout partner). """ # This function can be called in json with mobile app. # In case of mobile app, no uid is set on the jsonRequest env. @@ -189,10 +197,21 @@ class WebsiteVisitor(models.Model): # Cookie associated to a Partner visitor = Visitor + # also check that visitor parent partner is not different from user's one + # (indicates duplicate due to invalid or wrong cookie) + if visitor and visitor.parent_id.partner_id: + if self.env.user._is_public(): + visitor = self.env['website.visitor'].sudo() + elif not visitor.partner_id: + visitor = self.env['website.visitor'].sudo().with_context(active_test=False).search( + [('partner_id', '=', self.env.user.partner_id.id)] + ) + if visitor and not visitor.timezone: tz = self._get_visitor_timezone() if tz: visitor._update_visitor_timezone(tz) + if not visitor and force_create: visitor = self._create_visitor() @@ -265,15 +284,14 @@ class WebsiteVisitor(models.Model): vals.update(update_values) self.write(vals) - def _link_to_visitor(self, target, keep_unique=True): + def _link_to_visitor(self, target): """ 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 + 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: @@ -281,8 +299,10 @@ class WebsiteVisitor(models.Model): # Link sub records of self to target visitor self.website_track_ids.write({'visitor_id': target.id}) - if keep_unique: - self.unlink() + # Archive current record and set its parent visitor + self.partner_id = False + self.parent_id = target.id + self.active = False return target diff --git a/addons/website/tests/test_website_visitor.py b/addons/website/tests/test_website_visitor.py index 374594f955c..40218fb9614 100644 --- a/addons/website/tests/test_website_visitor.py +++ b/addons/website/tests/test_website_visitor.py @@ -120,21 +120,13 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): ) def assertVisitorDeactivated(self, visitor, main_visitor): - """ Temporary method to check that a visitor has been de-activated / merged + """ Method that checks that a visitor has been de-activated / merged with other visitor, notably in case of login (see User.authenticate() as - well as Visitor._link_to_visitor() ). - - As final result depends on installed modules (see overrides) due to stable - improvements linked to EventOnline, this method contains a hack to avoid - doing too much overrides just for that behavior. """ - if 'parent_id' in self.env['website.visitor']: - self.assertTrue(bool(visitor)) - self.assertFalse(visitor.active) - self.assertTrue(main_visitor.active) - self.assertEqual(visitor.parent_id, main_visitor) - else: - self.assertFalse(visitor) - self.assertTrue(bool(main_visitor)) + well as Visitor._link_to_visitor() ). """ + self.assertTrue(bool(visitor)) + self.assertFalse(visitor.active) + self.assertTrue(main_visitor.active) + self.assertEqual(visitor.parent_id, main_visitor) def test_visitor_creation_on_tracked_page(self): """ Test various flows involving visitor creation and update. """ diff --git a/addons/website/views/website_visitor_views.xml b/addons/website/views/website_visitor_views.xml index 6b0b46b0b96..8ec39791db4 100644 --- a/addons/website/views/website_visitor_views.xml +++ b/addons/website/views/website_visitor_views.xml @@ -128,12 +128,16 @@
Visits
- -
Last Page
+ + + +
Last Page
- -
Visited Pages
+ + + +
Visited Pages