[IMP] website[_event|_crm]: clean website.visitors and consolidate information

PURPOSE

Consolidate information on a 'main' website.visitor when using multiple
devices / browsers.
Which in turn allows a much deeper cleaning of irrelevant and inactive
website.visitors instead of archiving them, saving a lot of DB space.

SPECS

This purpose is reached in 3 steps:

### First step

First move the concept of 'parent_id' of website.visitor into the 'website'
module.

The implementation of 'parent_id' was done in website_event because of stable
development requirements but it makes sense to move it to the website module.

### Second step

Then enforce the consolidation of all visitors information onto the
'main visitor'.

Visitors are linked when we are able to identify a single user through its Odoo
account. The browser where the last connection has been made will be linked to
the main visitor, other visitor records are linked to it through the parent_id
field.

When using multiple devices / browsers, we want to consolidate all the relevant
information on the main visitor, including:
- Created crm.leads
- Subscriptions to events
- Wishlisted tracks
- (Subscriptions to push notifications, done in enterprise counterpart)

Once we have clean visitor records containing necessary important information,
we can safely go ahead and modify the CRON cleaning inactive visitors to DELETE
them instead of archiving them, leading less database space required / improved
query performances.

### Third step

Clean now unnecessary code for wishlisted tracks.
This code was necessary when we had to keep track of all visitors and its
children records when there was only one push_token allowed per
website.visitor.

Now that a website.visitor can have multiple push subscriptions (see enterprise
counterpart), this code is no longer necessary, leading to better logic and
increased performances.

Check underlying commits for detailed information about those 3 steps.

LINKS

Task-2429652

closes odoo/odoo#65113

Related: odoo/enterprise#16781
Related: odoo/upgrade#2111
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>

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',

addons/website/data/website_visitor_cron.xml

@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding='UTF-8'?>
 <odoo>
     <record id="website_visitor_cron" model="ir.cron">
-        <field name="name">Website Visitor : Archive old visitors</field>
+        <field name="name">Website Visitor : clean inactive visitors</field>
         <field name="model_id" ref="model_website_visitor"/>
         <field name="state">code</field>
-        <field name="code">model._cron_archive_visitors()</field>
+        <field name="code">model._cron_unlink_old_visitors()</field>
         <field name="interval_number">1</field>
         <field name="interval_type">days</field>
         <field name="numbercall">-1</field>

addons/website/data/website_visitor_demo.xml

@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<odoo>
+
+    <record id="website_visitor_0" model="website.visitor">
+        <field name="name">Edwin Hansen</field>
+        <field name="partner_id" ref="base.res_partner_address_5"/>
+        <field name="country_id" ref="base.nl"/>
+        <field name="page_ids" eval="[(4, ref('website.homepage_page'))]"/>
+        <field name="website_track_ids" eval="[(0, 0, {'page_id': ref('website.homepage_page'), 'url': '/'})]"/>
+    </record>
+    <record id="website_visitor_1" model="website.visitor">
+        <field name="name">Soham Palmer</field>
+        <field name="partner_id" ref="base.res_partner_address_11"/>
+        <field name="country_id" ref="base.us"/>
+        <field name="page_ids" eval="[(4, ref('website.contactus_page'))]"/>
+        <field name="website_track_ids" eval="[(0, 0, {'page_id': ref('website.contactus_page'), 'url': '/contactus'})]"/>
+    </record>
+    <record id="website_visitor_2" model="website.visitor">
+        <field name="name">Philipe J. Fry</field>
+        <field name="partner_id" eval="False"/>
+        <field name="country_id" ref="base.be"/>
+        <field name="page_ids" eval="[(4, ref('website.homepage_page'))]"/>
+        <field name="website_track_ids" eval="[(0, 0, {'page_id': ref('website.homepage_page'), 'url': '/'})]"/>
+    </record>
+    <record id="website_visitor_2_1" model="website.visitor">
+        <field name="name">Philipe J. Fry (old)</field>
+        <field name="active" eval="False"></field>
+        <field name="parent_id" ref="website.website_visitor_2"/>
+        <field name="partner_id" eval="False"/>
+        <field name="country_id" ref="base.be"/>
+        <field name="page_ids" eval="[(4, ref('website.contactus_page'))]"/>
+        <field name="website_track_ids" eval="[(0, 0, {'page_id': ref('website.contactus_page'), 'url': '/contactus'})]"/>
+    </record>
+
+</odoo>

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()

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,16 +299,34 @@ 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
 
-    def _cron_archive_visitors(self):
-        delay_days = int(self.env['ir.config_parameter'].sudo().get_param('website.visitor.live.days', 30))
+    def _cron_unlink_old_visitors(self):
+        """ Unlink inactive visitors (see '_inactive_visitors_domain' for details).
+
+        Visitors were previously archived but we came to the conclusion that archived visitors
+        have very little value and bloat the database for no reason. """
+
+        inactive_visitors = self.env['website.visitor'].sudo() \
+            .with_context(active_test=False) \
+            .search(self._inactive_visitors_domain())
+        inactive_visitors.unlink()
+
+    def _inactive_visitors_domain(self):
+        """ This method defines the domain of visitors that can be cleaned. By default visitors
+        not linked to any partner and not active for 'website.visitor.live.days' days (default being 60)
+        are considered as inactive.
+
+        This method is meant to be overridden by sub-modules to further refine inactivity conditions. """
+
+        delay_days = int(self.env['ir.config_parameter'].sudo().get_param('website.visitor.live.days', 60))
         deadline = datetime.now() - timedelta(days=delay_days)
