diff --git a/addons/test_event_full/tests/test_performance.py b/addons/test_event_full/tests/test_performance.py index c70bbf17c13..d2e6bc5a227 100644 --- a/addons/test_event_full/tests/test_performance.py +++ b/addons/test_event_full/tests/test_performance.py @@ -452,7 +452,7 @@ class TestOnlineEventPerformance(EventPerformanceCase, UtilPerf): # website customer data with freeze_time(self.reference_now): self.authenticate(None, None) - with self.assertQueryCount(default=39): # com runbot: 39 - ent runbot: 39 + with self.assertQueryCount(default=34): # com runbot: 34 - ent runbot: 34 self._test_url_open('/event') # @warmup diff --git a/addons/test_event_full/tests/test_wevent_register.py b/addons/test_event_full/tests/test_wevent_register.py index 32ec951e657..21861369dcf 100644 --- a/addons/test_event_full/tests/test_wevent_register.py +++ b/addons/test_event_full/tests/test_wevent_register.py @@ -37,10 +37,8 @@ class TestWEventRegister(TestWEventCommon): ) # check visitor stored information - self.assertEqual(visitor.name, "Raoulette Poiluchette") + self.assertEqual(visitor.display_name, "Raoulette Poiluchette") self.assertEqual(visitor.event_registration_ids, new_registrations) self.assertEqual(visitor.partner_id, self.env['res.partner']) self.assertEqual(visitor.mobile, "0456112233") self.assertEqual(visitor.email, "raoulette@example.com") - self.assertFalse(visitor.parent_id) - self.assertTrue(visitor.active) diff --git a/addons/website/data/website_visitor_demo.xml b/addons/website/data/website_visitor_demo.xml index b03141e5fc1..c8f3adeb81f 100644 --- a/addons/website/data/website_visitor_demo.xml +++ b/addons/website/data/website_visitor_demo.xml @@ -2,34 +2,22 @@ - Edwin Hansen - + - Soham Palmer - + - Philipe J. Fry - - - - Philipe J. Fry (old) - - - - - - + f9d24e459528093a790f38e5cc989a8d diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index 5d08d95812e..062f9a6947e 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -148,7 +148,7 @@ class Http(models.AbstractModel): view = template and request.env['website'].get_template(template) if view and view.track: - request.env['website.visitor']._handle_webpage_dispatch(response, website_page) + request.env['website.visitor']._handle_webpage_dispatch(website_page) return False diff --git a/addons/website/models/res_users.py b/addons/website/models/res_users.py index c03de1230e3..71a507e878c 100644 --- a/addons/website/models/res_users.py +++ b/addons/website/models/res_users.py @@ -2,7 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import logging -from odoo import api, fields, models, tools, _ +from odoo import api, fields, models, _ from odoo.exceptions import ValidationError from odoo.http import request @@ -69,32 +69,27 @@ 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. - 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. """ + If a visitor already exists for that user, assign it data from the + current anonymous visitor (if exists). + Purpose is to try to aggregate as much sub-records (tracked pages, + leads, ...) as possible. """ + visitor_pre_authenticate_sudo = None + if request and request.env: + visitor_pre_authenticate_sudo = request.env['website.visitor']._get_visitor_from_request() uid = super(ResUsers, cls).authenticate(db, login, password, user_agent_env) - if uid: + if uid and visitor_pre_authenticate_sudo: with cls.pool.cursor() as cr: env = api.Environment(cr, uid, {}) - visitor_sudo = env['website.visitor']._get_visitor_from_request() - if visitor_sudo: - 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) - visitor_main.name = user_partner.name - visitor_main.active = True - visitor_main._update_visitor_last_visit() - else: - 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() + user_partner = env.user.partner_id + visitor_current_user_sudo = env['website.visitor'].sudo().search([ + ('partner_id', '=', user_partner.id) + ], limit=1) + if visitor_current_user_sudo: + # A visitor exists for the logged in user, link public + # visitor records to it. + visitor_pre_authenticate_sudo._merge_visitor(visitor_current_user_sudo) + visitor_current_user_sudo._update_visitor_last_visit() + else: + visitor_pre_authenticate_sudo.access_token = user_partner.id + visitor_pre_authenticate_sudo._update_visitor_last_visit() return uid diff --git a/addons/website/models/website_visitor.py b/addons/website/models/website_visitor.py index cff017d8f97..6bb173cdbcd 100644 --- a/addons/website/models/website_visitor.py +++ b/addons/website/models/website_visitor.py @@ -2,7 +2,9 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from datetime import datetime, timedelta -import uuid +from psycopg2 import sql + +import hashlib import pytz from odoo import fields, models, api, _ @@ -28,14 +30,29 @@ class WebsiteTrack(models.Model): class WebsiteVisitor(models.Model): _name = 'website.visitor' _description = 'Website Visitor' - _order = 'last_connection_datetime DESC' + _order = 'id DESC' - name = fields.Char('Name') - access_token = fields.Char(required=True, default=lambda x: uuid.uuid4().hex, copy=False, groups='website.group_website_publisher') - active = fields.Boolean('Active', default=True) + def _get_access_token(self): + """ Either the user's partner.id or a hash. """ + if not request: + raise ValueError("Visitors can only be created through the frontend.") + + if not self.env.user._is_public(): + return self.env.user.partner_id.id + + msg = repr(( + request.httprequest.remote_addr, + request.httprequest.environ.get('HTTP_USER_AGENT'), + request.session.sid, + )).encode('utf-8') + # Keep same length (32) as before, it will ease the migration without + # any real downside + return hashlib.sha1(msg).hexdigest()[:32] + + name = fields.Char('Name', related='partner_id.name') + access_token = fields.Char(required=True, default=_get_access_token, copy=False) 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_id = fields.Many2one('res.partner', string="Contact", help="Partner of the last logged in user.", compute='_compute_partner_id', store=True, index='btree_not_null') partner_image = fields.Binary(related='partner_id.image_1920') # localisation and info @@ -62,19 +79,27 @@ class WebsiteVisitor(models.Model): _sql_constraints = [ ('access_token_unique', 'unique(access_token)', 'Access token should be unique.'), - ('partner_uniq', 'unique(partner_id)', 'A partner is linked to only one visitor.'), ] - @api.depends('name') + @api.depends('partner_id') def name_get(self): res = [] for record in self: res.append(( record.id, - record.name or _('Website Visitor #%s', record.id) + record.partner_id.name or _('Website Visitor #%s', record.id) )) return res + @api.depends('access_token') + def _compute_partner_id(self): + # The browse in the loop is fine, there is no SQL Query on partner here + for visitor in self: + # If the access_token is not a 32 length hexa string, it means that + # the visitor is linked to a logged in user, in which case its + # partner_id is used instead as the token. + visitor.partner_id = len(visitor.access_token) != 32 and self.env['res.partner'].browse([visitor.access_token]) + @api.depends('partner_id.email_normalized', 'partner_id.mobile', 'partner_id.phone') def _compute_email_phone(self): results = self.env['res.partner'].search_read( @@ -118,9 +143,10 @@ class WebsiteVisitor(models.Model): @api.depends('website_track_ids.page_id') def _compute_last_visited_page_id(self): - results = self.env['website.track']._read_group([('visitor_id', 'in', self.ids)], - ['visitor_id', 'page_id', 'visit_datetime:max'], - ['visitor_id', 'page_id'], lazy=False) + results = self.env['website.track']._read_group( + [('visitor_id', 'in', self.ids)], + ['visitor_id', 'page_id', 'visit_datetime:max'], + ['visitor_id', 'page_id'], lazy=False) mapped_data = {result['visitor_id'][0]: result['page_id'][0] for result in results if result['page_id']} for visitor in self: visitor.last_visited_page_id = mapped_data.get(visitor.id, False) @@ -166,88 +192,115 @@ class WebsiteVisitor(models.Model): 'context': compose_ctx, } - def _get_visitor_from_request(self, force_create=False): - """ Return the visitor as sudo from the request if there is a - visitor_uuid cookie. + def _upsert_visitor(self, access_token, force_track_values=None): + """ Based on the given `access_token`, either create or return the + related visitor if exists, through a single raw SQL UPSERT Query. - 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). + It will also create a tracking record if requested, in the same query. - 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). """ + :param access_token: token to be used to upsert the visitor + :param force_track_values: an optional dict to create a track at the + same time. + :return: a tuple containing the visitor id and the upsert result (either + `inserted` or `updated). + """ + country_code = request.geoip.get('country_code') + country_id = request.env['res.country'].sudo().search([ + ('code', '=', country_code) + ], limit=1).id if country_code else None + create_values = { + 'access_token': access_token, + 'lang_id': request.lang.id, + 'country_id': country_id, + 'website_id': request.website.id, + 'timezone': self._get_visitor_timezone() or None, + 'write_uid': self.env.uid, + 'create_uid': self.env.uid, + # If the access_token is not a 32 length hexa string, it means that the + # visitor is linked to a logged in user, in which case its partner_id is + # used instead as the token. + 'partner_id': None if len(str(access_token)) == 32 else access_token, + } + + query = """ + INSERT INTO website_visitor ( + partner_id, access_token, last_connection_datetime, visit_count, lang_id, + country_id, website_id, timezone, write_uid, create_uid, write_date, create_date) + VALUES ( + %(partner_id)s, %(access_token)s, now() at time zone 'UTC', 1, %(lang_id)s, + %(country_id)s, %(website_id)s, %(timezone)s, %(create_uid)s, %(write_uid)s, + now() at time zone 'UTC', now() at time zone 'UTC') + ON CONFLICT (access_token) + DO UPDATE SET + last_connection_datetime=excluded.last_connection_datetime, + visit_count = CASE WHEN website_visitor.last_connection_datetime < NOW() AT TIME ZONE 'UTC' - INTERVAL '8 hours' + THEN website_visitor.visit_count + 1 + ELSE website_visitor.visit_count + END + RETURNING id, CASE WHEN create_date = now() at time zone 'UTC' THEN 'inserted' ELSE 'updated' END AS upsert + """ + + if force_track_values: + create_values['url'] = force_track_values['url'] + create_values['page_id'] = force_track_values.get('page_id') + query = sql.SQL(""" + WITH visitor AS ( + {query}, %(url)s AS url, %(page_id)s AS page_id + ), track AS ( + INSERT INTO website_track (visitor_id, url, page_id, visit_datetime) + SELECT id, url, page_id::integer, now() at time zone 'UTC' FROM visitor + ) + SELECT id, upsert from visitor; + """).format(query=sql.SQL(query)) + + self.env.cr.execute(query, create_values) + return self.env.cr.fetchone() + + def _get_visitor_from_request(self, force_create=False, force_track_values=None): + """ Return the visitor as sudo from the request. + + :param bool force_create: force a visitor creation if no visitor exists + :param force_track_values: an optional dict to create a track at the + same time. + :return: the website visitor if exists or forced, empty recordset + otherwise. + """ # This function can be called in json with mobile app. # In case of mobile app, no uid is set on the jsonRequest env. # In case of multi db, _env is None on request, and request.env unbound. if not (request and request.env and request.env.uid): return None + Visitor = self.env['website.visitor'].sudo() visitor = Visitor - access_token = request.httprequest.cookies.get('visitor_uuid') - if access_token: - visitor = Visitor.with_context(active_test=False).search([('access_token', '=', access_token)]) - # Prefetch access_token and other fields. Since access_token has a restricted group and we access - # a non restricted field (partner_id) first it is not fetched and will require an additional query to be retrieved. - visitor.access_token + access_token = self._get_access_token() - if not self.env.user._is_public(): - partner_id = self.env.user.partner_id - if not visitor or visitor.partner_id and visitor.partner_id != partner_id: - # Partner and no cookie or wrong cookie - visitor = Visitor.with_context(active_test=False).search([('partner_id', '=', partner_id.id)]) - elif visitor and visitor.partner_id: - # Cookie associated to a Partner - visitor = Visitor + if force_create: + visitor_id, _ = self._upsert_visitor(access_token, force_track_values) + visitor = Visitor.browse(visitor_id) + else: + visitor = Visitor.search([('access_token', '=', access_token)]) - # 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: + if not force_create and 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() - return visitor - def _handle_webpage_dispatch(self, response, website_page): - # get visitor. Done here to avoid having to do it multiple times in case of override. - visitor_sudo = self._get_visitor_from_request(force_create=True) - if request.httprequest.cookies.get('visitor_uuid', '') != visitor_sudo.access_token: - expiration_date = datetime.now() + timedelta(days=365) - response.set_cookie('visitor_uuid', visitor_sudo.access_token, expires=expiration_date) - self._handle_website_page_visit(website_page, visitor_sudo) + def _handle_webpage_dispatch(self, website_page): + """ Create a website.visitor if the http request object is a tracked + website.page or a tracked ir.ui.view. + Since this method is only called on tracked elements, the + last_connection_datetime might not be accurate as the visitor could have + been visiting only untracked page during his last visit.""" - def _handle_website_page_visit(self, website_page, visitor_sudo): - """ Called on dispatch. This will create a website.visitor if the http request object - is a tracked website page or a tracked view. Only on tracked elements to avoid having - too much operations done on every page or other http requests. - Note: The side effect is that the last_connection_datetime is updated ONLY on tracked elements.""" url = request.httprequest.url - website_track_values = { - 'url': url, - 'visit_datetime': datetime.now(), - } + website_track_values = {'url': url} if website_page: website_track_values['page_id'] = website_page.id - domain = [('page_id', '=', website_page.id)] - else: - domain = [('url', '=', url)] - visitor_sudo._add_tracking(domain, website_track_values) - if visitor_sudo.lang_id.id != request.lang.id: - visitor_sudo.write({'lang_id': request.lang.id}) + self._get_visitor_from_request(force_create=True, force_track_values=website_track_values) def _add_tracking(self, domain, website_track_values): """ Add the track and update the visitor""" @@ -258,76 +311,44 @@ class WebsiteVisitor(models.Model): self.env['website.track'].create(website_track_values) self._update_visitor_last_visit() - def _create_visitor(self): - """ Create a visitor. Tracking is added after the visitor has been created.""" - country_code = request.geoip.get('country_code') - country_id = request.env['res.country'].sudo().search([('code', '=', country_code)], limit=1).id if country_code else False - vals = { - 'lang_id': request.lang.id, - 'country_id': country_id, - 'website_id': request.website.id, - } + def _merge_visitor(self, target): + """ Merge an anonymous visitor data to a partner visitor then unlink + that anonymous visitor. + Purpose is to try to aggregate as much sub-records (tracked pages, + leads, ...) as possible. + It is especially useful to aggregate data from the same user on + different devices. - tz = self._get_visitor_timezone() - if tz: - vals['timezone'] = tz + This method is meant to be overridden for other modules to merge their + own anonymous visitor data to the partner visitor before unlink. - if not self.env.user._is_public(): - vals['partner_id'] = self.env.user.partner_id.id - 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): - """ 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. + This method is only called after the user logs in. :param target: main visitor, target of link process; """ - # 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}) - - # Archive current record and set its parent visitor - self.partner_id = False - self.parent_id = target.id - self.active = False - - return target + if not target.partner_id: + raise ValueError("The `target` visitor should be linked to a partner.") + self.website_track_ids.visitor_id = target.id + self.unlink() def _cron_unlink_old_visitors(self): - """ Unlink inactive visitors (see '_inactive_visitors_domain' for details). + """ 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. """ + 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() + self.env['website.visitor'].sudo().search(self._inactive_visitors_domain()).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 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. """ + 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) @@ -346,24 +367,18 @@ class WebsiteVisitor(models.Model): self.env.cr.execute(query, (timezone, self.id)) def _update_visitor_last_visit(self): - """ We need to do this part here to avoid concurrent updates error. """ - try: - with self.env.cr.savepoint(): - query_lock = "SELECT * FROM website_visitor where id = %s FOR NO KEY UPDATE NOWAIT" - self.env.cr.execute(query_lock, (self.id,), log_exceptions=False) - - date_now = datetime.now() - query = "UPDATE website_visitor SET " - if self.last_connection_datetime < (date_now - timedelta(hours=8)): - query += "visit_count = visit_count + 1," - query += """ - active = True, - last_connection_datetime = %s - WHERE id = %s - """ - self.env.cr.execute(query, (date_now, self.id), log_exceptions=False) - except Exception: - pass + date_now = datetime.now() + query = "UPDATE website_visitor SET " + if self.last_connection_datetime < (date_now - timedelta(hours=8)): + query += "visit_count = visit_count + 1," + query += """ + last_connection_datetime = %s + WHERE id IN ( + SELECT id FROM website_visitor WHERE id = %s + FOR NO KEY UPDATE SKIP LOCKED + ) + """ + self.env.cr.execute(query, (date_now, self.id), log_exceptions=False) def _get_visitor_timezone(self): tz = request.httprequest.cookies.get('tz') if request else None diff --git a/addons/website/tests/test_performance.py b/addons/website/tests/test_performance.py index f960ea66da1..a44bc309b23 100644 --- a/addons/website/tests/test_performance.py +++ b/addons/website/tests/test_performance.py @@ -104,16 +104,16 @@ class TestWebsitePerformance(UtilPerf): def test_15_perf_sql_queries_page(self): # standard tracked website.page self.page.track = True - self.assertEqual(self._get_url_hot_query(self.page.url), 14) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 18) + self.assertEqual(self._get_url_hot_query(self.page.url), 7) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 11) self.menu.unlink() - self.assertEqual(self._get_url_hot_query(self.page.url), 14) - self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 18) + self.assertEqual(self._get_url_hot_query(self.page.url), 7) + self.assertEqual(self._get_url_hot_query(self.page.url, cache=False), 11) def test_20_perf_sql_queries_homepage(self): # homepage "/" has its own controller - self.assertEqual(self._get_url_hot_query('/'), 14) - self.assertEqual(self._get_url_hot_query('/', cache=False), 16) + self.assertEqual(self._get_url_hot_query('/'), 7) + self.assertEqual(self._get_url_hot_query('/', cache=False), 9) def test_30_perf_sql_queries_page_no_layout(self): # website.page with no call to layout templates diff --git a/addons/website/tests/test_website_visitor.py b/addons/website/tests/test_website_visitor.py index d164b66a43c..e909069e8ef 100644 --- a/addons/website/tests/test_website_visitor.py +++ b/addons/website/tests/test_website_visitor.py @@ -1,12 +1,13 @@ # coding: utf-8 # Part of Odoo. See LICENSE file for full copyright and licensing details. +import random + from contextlib import contextmanager from datetime import datetime, timedelta from unittest.mock import patch 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 @@ -34,7 +35,6 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): self.website = self.env['website'].search([ ('company_id', '=', self.env.user.company_id.id) ], limit=1) - self.cookies = {} untracked_view = self.env['ir.ui.view'].create({ 'name': 'UntackedView', @@ -122,17 +122,28 @@ class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo): def assertVisitorDeactivated(self, visitor, main_visitor): """ 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() ). """ - self.assertTrue(bool(visitor)) - self.assertFalse(visitor.active) - self.assertTrue(main_visitor.active) - self.assertEqual(visitor.parent_id, main_visitor) - - # tracks are linked + well as Visitor._merge_visitor() ). """ + self.assertFalse(visitor.exists(), "The anonymous visitor should be deleted") 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. """ + + def authenticate(login, pwd): + # We can't call `self.authenticate` because that tour util is + # regenerating a new session.id before calling the real + # `authenticate` method. + # But we need the session id in the `authenticate` method because + # we need to retrieve the visitor before the authentication, which + # require the session id. + res = self.url_open('/web/login') + csrf_anchor = ' - @@ -271,8 +270,6 @@ - - diff --git a/addons/website_blog/tests/test_performance.py b/addons/website_blog/tests/test_performance.py index cf0d0fd7ae5..164656e44c2 100644 --- a/addons/website_blog/tests/test_performance.py +++ b/addons/website_blog/tests/test_performance.py @@ -28,8 +28,8 @@ class TestBlogPerformance(UtilPerf): blog_tags = blog_tags[:-1] self.assertEqual(self._get_url_hot_query('/blog'), 11) self.assertEqual(self._get_url_hot_query('/blog', cache=False), 34) - self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 26) - self.assertEqual(self._get_url_hot_query(blog_post[0].website_url, cache=False), 29) + self.assertEqual(self._get_url_hot_query(blog_post[0].website_url), 19) + self.assertEqual(self._get_url_hot_query(blog_post[0].website_url, cache=False), 22) def test_30_perf_sql_blog_bigger_data_scaling(self): BlogPost = self.env['blog.post'] diff --git a/addons/website_crm/models/crm_lead.py b/addons/website_crm/models/crm_lead.py index 5df42da3765..34e1fdbbc07 100644 --- a/addons/website_crm/models/crm_lead.py +++ b/addons/website_crm/models/crm_lead.py @@ -21,7 +21,7 @@ class Lead(models.Model): JOIN crm_lead_website_visitor_rel lv ON l.id = lv.crm_lead_id JOIN website_visitor v ON v.id = lv.website_visitor_id JOIN website_track p ON p.visitor_id = v.id - WHERE l.id in %s AND v.active = TRUE + WHERE l.id in %s GROUP BY l.id""" self.env.cr.execute(sql, (tuple(self.ids),)) page_data = self.env.cr.dictfetchall() diff --git a/addons/website_crm/models/website_visitor.py b/addons/website_crm/models/website_visitor.py index 717d5e0effd..c990a7ab1ff 100644 --- a/addons/website_crm/models/website_visitor.py +++ b/addons/website_crm/models/website_visitor.py @@ -48,14 +48,14 @@ class WebsiteVisitor(models.Model): domain = super()._inactive_visitors_domain() return expression.AND([domain, [('lead_ids', '=', False)]]) - def _link_to_visitor(self, target): + def _merge_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) + return super()._merge_visitor(target) def _prepare_message_composer_context(self): if not self.partner_id and self.lead_ids: diff --git a/addons/website_crm/tests/test_crm_lead_merge.py b/addons/website_crm/tests/test_crm_lead_merge.py index a066266aeb2..efcb45b492f 100644 --- a/addons/website_crm/tests/test_crm_lead_merge.py +++ b/addons/website_crm/tests/test_crm_lead_merge.py @@ -22,11 +22,13 @@ class TestLeadVisitorMerge(TestLeadMergeCommon): TestLeadMergeCommon.merge_fields.append('visitor_ids') visitors = self.env['website.visitor'].sudo().create([ - {'name': 'Visitor 1', - 'lead_ids': [(4, self.lead_w_partner_company.id)], + { + 'access_token': 'f9d2ffa0427d4e4b1d740cf5eb3cdc20', + 'lead_ids': [(4, self.lead_w_partner_company.id)], }, - {'name': 'Visitor 2', - 'lead_ids': [(4, self.lead_w_partner.id)], + { + 'access_token': 'f9d2c3f741a79200487728eac989e678', + 'lead_ids': [(4, self.lead_w_partner.id)], } ]) self.assertEqual(self.lead_w_partner_company.visitor_ids, visitors[0]) diff --git a/addons/website_crm/tests/test_website_visitor.py b/addons/website_crm/tests/test_website_visitor.py index 143886738b8..c537cc63155 100644 --- a/addons/website_crm/tests/test_website_visitor.py +++ b/addons/website_crm/tests/test_website_visitor.py @@ -24,7 +24,7 @@ class TestWebsiteVisitor(TestCrmCommon, WebsiteVisitorTests): @users('user_sales_manager') def test_compute_email_phone(self): visitor_sudo = self.env['website.visitor'].sudo().create({ - 'name': 'Mega Visitor', + 'access_token': 'f9d2c6f3b024320ac31248595ac7fcb6', }) visitor = visitor_sudo.with_user(self.env.user) # as of 13.0 salesmen cannot create visitors, only read them customer = self.test_partner.with_user(self.env.user) @@ -75,11 +75,11 @@ class TestWebsiteVisitor(TestCrmCommon, WebsiteVisitorTests): 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), + 'access_token': 'f9d28aad05ebee0bca215837b129aa00', 'lead_ids': [(0, 0, { 'name': 'Lead Carl' })] @@ -96,7 +96,7 @@ class TestWebsiteVisitor(TestCrmCommon, WebsiteVisitorTests): self._prepare_linked_visitor_data() ]) all_leads = (main_visitor + linked_visitor).lead_ids - linked_visitor._link_to_visitor(main_visitor) + linked_visitor._merge_visitor(main_visitor) self.assertVisitorDeactivated(linked_visitor, main_visitor) diff --git a/addons/website_event/controllers/main.py b/addons/website_event/controllers/main.py index a987993794d..2eec2e75624 100644 --- a/addons/website_event/controllers/main.py +++ b/addons/website_event/controllers/main.py @@ -287,8 +287,6 @@ class WebsiteEventController(http.Controller): as much informations as possible, notably to ease future communications. Also try to update visitor informations based on registration info. """ visitor_sudo = request.env['website.visitor']._get_visitor_from_request(force_create=True) - visitor_sudo._update_visitor_last_visit() - visitor_values = {} registrations_to_create = [] for registration_values in registration_data: @@ -298,18 +296,11 @@ class WebsiteEventController(http.Controller): elif not registration_values.get('partner_id'): registration_values['partner_id'] = False if request.env.user._is_public() else request.env.user.partner_id.id - if visitor_sudo: - # registration may give a name to the visitor, yay - if registration_values.get('name') and not visitor_sudo.name and not visitor_values.get('name'): - visitor_values['name'] = registration_values['name'] - # update registration based on visitor - registration_values['visitor_id'] = visitor_sudo.id + # update registration based on visitor + registration_values['visitor_id'] = visitor_sudo.id registrations_to_create.append(registration_values) - if visitor_values: - visitor_sudo.write(visitor_values) - return request.env['event.registration'].sudo().create(registrations_to_create) @http.route(['''/event//registration/confirm'''], type='http', auth="public", methods=['POST'], website=True) diff --git a/addons/website_event/models/website_visitor.py b/addons/website_event/models/website_visitor.py index 24a1b8f3a1a..c4dd71a0bfe 100644 --- a/addons/website_event/models/website_visitor.py +++ b/addons/website_event/models/website_visitor.py @@ -21,6 +21,16 @@ class WebsiteVisitor(models.Model): search="_search_event_registered_ids", groups="event.group_event_registration_desk") + @api.depends('partner_id, event_registration_ids.name') + def name_get(self): + """ If there is an event registration for an anonymous visitor, use that + registered attendee name as visitor name. """ + res_dict = dict(super().name_get()) + # sudo is needed for `event_registration_ids` + for visitor in self.sudo().filtered(lambda v: not v.partner_id and v.event_registration_ids): + res_dict[visitor.id] = visitor.event_registration_ids[-1].name + return list(res_dict.items()) + @api.depends('event_registration_ids') def _compute_event_registration_count(self): if self.ids: @@ -46,12 +56,12 @@ class WebsiteVisitor(models.Model): if not visitor.mobile: visitor.mobile = next((reg.mobile or reg.phone for reg in linked_registrations if reg.mobile or reg.phone), False) - @api.depends('parent_id', 'event_registration_ids') + @api.depends('event_registration_ids') def _compute_event_registered_ids(self): # include parent's registrations in a visitor o2m field. We don't add # child one as child should not have registrations (moved to the parent) for visitor in self: - all_registrations = visitor.event_registration_ids | visitor.parent_id.event_registration_ids + all_registrations = visitor.event_registration_ids visitor.event_registered_ids = all_registrations.mapped('event_id') def _search_event_registered_ids(self, operator, operand): @@ -65,12 +75,7 @@ class WebsiteVisitor(models.Model): ('event_id', operator, operand) ]) if all_registrations: - # search children, even archived one, to contact them - visitors = all_registrations.with_context(active_test=False).mapped('visitor_id') - children = self.env['website.visitor'].with_context( - active_test=False - ).sudo().search([('parent_id', 'in', visitors.ids)]) - visitor_ids = (visitors + children).ids + visitor_ids = all_registrations.with_context(active_test=False).visitor_id.ids else: visitor_ids = [] @@ -81,16 +86,10 @@ class WebsiteVisitor(models.Model): 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: - registration_wo_partner = self.event_registration_ids.filtered(lambda registration: not registration.partner_id) - if registration_wo_partner: - registration_wo_partner.partner_id = partner - super(WebsiteVisitor, self)._link_to_partner(partner, update_values=update_values) - - def _link_to_visitor(self, target): + def _merge_visitor(self, target): """ Override linking process to link registrations to the final visitor. """ - self.event_registration_ids.write({'visitor_id': target.id}) - - return super(WebsiteVisitor, self)._link_to_visitor(target) + self.event_registration_ids.visitor_id = target.id + registration_wo_partner = self.event_registration_ids.filtered(lambda registration: not registration.partner_id) + if registration_wo_partner: + registration_wo_partner.partner_id = target.partner_id + return super()._merge_visitor(target) diff --git a/addons/website_event/tests/test_event_visitor.py b/addons/website_event/tests/test_event_visitor.py index 07aebd51888..8f1fdfea89d 100644 --- a/addons/website_event/tests/test_event_visitor.py +++ b/addons/website_event/tests/test_event_visitor.py @@ -15,11 +15,11 @@ 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), + 'access_token': 'f9d2af99f543874642f89bd334fa4a49', 'event_registration_ids': [(0, 0, { 'event_id': self.event_0.id })] @@ -50,7 +50,7 @@ class TestEventVisitor(TestEventOnlineCommon, WebsiteVisitorTests): self.assertEqual(self.event_0, main_visitor.event_registered_ids) self.assertEqual(event_1, linked_visitor.event_registered_ids) - linked_visitor._link_to_visitor(main_visitor) + linked_visitor._merge_visitor(main_visitor) self.assertVisitorDeactivated(linked_visitor, main_visitor) # main_visitor is now attending both events diff --git a/addons/website_event/views/website_visitor_views.xml b/addons/website_event/views/website_visitor_views.xml index bd516f0fa87..da6dc0221fb 100644 --- a/addons/website_event/views/website_visitor_views.xml +++ b/addons/website_event/views/website_visitor_views.xml @@ -1,17 +1,6 @@ - - website.visitor.view.search.inherit.event - website.visitor - - - - - - - - website.visitor.view.tree.inherit.event website.visitor @@ -36,9 +25,6 @@ - - - diff --git a/addons/website_event_meet_quiz/views/event_meet_templates.xml b/addons/website_event_meet_quiz/views/event_meet_templates.xml index 68f2c78b259..8a3821dbd6e 100644 --- a/addons/website_event_meet_quiz/views/event_meet_templates.xml +++ b/addons/website_event_meet_quiz/views/event_meet_templates.xml @@ -25,7 +25,7 @@ t-att-src="image_data_uri(visitor['visitor'].partner_image) if visitor['visitor'].partner_image else '/web/static/img/user_placeholder.jpg'" t-att-alt="visitor['visitor'].display_name"/>
- You + You
diff --git a/addons/website_event_track/controllers/event_track.py b/addons/website_event_track/controllers/event_track.py index eadcf5b13a5..eab787289a5 100644 --- a/addons/website_event_track/controllers/event_track.py +++ b/addons/website_event_track/controllers/event_track.py @@ -384,7 +384,6 @@ class EventTrackController(http.Controller): track = self._fetch_track(track_id, allow_sudo=True) force_create = set_reminder_on or track.wishlisted_by_default event_track_partner = track._get_event_track_visitors(force_create=force_create) - visitor_sudo = event_track_partner.visitor_id if not track.wishlisted_by_default: if not event_track_partner or event_track_partner.is_wishlisted == set_reminder_on: # ignore if new state = old state @@ -396,8 +395,6 @@ class EventTrackController(http.Controller): event_track_partner.is_blacklisted = not set_reminder_on result = {'reminderOn': set_reminder_on} - if request.httprequest.cookies.get('visitor_uuid', '') != visitor_sudo.access_token: - result['visitor_uuid'] = visitor_sudo.access_token return result diff --git a/addons/website_event_track/models/website_visitor.py b/addons/website_event_track/models/website_visitor.py index 65c80077553..3ecfbcb6439 100644 --- a/addons/website_event_track/models/website_visitor.py +++ b/addons/website_event_track/models/website_visitor.py @@ -53,15 +53,10 @@ class WebsiteVisitor(models.Model): 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 """ - if partner: - track_visitor_wo_partner = self.event_track_visitor_ids.filtered(lambda track_visitor: not track_visitor.partner_id) - if track_visitor_wo_partner: - track_visitor_wo_partner.partner_id = partner - super(WebsiteVisitor, self)._link_to_partner(partner, update_values=update_values) - - def _link_to_visitor(self, target): + def _merge_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) + track_visitor_wo_partner = self.event_track_visitor_ids.filtered(lambda track_visitor: not track_visitor.partner_id) + if track_visitor_wo_partner: + track_visitor_wo_partner.partner_id = target.partner_id + return super()._merge_visitor(target) diff --git a/addons/website_event_track/static/src/js/event_track_reminder.js b/addons/website_event_track/static/src/js/event_track_reminder.js index 1e3c97a90b1..840bbf0d4fa 100644 --- a/addons/website_event_track/static/src/js/event_track_reminder.js +++ b/addons/website_event_track/static/src/js/event_track_reminder.js @@ -3,7 +3,6 @@ odoo.define('website_event_track.website_event_track_reminder', function (requir var core = require('web.core'); var _t = core._t; -var utils = require('web.utils'); var publicWidget = require('web.public.widget'); publicWidget.registry.websiteEventTrackReminder = publicWidget.Widget.extend({ @@ -71,9 +70,6 @@ publicWidget.registry.websiteEventTrackReminder = publicWidget.Widget.extend({ }); } } - if (result.visitor_uuid) { - utils.set_cookie('visitor_uuid', result.visitor_uuid); - } }); }, diff --git a/addons/website_event_track/tests/test_track_internals.py b/addons/website_event_track/tests/test_track_internals.py index e2af8a0af41..50a7115f737 100644 --- a/addons/website_event_track/tests/test_track_internals.py +++ b/addons/website_event_track/tests/test_track_internals.py @@ -145,8 +145,8 @@ class TestTrackSuggestions(TestEventOnlineCommon): }]) emp_visitor = self.env['website.visitor'].create({ - 'name': 'Visitor', - 'partner_id': self.user_employee.partner_id.id + 'partner_id': self.user_employee.partner_id.id, + 'access_token': self.user_employee.partner_id.id, }) visitor_track = self.env['event.track.visitor'].create({ 'visitor_id': emp_visitor.id, diff --git a/addons/website_event_track/tests/test_website_visitor.py b/addons/website_event_track/tests/test_website_visitor.py index 1d54769cf2c..cfe3e0ddc75 100644 --- a/addons/website_event_track/tests/test_website_visitor.py +++ b/addons/website_event_track/tests/test_website_visitor.py @@ -25,6 +25,7 @@ class WebsiteVisitorTestsEventTrack(TestEventOnlineCommon, WebsiteVisitorTests): 'country_id': self.env.ref('base.be').id, 'website_id': 1, 'last_connection_datetime': datetime.now() - timedelta(days=8), + 'access_token': 'f9d2b93591d6f602e5e8afa238e35a6c', 'event_track_visitor_ids': [(0, 0, { 'track_id': track_1.id, 'is_wishlisted': True @@ -60,7 +61,7 @@ class WebsiteVisitorTestsEventTrack(TestEventOnlineCommon, WebsiteVisitorTests): 'is_wishlisted': True, }]) - linked_visitor._link_to_visitor(main_visitor) + linked_visitor._merge_visitor(main_visitor) self.assertVisitorDeactivated(linked_visitor, main_visitor) diff --git a/addons/website_event_track_quiz/controllers/event_track_quiz.py b/addons/website_event_track_quiz/controllers/event_track_quiz.py index b990805d32e..748ebc80981 100644 --- a/addons/website_event_track_quiz/controllers/event_track_quiz.py +++ b/addons/website_event_track_quiz/controllers/event_track_quiz.py @@ -19,7 +19,6 @@ class WebsiteEventTrackQuiz(EventTrackController): track_sudo = track.sudo() event_track_visitor = track._get_event_track_visitors(force_create=True) - visitor_sudo = event_track_visitor.visitor_id if event_track_visitor.quiz_completed: return {'error': 'track_quiz_done'} @@ -45,8 +44,6 @@ class WebsiteEventTrackQuiz(EventTrackController): 'quiz_completed': event_track_visitor.quiz_completed, 'quiz_points': answers_details['points'] } - if visitor_sudo and request.httprequest.cookies.get('visitor_uuid', '') != visitor_sudo.access_token: - result['visitor_uuid'] = visitor_sudo.access_token return result @http.route('/event_track/quiz/reset', type="json", auth="public", website=True) diff --git a/addons/website_event_track_quiz/static/src/js/event_quiz.js b/addons/website_event_track_quiz/static/src/js/event_quiz.js index b9fbc3d003e..47331f11732 100644 --- a/addons/website_event_track_quiz/static/src/js/event_quiz.js +++ b/addons/website_event_track_quiz/static/src/js/event_quiz.js @@ -5,7 +5,6 @@ odoo.define('website_event_track_quiz.event.quiz', function (require) { var publicWidget = require('web.public.widget'); var core = require('web.core'); var session = require('web.session'); -var utils = require('web.utils'); var QWeb = core.qweb; var _t = core._t; @@ -235,9 +234,6 @@ var Quiz = publicWidget.Widget.extend({ } self._renderAnswersHighlightingAndComments(); self._renderValidationInfo(); - if (data.visitor_uuid) { - utils.set_cookie('visitor_uuid', data.visitor_uuid); - } } return Promise.resolve(data); diff --git a/addons/website_event_track_quiz/views/event_leaderboard_templates.xml b/addons/website_event_track_quiz/views/event_leaderboard_templates.xml index a438718f6da..6fa4aaba1e3 100644 --- a/addons/website_event_track_quiz/views/event_leaderboard_templates.xml +++ b/addons/website_event_track_quiz/views/event_leaderboard_templates.xml @@ -61,7 +61,7 @@ t-att-src="image_data_uri(visitor['visitor'].partner_image) if visitor['visitor'].partner_image else '/web/static/img/user_placeholder.jpg'"/> User rank -

You

+

You