From f8b87c959ab37a6128f486d15f1fbe218dec3662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 3 Jul 2020 10:14:18 +0000 Subject: [PATCH 1/5] [IMP] base: have user_admin / partner_admin available in HttpCaseWithUserDemo on self Purpose is to simplify future tests to have admin data directly available at hand, both its user and partner. It will ease future test writing. 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: f51fd6e54da5ee8aee0df74e6d510dc0b077eba8 --- odoo/addons/base/tests/common.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/odoo/addons/base/tests/common.py b/odoo/addons/base/tests/common.py index 4d7e7e00c87..fcaa4df8be1 100644 --- a/odoo/addons/base/tests/common.py +++ b/odoo/addons/base/tests/common.py @@ -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 From 150a24ad8147584903f1ce70de143019f4f2508a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 3 Jul 2020 10:02:06 +0000 Subject: [PATCH 2/5] [IMP] website: use HttpCaseWithUserDemo for tests and clean boostrapping 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 and clean bootstrapping of data. 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. 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: 4e0d4e7c315b5a9d880754e02966debd6a59c728 --- addons/website/models/website_visitor.py | 2 +- addons/website/tests/test_website_visitor.py | 138 +++++++++++-------- 2 files changed, 81 insertions(+), 59 deletions(-) diff --git a/addons/website/models/website_visitor.py b/addons/website/models/website_visitor.py index 7ff7702df94..34b0f0cd05a 100644 --- a/addons/website/models/website_visitor.py +++ b/addons/website/models/website_visitor.py @@ -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 diff --git a/addons/website/tests/test_website_visitor.py b/addons/website/tests/test_website_visitor.py index 0179bfacef4..90b22edf568 100644 --- a/addons/website/tests/test_website_visitor.py +++ b/addons/website/tests/test_website_visitor.py @@ -1,31 +1,53 @@ # 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({ + + untracked_view = self.env['ir.ui.view'].create({ 'name': 'Base', 'type': 'qweb', 'arch': ''' - I am a generic page + I am a generic page² ''', 'key': 'test.base_view', 'track': False, }) - tracked_view = View.create({ + tracked_view = self.env['ir.ui.view'].create({ 'name': 'Base', 'type': 'qweb', 'arch': ''' @@ -36,7 +58,7 @@ class WebsiteVisitorTests(tests.HttpCase): 'key': 'test.base_view', 'track': True, }) - tracked_view_2 = View.create({ + tracked_view_2 = self.env['ir.ui.view'].create({ 'name': 'Base', 'type': 'qweb', 'arch': ''' @@ -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', @@ -66,44 +88,44 @@ 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.assertEqual(len(self.env['website.visitor'].search([])), 0, "No visitor at the moment") + self.assertEqual(len(self.env['website.track'].search([])), 0, "No track at the moment") + self.url_open(self.untracked_page.url) + self.url_open(self.tracked_page.url) + self.url_open(self.tracked_page.url) + self.assertEqual(len(self.env['website.visitor'].search([])), 1, "1 visitor should be created") + self.assertEqual(len(self.env['website.track'].search([])), 1, "There should be 1 tracked page") # admin connects - visitor_admin = self.Visitor.search([]) + visitor_admin = self.env['website.visitor'].search([]) self.cookies = {'visitor_uuid': visitor_admin.access_token} with MockRequest(self.env, website=self.website, cookies=self.cookies): self.authenticate('admin', 'admin') # 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") + self.assertEqual(visitor_admin.partner_id, self.env['res.users'].browse(self.session.uid).partner_id, "self.env['website.visitor'] should be linked with connected partner") # 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.assertEqual(len(self.env['website.visitor'].search([])), 1, "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.assertEqual(len(self.env['website.visitor'].search([])), 2, "One extra visitor should be created") + visitor_portal = self.env['website.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") + self.assertEqual(visitor_portal.partner_id, self.env['res.users'].browse(self.session.uid).partner_id, "self.env['website.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") @@ -111,28 +133,28 @@ class WebsiteVisitorTests(tests.HttpCase): 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.assertEqual(len(self.env['website.visitor'].search([])), 3, "One extra visitor should be created") + visitor = self.env['website.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") + self.assertFalse(visitor.partner_id, "self.env['website.visitor'] should not be linked to any partner") # admin connects with MockRequest(self.env, website=self.website, cookies=self.cookies): self.authenticate('admin', 'admin') # one visitor is deleted - self.assertEqual(len(self.Visitor.search([])), 2, "One visitor should be deleted") + self.assertEqual(len(self.env['website.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_admin = self.env['website.visitor'].search([('partner_id', '=', admin_partner_id.id)]) # tracks are linked self.assertEqual(len(visitor_admin.website_track_ids), 4, "There should be 4 tracked page for the admin") @@ -140,28 +162,28 @@ class WebsiteVisitorTests(tests.HttpCase): 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.assertEqual(len(self.env['website.visitor'].search([])), 3, "One extra visitor should be created") + visitor = self.env['website.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") + self.assertFalse(visitor.partner_id, "self.env['website.visitor'] should not be linked to any partner") # portal user connects with MockRequest(self.env, website=self.website, cookies=self.cookies): self.authenticate('portal', 'portal') # one visitor is deleted - self.assertEqual(len(self.Visitor.search([])), 2, "One visitor should be deleted") + self.assertEqual(len(self.env['website.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)]) + visitor_portal = self.env['website.visitor'].search([('partner_id', '=', portal_partner_id.id)]) # tracks are linked self.assertEqual(len(visitor_portal.website_track_ids), 4, "There should be 4 tracked page for the portal user") @@ -170,21 +192,21 @@ class WebsiteVisitorTests(tests.HttpCase): track.write({'visit_datetime': track.visit_datetime - timedelta(minutes=30)}) # visit a page - self.url_open(self.tracked_view.url) + self.url_open(self.tracked_page.url) visitor_portal.refresh() # 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) + self.url_open(self.tracked_page.url) visitor_portal.refresh() # 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({ + 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, @@ -195,16 +217,16 @@ class WebsiteVisitorTests(tests.HttpCase): # archive old visitor old_visitor.last_connection_datetime = datetime.now() - timedelta(days=8) - self.Visitor._cron_archive_visitors() + self.env['website.visitor']._cron_archive_visitors() self.assertEqual(old_visitor.active, False, "The visitor should be archived after one week of 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 + self.url_open(self.tracked_page.url) + new_visitor = self.env['website.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, + self.assertEqual(len(self.env['website.track'].search([('visitor_id', '=', new_visitor.id)])), 1, "A track for the new visitor should be created after visiting a tracked view") # override the get_visitor_from_request to mock that is new_visitor that authenticates @@ -215,6 +237,6 @@ class WebsiteVisitorTests(tests.HttpCase): 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.") From 17d83775dbce251f49da8ca35214211657beb6b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 3 Jul 2020 11:56:46 +0000 Subject: [PATCH 3/5] [IMP] website: improve visitor 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 Make tests independent from existing database data, notably existing visitors. Use newly-introduced tools and data. Clean tests and make them easier to understand, notably connection / disconnection effects. Make more tests about visitor data: name, partner_id, tracks move from visitor to authenticatedf visitor, ... 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: 015f70dd1ce1394d279f8fbb949948f4845c65fc --- addons/website/tests/test_website_visitor.py | 209 ++++++++++++------- 1 file changed, 135 insertions(+), 74 deletions(-) diff --git a/addons/website/tests/test_website_visitor.py b/addons/website/tests/test_website_visitor.py index 90b22edf568..572f4f8f8b8 100644 --- a/addons/website/tests/test_website_visitor.py +++ b/addons/website/tests/test_website_visitor.py @@ -37,7 +37,7 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): self.cookies = {} untracked_view = self.env['ir.ui.view'].create({ - 'name': 'Base', + 'name': 'UntackedView', 'type': 'qweb', 'arch': ''' @@ -48,7 +48,7 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): 'track': False, }) tracked_view = self.env['ir.ui.view'].create({ - 'name': 'Base', + 'name': 'TrackedView', 'type': 'qweb', 'arch': ''' @@ -59,7 +59,7 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): 'track': True, }) tracked_view_2 = self.env['ir.ui.view'].create({ - 'name': 'Base', + 'name': 'TrackedView2', 'type': 'qweb', 'arch': ''' @@ -87,33 +87,82 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): }, ]) - def test_create_visitor_on_tracked_page(self): - self.assertEqual(len(self.env['website.visitor'].search([])), 0, "No visitor at the moment") - self.assertEqual(len(self.env['website.track'].search([])), 0, "No track at the moment") + 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])], + }) + + 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) - self.assertEqual(len(self.env['website.visitor'].search([])), 1, "1 visitor should be created") - self.assertEqual(len(self.env['website.track'].search([])), 1, "There should be 1 tracked page") - # admin connects - visitor_admin = self.env['website.visitor'].search([]) - self.cookies = {'visitor_uuid': visitor_admin.access_token} + 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_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.env['website.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.env['website.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_page.url) self.url_open(self.untracked_page.url) @@ -121,13 +170,16 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): 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.env['website.visitor'].search([])), 2, "One extra visitor should be created") - visitor_portal = self.env['website.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.env['website.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() @@ -139,26 +191,37 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): 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.env['website.visitor'].search([])), 3, "One extra visitor should be created") - visitor = self.env['website.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.env['website.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.env['website.visitor'].search([])), 2, "One visitor should be deleted") - admin_partner_id = self.env['res.users'].browse(self.session.uid).partner_id - visitor_admin = self.env['website.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 @@ -168,23 +231,26 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): 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.env['website.visitor'].search([])), 3, "One extra visitor should be created") - visitor = self.env['website.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.env['website.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.env['website.visitor'].search([])), 2, "One visitor should be deleted") - portal_partner_id = self.env['res.users'].browse(self.session.uid).partner_id - visitor_portal = self.env['website.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 @@ -193,50 +259,45 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): # visit a page self.url_open(self.tracked_page.url) - visitor_portal.refresh() + 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_page.url) - visitor_portal.refresh() + 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 + def test_visitor_archive(self): + """ Test cron archiving inactive visitors and their re-activation when + authenticating an user. """ + 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.env['website.visitor']._cron_archive_visitors() - self.assertEqual(old_visitor.active, False, "The visitor should be archived after one week of inactivity") + self.assertEqual(old_visitor.active, False, "Visitor should be archived after inactivity") # reconnect with new visitor. self.url_open(self.tracked_page.url) - new_visitor = self.env['website.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.env['website.track'].search([('visitor_id', '=', new_visitor.id)])), 1, - "A track for the new visitor should be created after visiting a tracked view") + new_visitor = self._get_last_visitor() + 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') + 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.env['website.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.") From 26488406246db84661412d200177fc16c1681e71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 1 Jul 2020 07:54:21 +0000 Subject: [PATCH 4/5] [REF] website: allow to configure delay before archiving visitors 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 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. Purpose of extending delay is to be able to use visitor information a bit longer in business flows. For example one could contact visitors 2 weeks after an event to get their feedback. Adding a bit of delay allow to keep those visitors alive a bit longer by default. Allowing to configure it gives more flexibility to admins and deployment. 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: f83af1d53716499dcad94f363aff609a2f8e55d9 Co-authored-by: Aurélien Warnon Co-authored-by: David Beguin Co-authored-by: Thibault Delavallée --- addons/website/models/website_visitor.py | 5 +++-- addons/website/tests/test_website_visitor.py | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/addons/website/models/website_visitor.py b/addons/website/models/website_visitor.py index 34b0f0cd05a..83a47f8554a 100644 --- a/addons/website/models/website_visitor.py +++ b/addons/website/models/website_visitor.py @@ -241,8 +241,9 @@ class WebsiteVisitor(models.Model): return self.sudo().create(vals) 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): diff --git a/addons/website/tests/test_website_visitor.py b/addons/website/tests/test_website_visitor.py index 572f4f8f8b8..80f50d76be4 100644 --- a/addons/website/tests/test_website_visitor.py +++ b/addons/website/tests/test_website_visitor.py @@ -273,6 +273,8 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): 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, 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 5/5] [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)])