-        visitors_to_archive = self.env['website.visitor'].sudo().search([('last_connection_datetime', '<', deadline)])
-        visitors_to_archive.write({'active': False})
+        return [('last_connection_datetime', '<', deadline), ('partner_id', '=', False)]
 
     def _update_visitor_timezone(self, timezone):
         """ We need to do this part here to avoid concurrent updates error. """

addons/website/tests/test_website_visitor.py

@@ -120,21 +120,16 @@ 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() ).
+        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)
 
-        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))
+        # tracks are linked
+        self.assertTrue(visitor.website_track_ids < main_visitor.website_track_ids)
 
     def test_visitor_creation_on_tracked_page(self):
         """ Test various flows involving visitor creation and update. """
@@ -287,40 +282,110 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
         # check number of visits
         self.assertEqual(visitor_portal.visit_count, 2, "There should be 2 visits for the portal user")
 
-    def test_visitor_archive(self):
-        """ Test cron archiving inactive visitors and their re-activation when
-        authenticating an user. """
-        self.env['ir.config_parameter'].sudo().set_param('website.visitor.live.days', 7)
-
-        partner_demo = self.partner_demo
-        old_visitor = self.env['website.visitor'].create({
+    def test_clean_inactive_visitors(self):
+        inactive_visitors = self.env['website.visitor'].create([{
+            'name': 'Crazy Lazy Bernie',
             'lang_id': self.env.ref('base.lang_en').id,
             'country_id': self.env.ref('base.be').id,
             'website_id': 1,
-            'partner_id': partner_demo.id,
-        })
-        self.assertTrue(old_visitor.active)
-        self.assertEqual(partner_demo.visitor_ids, old_visitor, "Visitor and its partner should be synchronized")
+            'last_connection_datetime': datetime.now() - timedelta(days=8)
+        }, {
+            'name': 'Sleeping Joe',
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'last_connection_datetime': datetime.now() - timedelta(days=15)
+        }])
 
-        # archive old visitor
-        old_visitor.last_connection_datetime = datetime.now() - timedelta(days=8)
-        self.env['website.visitor']._cron_archive_visitors()
-        self.assertEqual(old_visitor.active, False, "Visitor should be archived after inactivity")
+        active_visitors = self.env['website.visitor'].create([{
+            'name': 'Active Donald',
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'last_connection_datetime': datetime.now() - timedelta(days=1)
+        }, {
+            'name': 'Vladimir Customer',
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'partner_id': self.partner_demo.id,
+            'last_connection_datetime': datetime.now() - timedelta(days=15)
+        }])
 
-        # 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)
+        self._test_unlink_old_visitors(inactive_visitors, active_visitors)
 
-        with self.mock_visitor_from_request(force_visitor=new_visitor):
-            self.authenticate('demo', 'demo')
-        (new_visitor | old_visitor).flush()
-        partner_demo.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.")
+    def _test_unlink_old_visitors(self, inactive_visitors, active_visitors):
+        """ This method will test that the visitors are correctly deleted when inactive.
 
-        new_visitor = self.env['website.visitor'].search([('id', '=', new_visitor.id)])
-        self.assertEqual(len(new_visitor), 0, "The new visitor should be deleted when visitor authenticate once again.")
-        self.assertEqual(old_visitor.active, True, "The old visitor should be reactivated when visitor authenticates once again.")
+        - inactive_visitors: all visitors that should be unlinked by the CRON
+          '_cron_unlink_old_visitors'
+        - active_visitors: all visitors that should NOT be cleaned because they are either active
+          or have some important data linked to them (partner, ...) and we want to keep them.
+
+        We use this method as a private tool so that sub-module can also test the cleaning of visitors
+        based on their own sets of conditions. """
+
+        WebsiteVisitor = self.env['website.visitor'].with_context(active_test=False)
+
+        self.env['ir.config_parameter'].sudo().set_param('website.visitor.live.days', 7)
+
+        # ensure we keep a single query by correct usage of "not inselect"
+        # (+1 query to fetch the 'ir.config_parameter')
+        with self.assertQueryCount(2):
+            WebsiteVisitor.search(WebsiteVisitor._inactive_visitors_domain())
+
+        inactive_visitor_ids = inactive_visitors.ids
+        active_visitor_ids = active_visitors.ids
+
+        WebsiteVisitor._cron_unlink_old_visitors()
+        if inactive_visitor_ids:
+            # all inactive visitors should be deleted
+            self.assertFalse(bool(WebsiteVisitor.search([('id', 'in', inactive_visitor_ids)])))
+        if active_visitor_ids:
+            # all active visitors should be kept
+            self.assertEqual(active_visitors, WebsiteVisitor.search([('id', 'in', active_visitor_ids)]))
+
+    def test_link_to_visitor(self):
+        """ Visitors are 'linked' together when the user, previously not connected, authenticates
+        and the system detects it already had a website.visitor for that partner_id.
+        This can happen quite often if the user switches browsers / hardwares.
+
+        When 'linking' visitors together, the new visitor is archived and all its relevant data is
+        merged within the main visitor. See 'website.visitor#_link_to_visitor' for more details.
+
+        This test ensures that all the relevant data are properly merged.
+
+        We build this logic with sub-methods so that sub-modules can easily add their own data and
+        test that they are correctly merged."""
+
+        [main_visitor, linked_visitor] = self.env['website.visitor'].create([
+            self._prepare_main_visitor_data(),
+            self._prepare_linked_visitor_data()
+        ])
+        linked_visitor._link_to_visitor(main_visitor)
+
+        self.assertVisitorDeactivated(linked_visitor, main_visitor)
+
+    def _prepare_main_visitor_data(self):
+        return {
+            'name': "Mitchell Main",
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'website_track_ids': [(0, 0, {
+                'page_id': self.tracked_page.id,
+                'url': self.tracked_page.url
+            })]
+        }
+
+    def _prepare_linked_visitor_data(self):
+        return {
+            'name': "Mitchell Main's Phone",
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'website_track_ids': [(0, 0, {
+                'page_id': self.tracked_page_2.id,
+                'url': self.tracked_page_2.url
+            })]
+        }

