[FW][MERGE] website: improve visitor synchronization and tests

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

Clean existing visitor tests. Use HttpCaseWithUserDemo. Make tests
independent from existing database data, notably existing visitors.
Currently if any visitor is present in db tests crash as they are badly
designed.

Introduce a mock for visitor from request allowing to shortcut some visitor /
user synchronization and test directly expected results without too much
boilerplate in tests.

Add anchor methods to somehow merge visitors and update partner linked
to visitors and their sub records. 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.

Introduce a new configuration parameter in website allowing to set number of
days before de-activating visitors: ``website.visitor.live.days`` . It is
set to 30 days instead of 7 as before.

See sub commits for more details

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 odoo/odoo#54036

closes odoo/odoo#54078

Forward-port-of: odoo/odoo#54036
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
Odoo's Mergebot
2020-07-22 11:45:03 +02:00
committed by GitHub
4 changed files with 259 additions and 128 deletions
+22 -15
View File
@@ -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
+37 -3
View File
@@ -4,7 +4,7 @@
from datetime import datetime, timedelta
import uuid
from odoo import fields, models, api, registry, _
from odoo import fields, models, api, _
from odoo.addons.base.models.res_partner import _tz_get
from odoo.exceptions import UserError
from odoo.tools.misc import _format_time_ago
@@ -240,9 +240,43 @@ 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):
one_week_ago = datetime.now() - timedelta(days=7)
visitors_to_archive = self.env['website.visitor'].sudo().search([('last_connection_datetime', '<', one_week_ago)])
delay_days = int(self.env['ir.config_parameter'].sudo().get_param('website.visitor.live.days', 30))
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})
def _update_visitor_last_visit(self):
+197 -109
View File
@@ -1,32 +1,54 @@
# coding: utf-8
# Part of Odoo. See LICENSE file for full copyright and licensing details.
from contextlib import contextmanager
from datetime import datetime, timedelta
from unittest.mock import patch
from odoo import tests
from odoo.addons.base.tests.common import HttpCaseWithUserDemo
from odoo.addons.website.tools import MockRequest
from odoo.addons.website.models.website_visitor import WebsiteVisitor
from odoo.tests import common, tagged
class MockVisitor(common.BaseCase):
@contextmanager
def mock_visitor_from_request(self, force_visitor=False):
def _get_visitor_from_request(model, *args, **kwargs):
return force_visitor
with patch.object(WebsiteVisitor, '_get_visitor_from_request',
autospec=True, wraps=WebsiteVisitor,
side_effect=_get_visitor_from_request) as _get_visitor_from_request_mock:
yield
@tagged('-at_install', 'post_install', 'website_visitor')
class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
@tests.tagged('-at_install', 'post_install')
class WebsiteVisitorTests(tests.HttpCase):
def setUp(self):
super().setUp()
self.website = self.env['website'].browse(1)
super(WebsiteVisitorTests, self).setUp()
self.website = self.env['website'].search([
('company_id', '=', self.env.user.company_id.id)
], limit=1)
self.cookies = {}
self.Visitor = self.env['website.visitor']
self.Track = self.env['website.track']
Page = self.env['website.page']
View = self.env['ir.ui.view']
untracked_view = View.create({
'name': 'Base',
untracked_view = self.env['ir.ui.view'].create({
'name': 'UntackedView',
'type': 'qweb',
'arch': '''<t name="Homepage" t-name="website.base_view">
<t t-call="website.layout">
I am a generic page
I am a generic page²
</t>
</t>''',
'key': 'test.base_view',
'track': False,
})
tracked_view = View.create({
'name': 'Base',
tracked_view = self.env['ir.ui.view'].create({
'name': 'TrackedView',
'type': 'qweb',
'arch': '''<t name="Homepage" t-name="website.base_view">
<t t-call="website.layout">
@@ -36,8 +58,8 @@ class WebsiteVisitorTests(tests.HttpCase):
'key': 'test.base_view',
'track': True,
})
tracked_view_2 = View.create({
'name': 'Base',
tracked_view_2 = self.env['ir.ui.view'].create({
'name': 'TrackedView2',
'type': 'qweb',
'arch': '''<t name="OtherPage" t-name="website.base_view">
<t t-call="website.layout">
@@ -47,7 +69,7 @@ class WebsiteVisitorTests(tests.HttpCase):
'key': 'test.base_view',
'track': True,
})
[self.untracked_view, self.tracked_view, self.tracked_view_2] = Page.create([
[self.untracked_page, self.tracked_page, self.tracked_page_2] = self.env['website.page'].create([
{
'view_id': untracked_view.id,
'url': '/untracked_view',
@@ -65,104 +87,170 @@ class WebsiteVisitorTests(tests.HttpCase):
},
])
def test_create_visitor_on_tracked_page(self):
self.assertEqual(len(self.Visitor.search([])), 0, "No visitor at the moment")
self.assertEqual(len(self.Track.search([])), 0, "No track at the moment")
self.url_open(self.untracked_view.url)
self.url_open(self.tracked_view.url)
self.url_open(self.tracked_view.url)
self.assertEqual(len(self.Visitor.search([])), 1, "1 visitor should be created")
self.assertEqual(len(self.Track.search([])), 1, "There should be 1 tracked page")
self.user_portal = self.env['res.users'].search([('login', '=', 'portal')])
self.partner_portal = self.user_portal.partner_id
if not self.user_portal:
self.env['ir.config_parameter'].sudo().set_param('auth_password_policy.minlength', 4)
self.partner_portal = self.env['res.partner'].create({
'name': 'Joel Willis',
'email': 'joel.willis63@example.com',
})
self.user_portal = self.env['res.users'].create({
'login': 'portal',
'password': 'portal',
'partner_id': self.partner_portal.id,
'groups_id': [(6, 0, [self.env.ref('base.group_portal').id])],
})
# admin connects
visitor_admin = self.Visitor.search([])
self.cookies = {'visitor_uuid': visitor_admin.access_token}
def _get_last_visitor(self):
return self.env['website.visitor'].search([], limit=1, order="id DESC")
def assertPageTracked(self, visitor, page):
""" Check a page is in visitor tracking data """
self.assertIn(page, visitor.website_track_ids.page_id)
self.assertIn(page, visitor.page_ids)
def assertVisitorTracking(self, visitor, pages):
""" Check the whole tracking history of a visitor """
for page in pages:
self.assertPageTracked(visitor, page)
self.assertEqual(
len(visitor.website_track_ids),
len(pages)
)
def test_visitor_creation_on_tracked_page(self):
""" Test various flows involving visitor creation and update. """
existing_visitors = self.env['website.visitor'].search([])
existing_tracks = self.env['website.track'].search([])
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page.url)
self.url_open(self.tracked_page.url)
new_visitor = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
new_track = self.env['website.track'].search([('id', 'not in', existing_tracks.ids)])
self.assertEqual(len(new_visitor), 1, "1 visitor should be created")
self.assertEqual(len(new_track), 1, "There should be 1 tracked page")
self.assertEqual(new_visitor.visit_count, 1)
self.assertEqual(new_visitor.website_track_ids, new_track)
self.assertVisitorTracking(new_visitor, self.tracked_page)
# ------------------------------------------------------------
# Admin connects
# ------------------------------------------------------------
self.cookies = {'visitor_uuid': new_visitor.access_token}
with MockRequest(self.env, website=self.website, cookies=self.cookies):
self.authenticate('admin', 'admin')
self.authenticate(self.user_admin.login, 'admin')
visitor_admin = new_visitor
# visit a page
self.url_open(self.tracked_view_2.url)
self.url_open(self.tracked_page_2.url)
visitor_admin.refresh()
# page is tracked
self.assertEqual(len(visitor_admin.website_track_ids), 2, "There should be 2 tracked pages for the admin")
# visitor is linked
self.assertEqual(visitor_admin.partner_id, self.env['res.users'].browse(self.session.uid).partner_id, "self.Visitor should be linked with connected partner")
# check tracking and visitor / user sync
self.assertVisitorTracking(visitor_admin, self.tracked_page | self.tracked_page_2)
self.assertEqual(visitor_admin.partner_id, self.partner_admin)
self.assertEqual(visitor_admin.name, self.partner_admin.name)
# ------------------------------------------------------------
# Portal connects
# ------------------------------------------------------------
# portal user connects
with MockRequest(self.env, website=self.website, cookies=self.cookies):
self.authenticate('portal', 'portal')
self.assertEqual(len(self.Visitor.search([])), 1, "No extra visitor should be created")
self.authenticate(self.user_portal.login, 'portal')
self.assertFalse(
self.env['website.visitor'].search([('id', 'not in', (existing_visitors | visitor_admin).ids)]),
"No extra visitor should be created")
# visit a page
self.url_open(self.tracked_view.url)
self.url_open(self.untracked_view.url)
self.url_open(self.tracked_view_2.url)
self.url_open(self.tracked_view_2.url) # 2 time to be sure it does not record twice
self.url_open(self.tracked_page.url)
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page_2.url)
self.url_open(self.tracked_page_2.url) # 2 time to be sure it does not record twice
# new visitor is created
self.assertEqual(len(self.Visitor.search([])), 2, "One extra visitor should be created")
visitor_portal = self.Visitor.search([])[0]
self.cookies['visitor_uuid'] = visitor_portal.access_token
# visitor is linked
self.assertEqual(visitor_portal.partner_id, self.env['res.users'].browse(self.session.uid).partner_id, "self.Visitor should be linked with connected partner")
# tracks are created
self.assertEqual(len(visitor_portal.website_track_ids), 2, "There should be 2 tracked pages for the portal user")
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(len(new_visitors), 2, "One extra visitor should be created")
visitor_portal = new_visitors[0]
self.assertEqual(visitor_portal.partner_id, self.partner_portal)
self.assertEqual(visitor_portal.name, self.partner_portal.name)
self.assertVisitorTracking(visitor_portal, self.tracked_page | self.tracked_page_2)
# ------------------------------------------------------------
# Back to anonymous
# ------------------------------------------------------------
# portal user disconnects
self.logout()
# visit some pages
self.url_open(self.tracked_view.url)
self.url_open(self.untracked_view.url)
self.url_open(self.tracked_view_2.url)
self.url_open(self.tracked_view_2.url) # 2 time to be sure it does not record twice
self.url_open(self.tracked_page.url)
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page_2.url)
self.url_open(self.tracked_page_2.url) # 2 time to be sure it does not record twice
# new visitor is created
self.assertEqual(len(self.Visitor.search([])), 3, "One extra visitor should be created")
visitor = self.Visitor.search([])[0]
self.cookies['visitor_uuid'] = visitor.access_token
# tracks are created
self.assertEqual(len(visitor.website_track_ids), 2, "There should be 2 tracked page for the visitor")
# visitor is not linked
self.assertFalse(visitor.partner_id, "self.Visitor should not be linked to any partner")
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(len(new_visitors), 3, "One extra visitor should be created")
visitor_anonymous = new_visitors[0]
self.cookies['visitor_uuid'] = visitor_anonymous.access_token
self.assertFalse(visitor_anonymous.name)
self.assertFalse(visitor_anonymous.partner_id)
self.assertVisitorTracking(visitor_anonymous, self.tracked_page | self.tracked_page_2)
visitor_anonymous_tracks = visitor_anonymous.website_track_ids
# ------------------------------------------------------------
# Admin connects again
# ------------------------------------------------------------
# admin connects
with MockRequest(self.env, website=self.website, cookies=self.cookies):
self.authenticate('admin', 'admin')
self.authenticate(self.user_admin.login, 'admin')
# one visitor is deleted
self.assertEqual(len(self.Visitor.search([])), 2, "One visitor should be deleted")
admin_partner_id = self.env['res.users'].browse(self.session.uid).partner_id
visitor_admin = self.Visitor.search([('partner_id', '=', admin_partner_id.id)])
visitor_anonymous = self.env['website.visitor'].with_context(active_test=False).search([('id', '=', visitor_anonymous.id)])
self.assertFalse(visitor_anonymous)
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(new_visitors, visitor_admin | visitor_portal)
visitor_admin = self.env['website.visitor'].search([('partner_id', '=', self.partner_admin.id)])
# tracks are linked
self.assertTrue(visitor_anonymous_tracks < visitor_admin.website_track_ids)
self.assertEqual(len(visitor_admin.website_track_ids), 4, "There should be 4 tracked page for the admin")
# admin user disconnects
# ------------------------------------------------------------
# Back to anonymous
# ------------------------------------------------------------
# admin disconnects
self.logout()
# visit some pages
self.url_open(self.tracked_view.url)
self.url_open(self.untracked_view.url)
self.url_open(self.tracked_view_2.url)
self.url_open(self.tracked_view_2.url) # 2 time to be sure it does not record twice
self.url_open(self.tracked_page.url)
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page_2.url)
self.url_open(self.tracked_page_2.url) # 2 time to be sure it does not record twice
# new visitor created
self.assertEqual(len(self.Visitor.search([])), 3, "One extra visitor should be created")
visitor = self.Visitor.search([])[0]
self.cookies['visitor_uuid'] = visitor.access_token
# tracks are created
self.assertEqual(len(visitor.website_track_ids), 2, "There should be 2 tracked page for the visitor")
# visitor is not linked
self.assertFalse(visitor.partner_id, "self.Visitor should not be linked to any partner")
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(len(new_visitors), 3, "One extra visitor should be created")
visitor_anonymous_2 = new_visitors[0]
self.cookies['visitor_uuid'] = visitor_anonymous_2.access_token
self.assertFalse(visitor_anonymous_2.name)
self.assertFalse(visitor_anonymous_2.partner_id)
self.assertVisitorTracking(visitor_anonymous_2, self.tracked_page | self.tracked_page_2)
visitor_anonymous_2_tracks = visitor_anonymous_2.website_track_ids
# portal user connects
# ------------------------------------------------------------
# Portal connects again
# ------------------------------------------------------------
with MockRequest(self.env, website=self.website, cookies=self.cookies):
self.authenticate('portal', 'portal')
self.authenticate(self.user_portal.login, 'portal')
# one visitor is deleted
self.assertEqual(len(self.Visitor.search([])), 2, "One visitor should be deleted")
portal_partner_id = self.env['res.users'].browse(self.session.uid).partner_id
visitor_portal = self.Visitor.search([('partner_id', '=', portal_partner_id.id)])
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(new_visitors, visitor_admin | visitor_portal)
# tracks are linked
self.assertTrue(visitor_anonymous_2_tracks < visitor_portal.website_track_ids)
self.assertEqual(len(visitor_portal.website_track_ids), 4, "There should be 4 tracked page for the portal user")
# simulate the portal user comes back 30min later
@@ -170,51 +258,51 @@ class WebsiteVisitorTests(tests.HttpCase):
track.write({'visit_datetime': track.visit_datetime - timedelta(minutes=30)})
# visit a page
self.url_open(self.tracked_view.url)
visitor_portal.refresh()
self.url_open(self.tracked_page.url)
visitor_portal.invalidate_cache(fnames=['website_track_ids'])
# tracks are created
self.assertEqual(len(visitor_portal.website_track_ids), 5, "There should be 5 tracked page for the portal user")
# simulate the portal user comes back 8hours later
visitor_portal.write({'last_connection_datetime': visitor_portal.last_connection_datetime - timedelta(hours=8)})
self.url_open(self.tracked_view.url)
visitor_portal.refresh()
self.url_open(self.tracked_page.url)
visitor_portal.invalidate_cache(fnames=['visit_count'])
# check number of visits
self.assertEqual(visitor_portal.visit_count, 2, "There should be 2 visits for the portal user")
def test_long_period_inactivity(self):
# link visitor to partner
old_visitor = self.Visitor.create({
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({
'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,
})
partner_demo = self.env.ref('base.partner_demo')
old_visitor.partner_id = partner_demo.id
self.assertEqual(partner_demo.visitor_ids.id, old_visitor.id, "The partner visitor should be set correctly.")
self.assertTrue(old_visitor.active)
self.assertEqual(partner_demo.visitor_ids, old_visitor, "Visitor and its partner should be synchronized")
# archive old visitor
old_visitor.last_connection_datetime = datetime.now() - timedelta(days=8)
self.Visitor._cron_archive_visitors()
self.assertEqual(old_visitor.active, False, "The visitor should be archived after one week of inactivity")
self.env['website.visitor']._cron_archive_visitors()
self.assertEqual(old_visitor.active, False, "Visitor should be archived after inactivity")
# reconnect with new visitor.
self.url_open(self.tracked_view.url)
new_visitor = self.Visitor.search([('id', '!=', old_visitor.id)], limit=1, order="id desc") # get the last created visitor
new_visitor_id = new_visitor.id
self.assertEqual(new_visitor_id > old_visitor.id, True, "A new visitor should have been created.")
self.assertEqual(len(new_visitor), 1, "A visitor should be created after visiting a tracked view")
self.assertEqual(len(self.Track.search([('visitor_id', '=', new_visitor.id)])), 1,
"A track for the new visitor should be created after visiting a tracked view")
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)
# override the get_visitor_from_request to mock that is new_visitor that authenticates
def get_visitor_from_request(self_mock, force_create=False):
return new_visitor
self.patch(type(self.env['website.visitor']), '_get_visitor_from_request', get_visitor_from_request)
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.")
self.authenticate('demo', 'demo')
self.assertEqual(partner_demo.visitor_ids.id, old_visitor.id, "The partner visitor should be back to the 'old' visitor.")
new_visitor = self.Visitor.search([('id', '=', new_visitor_id)])
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.")
+3 -1
View File
@@ -32,7 +32,9 @@ class HttpCaseWithUserDemo(HttpCase):
def setUp(self):
super(HttpCaseWithUserDemo, self).setUp()
self.env.ref('base.partner_admin').write({'name': 'Mitchell Admin'})
self.user_admin = self.env.ref('base.user_admin')
self.user_admin.write({'name': 'Mitchell Admin'})
self.partner_admin = self.user_admin.partner_id
self.user_demo = self.env['res.users'].search([('login', '=', 'demo')])
self.partner_demo = self.user_demo.partner_id