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] [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.")