addons/website/views/website_visitor_views.xml

@@ -128,12 +128,16 @@
                                     <div>Visits</div>
                                 </div>
                                 <div class="col-lg col-sm-4 col-6 py-0 my-2">
-                                    <b><field name="last_visited_page_id"/></b>
-                                    <div>Last Page</div>
+                                    <span t-att-class="record.page_count.raw_value ? 'font-weight-bold' : 'text-muted'">
+                                        <field name="last_visited_page_id"/>
+                                    </span>
+                                    <div t-att-class="record.page_count.raw_value ? '' : 'text-muted'">Last Page</div>
                                 </div>
                                 <div id="wvisitor_visited_page" class="col-lg col-sm-4 col-6 py-0 my-2">
-                                    <b><field name="page_count"/></b>
-                                    <div>Visited Pages</div>
+                                     <span t-att-class="record.page_count.raw_value ? 'font-weight-bold' : 'text-muted'">
+                                        <field name="page_count"/>
+                                    </span>
+                                    <div t-att-class="record.page_count.raw_value ? '' : 'text-muted'">Visited Pages</div>
                                 </div>
                                 <div class="w_visitor_kanban_actions_ungrouped col-lg-3 col-sm-12 py-0 my-2 text-lg-right">
                                     <button name="action_send_mail" type="object"
@@ -155,9 +159,9 @@
                                     <field name="display_name"/>
                                 </strong>
                                 <div class="mb-2">Active <field name="time_since_last_action"/></div>
-                                <div>Last Page<span class="float-right font-weight-bold"><field name="last_visited_page_id"/></span></div>
+                                <div t-if="record.page_count.raw_value">Last Page<span class="float-right font-weight-bold"><field name="last_visited_page_id"/></span></div>
                                 <div>Visits<span class="float-right font-weight-bold"><field name="visit_count"/></span></div>
-                                <div id="o_page_count">Visited Pages<span class="float-right font-weight-bold"><field name="page_count"/></span></div>
+                                <div t-if="record.page_count.raw_value" id="o_page_count">Visited Pages<span class="float-right font-weight-bold"><field name="page_count"/></span></div>
                                 <div class="w_visitor_kanban_actions">
                                     <button name="action_send_mail" type="object"
                                             class="btn btn-secondary" attrs="{'invisible': [('email', '=', False)]}">

addons/website_crm/models/website_visitor.py

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo import fields, models, api
+from odoo.osv import expression
 
 
 class WebsiteVisitor(models.Model):
@@ -43,6 +44,20 @@ class WebsiteVisitor(models.Model):
             return True
         return check
 
+    def _inactive_visitors_domain(self):
+        """ Visitors tied to leads are considered always active and should not be deleted. """
+        domain = super()._inactive_visitors_domain()
+        return expression.AND([domain, [('lead_ids', '=', False)]])
+
+    def _link_to_visitor(self, target):
+        """ Link the leads to the main visitor to avoid them being lost. """
+        if self.lead_ids:
+            target.write({
+                'lead_ids': [(4, lead.id) for lead in self.lead_ids]
+            })
+
+        return super(WebsiteVisitor, self)._link_to_visitor(target)
+
     def _prepare_message_composer_context(self):
         if not self.partner_id and self.lead_ids:
             sorted_leads = self.lead_ids._sort_by_confidence_level(reverse=True)

addons/website_crm/tests/test_website_visitor.py

@@ -1,11 +1,16 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
+from datetime import datetime, timedelta
+
 from odoo.addons.crm.tests.common import TestCrmCommon
+from odoo.addons.website.tests.test_website_visitor import WebsiteVisitorTests
+from odoo.tests import tagged
 from odoo.tests.common import users
 
 
-class TestWebsiteVisitor(TestCrmCommon):
+@tagged('website_visitor')
+class TestWebsiteVisitor(TestCrmCommon, WebsiteVisitorTests):
 
     def setUp(self):
         super(TestWebsiteVisitor, self).setUp()
@@ -65,3 +70,56 @@ class TestWebsiteVisitor(TestCrmCommon):
         customer.write({'mobile': False})
         self.assertEqual(visitor.email, customer.email_normalized)
         self.assertEqual(visitor.mobile, lead_2.mobile)
+
+    @tagged('website_visitor')
+    def test_clean_inactive_visitors_crm(self):
+        """ Visitors attached to leads should not be deleted even if not connected recently. """
+        active_visitors = self.env['website.visitor'].create([{
+            'name': 'Lead Carl',
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'last_connection_datetime': datetime.now() - timedelta(days=8),
+            'lead_ids': [(0, 0, {
+                'name': 'Lead Carl'
+            })]
+        }])
+
+        self._test_unlink_old_visitors(self.env['website.visitor'], active_visitors)
+
+    @tagged('website_visitor')
+    def test_link_to_visitor_crm(self):
+        """ Same as parent's 'test_link_to_visitor' except we also test that leads
+        are merged into main visitor. """
+        [main_visitor, linked_visitor] = self.env['website.visitor'].create([
+            self._prepare_main_visitor_data(),
+            self._prepare_linked_visitor_data()
+        ])
+        all_leads = (main_visitor + linked_visitor).lead_ids
+        linked_visitor._link_to_visitor(main_visitor)
+
+        self.assertVisitorDeactivated(linked_visitor, main_visitor)
+
+        # leads of both visitors should be merged into main one
+        self.assertEqual(len(main_visitor.lead_ids), 2)
+        self.assertEqual(main_visitor.lead_ids, all_leads)
+        for lead in all_leads:
+            self.assertEqual(lead.visitor_ids, main_visitor)
+
+    def _prepare_main_visitor_data(self):
+        values = super()._prepare_main_visitor_data()
+        values.update({
+            'lead_ids': [(0, 0, {
+                'name': 'Mitchel Main Lead'
+            })]
+        })
+        return values
+
+    def _prepare_linked_visitor_data(self):
+        values = super()._prepare_linked_visitor_data()
+        values.update({
+            'lead_ids': [(0, 0, {
+                'name': 'Mitchel Secondary Lead'
+            })]
+        })
+        return values

addons/website_crm/views/website_visitor_views.xml

@@ -48,12 +48,19 @@
                 <field name="lead_count"/>
             </field>
             <xpath expr="//div[@id='o_page_count']" position="after">
-                <div>Leads / Opportunities<span class="float-right font-weight-bold"><field name="lead_count"/></span></div>
+                <div t-if="record.lead_count.raw_value">
+                    Leads / Opportunities
+                    <span class="float-right font-weight-bold">
+                        <field name="lead_count"/>
+                    </span>
+                </div>
             </xpath>
             <xpath expr="//div[@id='wvisitor_visited_page']" position="after">
                 <div class="col-lg col-sm-4 col-6 py-0 my-2">
-                    <b><field name="lead_count"/></b>
-                    <div>Leads / Opportunities</div>
+                    <span t-att-class="record.lead_count.raw_value ? 'font-weight-bold' : 'text-muted'">
+                        <field name="lead_count"/>
+                    </span>
+                    <div t-att-class="record.lead_count.raw_value ? '' : 'text-muted'">Leads / Opportunities</div>
                 </div>
             </xpath>
         </field>

addons/website_event/__manifest__.py

@@ -40,7 +40,6 @@
     ],
     'demo': [
         'data/res_partner_demo.xml',
-        'data/website_visitor_demo.xml',
         'data/event_demo.xml',
         'data/event_registration_demo.xml',
     ],

addons/website_event/data/event_registration_demo.xml

@@ -3,16 +3,16 @@
 
     <!-- OpenWood Collection Online Reveal: Gemini (all) -->
     <record id="event.event_registration_7_0" model="event.registration">
-        <field name="visitor_id" ref="website_event.website_visitor_event_0"/>
+        <field name="visitor_id" ref="website.website_visitor_0"/>
     </record>
     <record id="event.event_registration_7_1" model="event.registration">
-        <field name="visitor_id" ref="website_event.website_visitor_event_0"/>
+        <field name="visitor_id" ref="website.website_visitor_0"/>
     </record>
     <record id="event.event_registration_7_2" model="event.registration">
-        <field name="visitor_id" ref="website_event.website_visitor_event_1"/>
+        <field name="visitor_id" ref="website.website_visitor_1"/>
     </record>
     <record id="event.event_registration_7_3" model="event.registration">
-        <field name="visitor_id" ref="website_event.website_visitor_event_1"/>
+        <field name="visitor_id" ref="website.website_visitor_1"/>
     </record>
 
 </data></odoo>

addons/website_event/data/website_visitor_demo.xml

@@ -1,26 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<odoo>
-
-    <record id="website_visitor_event_0" model="website.visitor">
-        <field name="name">Edwin Hansen</field>
-        <field name="partner_id" ref="base.res_partner_address_5"/>
-        <field name="country_id" ref="base.us"/>
-    </record>
-    <record id="website_visitor_event_1" model="website.visitor">
-        <field name="name">Soham Palmer</field>
-        <field name="partner_id" ref="base.res_partner_address_11"/>
-        <field name="country_id" ref="base.us"/>
-    </record>
-    <record id="website_visitor_event_2" model="website.visitor">
-        <field name="name">Philipe J. Fry</field>
-        <field name="partner_id" eval="False"/>
-        <field name="country_id" ref="base.be"/>
-    </record>
-    <record id="website_visitor_event_2_1" model="website.visitor">
-        <field name="name">Philipe J. Fry (old)</field>
-        <field name="parent_id" ref="website_event.website_visitor_event_2"/>
-        <field name="partner_id" eval="False"/>
-        <field name="country_id" ref="base.be"/>
-    </record>
-
-</odoo>

addons/website_event/models/website_visitor.py

@@ -2,15 +2,13 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo import api, fields, models
+from odoo.osv import expression
 
 
 class WebsiteVisitor(models.Model):
     _name = 'website.visitor'
     _inherit = ['website.visitor']
 
-    parent_id = fields.Many2one(
-        'website.visitor', string="Parent", ondelete='set null', index='btree_not_null',
-        help="Main identity")
     event_registration_ids = fields.One2many(
         'event.registration', 'visitor_id', string='Event Registrations',
         groups="event.group_event_registration_desk")
@@ -79,6 +77,11 @@ class WebsiteVisitor(models.Model):
 
         return [('id', 'in', visitor_ids)]
 
+    def _inactive_visitors_domain(self):
+        """ Visitors registered to events are considered always active and should not be deleted. """
+        domain = super()._inactive_visitors_domain()
+        return expression.AND([domain, [('event_registration_ids', '=', False)]])
+
     def _link_to_partner(self, partner, update_values=None):
         """ Propagate partner update to registration records """
         if partner:
@@ -87,40 +90,8 @@ class WebsiteVisitor(models.Model):
                 registration_wo_partner.partner_id = partner
         super(WebsiteVisitor, self)._link_to_partner(partner, update_values=update_values)
 
-    def _link_to_visitor(self, target, keep_unique=True):
+    def _link_to_visitor(self, target):
         """ Override linking process to link registrations to the final visitor. """
         self.event_registration_ids.write({'visitor_id': target.id})
 
-        res = super(WebsiteVisitor, self)._link_to_visitor(target, keep_unique=False)
-
-        if keep_unique:
-            self.partner_id = False
-            self.parent_id = target.id
-            self.active = False
-
-        return res
-
-    def _get_visitor_from_request(self, force_create=False):
-        """ 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.
-
-        In base method, visitor associated to a partner in case of public user is
-        not taken into account. It is considered as desynchronized cookie. Here
-        we also discard if the visitor has a main visitor whose partner is set
-        (aka wrong after logout partner). """
-        visitor = super(WebsiteVisitor, self)._get_visitor_from_request(force_create=force_create)
-
-        # 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 not visitor and force_create:
-            visitor = self._create_visitor()
-
-        return visitor
+        return super(WebsiteVisitor, self)._link_to_visitor(target)

addons/website_event/tests/test_event_visitor.py

@@ -4,48 +4,63 @@
 from datetime import datetime, timedelta
 
 from odoo import fields
+from odoo.addons.website.tests.test_website_visitor import WebsiteVisitorTests
 from odoo.addons.website_event.tests.common import TestEventOnlineCommon
+from odoo.tests import tagged
 
 
-class TestEventVisitor(TestEventOnlineCommon):
+@tagged('website_visitor')
+class TestEventVisitor(TestEventOnlineCommon, WebsiteVisitorTests):
+
+    def test_clean_inactive_visitors_event(self):
+        """ Visitors registered to events should not be deleted even if not connected recently. """
+        active_visitors = self.env['website.visitor'].create([{
+            'name': 'Lead Douglas',
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'last_connection_datetime': datetime.now() - timedelta(days=8),
+            'event_registration_ids': [(0, 0, {
+                'event_id': self.event_0.id
+            })]
+        }])
+
+        self._test_unlink_old_visitors(self.env['website.visitor'], active_visitors)
+
+    def test_link_to_visitor_event(self):
+        """ Same as parent's 'test_link_to_visitor' except we also test that event
+        registrations are merged into main visitor. """
+        [main_visitor, linked_visitor] = self.env['website.visitor'].create([
+            self._prepare_main_visitor_data(),
+            self._prepare_linked_visitor_data()
+        ])
 
-    def test_visitor_events(self):
         event_1 = self.env['event.event'].create({
             'name': 'OtherEvent',
             'auto_confirm': True,
             'date_begin': fields.Datetime.to_string(datetime.today() + timedelta(days=1)),
             'date_end': fields.Datetime.to_string(datetime.today() + timedelta(days=15)),
         })
-
-        [main_visitor, child_visitor] = self.env['website.visitor'].create([{
-            'name': 'Main Visitor',
-            'event_registration_ids': [(0, 0, {
-                'event_id': self.event_0.id
-            })]
-        }, {
-            'name': 'Child Visitor',
+        linked_visitor.write({
             'event_registration_ids': [(0, 0, {
                 'event_id': event_1.id
             })]
-        }])
+        })
 
         self.assertEqual(self.event_0, main_visitor.event_registered_ids)
-        self.assertEqual(event_1, child_visitor.event_registered_ids)
-        self.assertEqual(
-            main_visitor,
-            self.env['website.visitor'].search([('event_registered_ids', 'in', self.event_0.ids)])
-        )
-        self.assertEqual(
-            child_visitor,
-            self.env['website.visitor'].search([('event_registered_ids', 'in', event_1.ids)])
-        )
-        child_visitor._link_to_visitor(main_visitor)
+        self.assertEqual(event_1, linked_visitor.event_registered_ids)
+
+        linked_visitor._link_to_visitor(main_visitor)
+        self.assertVisitorDeactivated(linked_visitor, main_visitor)
+
+        # main_visitor is now attending both events
         self.assertEqual(self.event_0 | event_1, main_visitor.event_registered_ids)
-        self.assertEqual(
-            main_visitor | child_visitor,
-            self.env['website.visitor'].with_context(active_test=False).search([('event_registered_ids', 'in', self.event_0.ids)])
-        )
-        self.assertEqual(
-            main_visitor | child_visitor,
-            self.env['website.visitor'].with_context(active_test=False).search([('event_registered_ids', 'in', event_1.ids)])
-        )
+
+    def _prepare_main_visitor_data(self):
+        values = super()._prepare_main_visitor_data()
+        values.update({
+            'event_registration_ids': [(0, 0, {
+                'event_id': self.event_0.id
+            })]
+        })
+        return values

addons/website_event_track/data/event_track_visitor_demo.xml

@@ -3,12 +3,12 @@
 
     <record id="event_7_track_2_visitor_0" model="event.track.visitor">
         <field name="track_id" ref="website_event_track.event_7_track_2"/>
-        <field name="visitor_id" ref="website_event.website_visitor_event_0"/>
+        <field name="visitor_id" ref="website.website_visitor_0"/>
         <field name="is_wishlisted" eval="True"/>
     </record>
     <record id="event_7_track_2_visitor_2" model="event.track.visitor">
         <field name="track_id" ref="website_event_track.event_7_track_2"/>
-        <field name="visitor_id" ref="website_event.website_visitor_event_2"/>
+        <field name="visitor_id" ref="website.website_visitor_2"/>
         <field name="is_wishlisted" eval="True"/>
     </record>
 

addons/website_event_track/models/website_visitor.py

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 from odoo import api, fields, models
+from odoo.osv import expression
 
 
 class WebsiteVisitor(models.Model):
@@ -21,27 +22,21 @@ class WebsiteVisitor(models.Model):
         compute="_compute_event_track_wishlisted_ids", compute_sudo=True,
         groups='event.group_event_user')
 
-    @api.depends('parent_id', 'event_track_visitor_ids.track_id', 'event_track_visitor_ids.is_wishlisted')
+    @api.depends('event_track_visitor_ids.track_id', 'event_track_visitor_ids.is_wishlisted')
     def _compute_event_track_wishlisted_ids(self):
-        # include parent's track visitors in a visitor o2m field. We don't add
-        # child one as child should not have track visitors (moved to the parent)
-        all_visitors = self + self.parent_id
         results = self.env['event.track.visitor'].read_group(
-            [('visitor_id', 'in', all_visitors.ids), ('is_wishlisted', '=', True)],
+            [('visitor_id', 'in', self.ids), ('is_wishlisted', '=', True)],
             ['visitor_id', 'track_id:array_agg'],
             ['visitor_id']
         )
         track_ids_map = {result['visitor_id'][0]: result['track_id'] for result in results}
         for visitor in self:
-            visitor_track_ids = track_ids_map.get(visitor.id, [])
-            parent_track_ids = track_ids_map.get(visitor.parent_id.id, [])
-            visitor.event_track_wishlisted_ids = visitor_track_ids + [track_id for track_id in parent_track_ids if track_id not in visitor_track_ids]
+            visitor.event_track_wishlisted_ids = track_ids_map.get(visitor.id, [])
             visitor.event_track_wishlisted_count = len(visitor.event_track_wishlisted_ids)
 
     def _search_event_track_wishlisted_ids(self, operator, operand):
         """ Search visitors with terms on wishlisted tracks. E.g. [('event_track_wishlisted_ids',
-        'in', [1, 2])] should return visitors having wishlisted tracks 1, 2 as
-        well as their children for notification purpose. """
+        'in', [1, 2])] should return visitors having wishlisted tracks 1, 2. """
         if operator == "not in":
             raise NotImplementedError("Unsupported 'Not In' operation on track wishlist visitors")
 
@@ -49,17 +44,14 @@ class WebsiteVisitor(models.Model):
             ('track_id', operator, operand),
             ('is_wishlisted', '=', True)
         ])
-        if track_visitors:
-            visitors = track_visitors.visitor_id
-            # search children, even archived one, to contact them
-            children = self.env['website.visitor'].with_context(
-                active_test=False
-            ).sudo().search([('parent_id', 'in', visitors.ids)])
-            visitor_ids = (visitors + children).ids
-        else:
-            visitor_ids = []
 
-        return [('id', 'in', visitor_ids)]
+        return [('id', 'in', track_visitors.visitor_id.ids)]
+
+    def _inactive_visitors_domain(self):
+        """ Visitors registered to push subscriptions are considered always active and should not be
+        deleted. """
+        domain = super()._inactive_visitors_domain()
+        return expression.AND([domain, [('event_track_visitor_ids', '=', False)]])
 
     def _link_to_partner(self, partner, update_values=None):
         """ Propagate partner update to track_visitor records """
@@ -69,7 +61,7 @@ class WebsiteVisitor(models.Model):
                 track_visitor_wo_partner.partner_id = partner
         super(WebsiteVisitor, self)._link_to_partner(partner, update_values=update_values)
 
-    def _link_to_visitor(self, target, keep_unique=True):
+    def _link_to_visitor(self, target):
         """ Override linking process to link wishlist to the final visitor. """
         self.event_track_visitor_ids.visitor_id = target.id
-        return super(WebsiteVisitor, self)._link_to_visitor(target, keep_unique=keep_unique)
+        return super(WebsiteVisitor, self)._link_to_visitor(target)

addons/website_event_track/tests/__init__.py

@@ -4,3 +4,4 @@
 from . import test_event_menus
 from . import test_track_internals
 from . import test_website_event
+from . import test_website_visitor

addons/website_event_track/tests/test_website_visitor.py

@@ -0,0 +1,70 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from datetime import datetime, timedelta
+
+from odoo.addons.website.tests.test_website_visitor import WebsiteVisitorTests
+from odoo.addons.website_event.tests.common import TestEventOnlineCommon
+from odoo.tests import tagged
+
+
+@tagged('website_visitor')
+class WebsiteVisitorTestsEventTrack(TestEventOnlineCommon, WebsiteVisitorTests):
+
+    def test_clean_inactive_visitors_event_track(self):
+        """ Visitors that have wishlisted tracks should not be deleted even if not connected
+        recently. """
+        track_1 = self.env['event.track'].create({
+            'name': 'Track 1',
+            'event_id': self.event_0.id,
+        })
+
+        active_visitors = self.env['website.visitor'].create([{
+            'name': 'Wishlister Alex',
+            'lang_id': self.env.ref('base.lang_en').id,
+            'country_id': self.env.ref('base.be').id,
+            'website_id': 1,
+            'last_connection_datetime': datetime.now() - timedelta(days=8),
+            'event_track_visitor_ids': [(0, 0, {
+                'track_id': track_1.id,
+                'is_wishlisted': True
+            })]
+        }])
+
+        self._test_unlink_old_visitors(self.env['website.visitor'], active_visitors)
+
+    def test_link_to_visitor_event_track(self):
+        """ Same as parent's 'test_link_to_visitor' except we also test that tracks
+        that are wishlisted are merged into main visitor. """
+
+        [track_1, track_2] = self.env['event.track'].create([{
+            'name': 'Track 1',
+            'event_id': self.event_0.id,
+        }, {
+            'name': 'Track 2',
+            'event_id': self.event_0.id,
+        }])
+
+        [main_visitor, linked_visitor] = self.env['website.visitor'].create([
+            self._prepare_main_visitor_data(),
+            self._prepare_linked_visitor_data()
+        ])
+
+        self.env['event.track.visitor'].create([{
+            'visitor_id': main_visitor.id,
+            'track_id': track_1.id,
+            'is_wishlisted': True,
+        }, {
+            'visitor_id': linked_visitor.id,
+            'track_id': track_2.id,
+            'is_wishlisted': True,
+        }])
+
+        linked_visitor._link_to_visitor(main_visitor)
+
+        self.assertVisitorDeactivated(linked_visitor, main_visitor)
+
+        # wishlisted tracks of both visitors should be merged into main one
+        self.assertEqual(
+            main_visitor.event_track_wishlisted_ids,
+            track_1 | track_2)

addons/website_event_track_quiz/data/quiz_demo.xml

@@ -133,21 +133,21 @@
 
     <record id="event_track_visitor_admin_event_7_track_1" model="event.track.visitor">
         <field name="track_id" ref="website_event_track.event_7_track_1"/>
-        <field name="visitor_id" ref="website_event.website_visitor_event_0"/>
+        <field name="visitor_id" ref="website.website_visitor_0"/>
         <field name="quiz_completed" eval="True"/>
         <field name="quiz_points">3</field>
     </record>
 
     <record id="event_track_visitor_demo_event_7_track_1" model="event.track.visitor">
         <field name="track_id" ref="website_event_track.event_7_track_1"/>
-        <field name="visitor_id" ref="website_event.website_visitor_event_1"/>
+        <field name="visitor_id" ref="website.website_visitor_1"/>
         <field name="quiz_completed" eval="True"/>
         <field name="quiz_points">2</field>
     </record>
 
     <record id="event_track_visitor_portal_event_7_track_1" model="event.track.visitor">
         <field name="track_id" ref="website_event_track.event_7_track_1"/>
-        <field name="visitor_id" ref="website_event.website_visitor_event_2"/>
+        <field name="visitor_id" ref="website.website_visitor_2"/>
         <field name="quiz_completed" eval="True"/>
         <field name="quiz_points">1</field>
     </record>

addons/website_livechat/models/website_visitor.py

@@ -92,11 +92,11 @@ class WebsiteVisitor(models.Model):
                 notifications.append([operator.partner_id, 'website_livechat.send_chat_request', mail_channel_info])
             self.env['bus.bus']._sendmany(notifications)
 
-    def _link_to_visitor(self, target, keep_unique=True):
+    def _link_to_visitor(self, target):
         """ Copy sessions of the secondary visitors to the main partner visitor. """
         if target.partner_id:
             target.mail_channel_ids |= self.mail_channel_ids
-        super(WebsiteVisitor, self)._link_to_visitor(target, keep_unique=keep_unique)
+        super(WebsiteVisitor, self)._link_to_visitor(target)
 
     def _link_to_partner(self, partner, update_values=None):
         """ Adapt partner in members of related livechats """

addons/website_livechat/tests/__init__.py

@@ -4,3 +4,4 @@ from . import common
 from . import test_ui
 from . import test_livechat_basic_flow
 from . import test_livechat_request
+from . import test_website_visitor

addons/website_livechat/tests/test_website_visitor.py

@@ -0,0 +1,45 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo.addons.website.tests.test_website_visitor import WebsiteVisitorTests
+from odoo.tests import tagged
+
+
+@tagged('website_visitor')
+class WebsiteVisitorTestsLivechat(WebsiteVisitorTests):
+
+    def test_link_to_visitor_livechat(self):
+        """ Same as parent's 'test_link_to_visitor' except we also test that conversations
+        are merged into main visitor. """
+        [main_visitor, linked_visitor] = self.env['website.visitor'].create([
+            self._prepare_main_visitor_data(),
+            self._prepare_linked_visitor_data()
+        ])
+        all_mail_channels = (main_visitor + linked_visitor).mail_channel_ids
+        linked_visitor._link_to_visitor(main_visitor)
+
+        self.assertVisitorDeactivated(linked_visitor, main_visitor)
+
+        # conversations of both visitors should be merged into main one
+        self.assertEqual(len(main_visitor.mail_channel_ids), 2)
+        self.assertEqual(main_visitor.mail_channel_ids, all_mail_channels)
+
+    def _prepare_main_visitor_data(self):
+        values = super()._prepare_main_visitor_data()
+        test_partner = self.env['res.partner'].create({'name': 'John Doe'})
+        values.update({
+            'partner_id': test_partner.id,
+            'mail_channel_ids': [(0, 0, {
+                'name': 'Conversation 1'
+            })]
+        })
+        return values
+
+    def _prepare_linked_visitor_data(self):
+        values = super()._prepare_linked_visitor_data()
+        values.update({
+            'mail_channel_ids': [(0, 0, {
+                'name': 'Conversation 2'
+            })]
+        })
+        return values

addons/website_livechat/views/website_visitor_views.xml

@@ -34,9 +34,10 @@
         <field name="arch" type="xml">
             <field name="page_ids" position="after">
                 <field name="livechat_operator_id"/>
+                <field name="session_count"/>
             </field>
             <xpath expr="//div[hasclass('w_visitor_kanban_actions')]" position="before">
-                <div>Chats<span class="float-right font-weight-bold"><field name="session_count"/></span></div>
+                <div t-if="record.session_count.raw_value">Chats<span class="float-right font-weight-bold"><field name="session_count"/></span></div>
                 <div t-if="record.livechat_operator_id.raw_value">
                     Speaking With
                     <div name="livechat_operator_id"
@@ -53,8 +54,10 @@
             </xpath>
             <xpath expr="//div[hasclass('w_visitor_kanban_actions_ungrouped')]" position="before">
                 <div class="col-lg col-sm-4 col-6 py-0 my-2">
-                    <b><field name="session_count"/></b>
-                    <div>Chats</div>
+                    <span t-att-class="record.session_count.raw_value ? 'font-weight-bold' : 'text-muted'">
+                        <field name="session_count"/>
+                    </span>
+                    <div t-att-class="record.session_count.raw_value ? '' : 'text-muted'">Chats</div>
                 </div>
                 <div t-if="record.livechat_operator_id.raw_value" class="col-lg col-sm-4 col-6 py-0 my-2">
                     <div>