From 043b3385d239b92738b3f08b2a2709b0ce524def Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Warnon?= Date: Tue, 5 Oct 2021 14:15:45 +0000 Subject: [PATCH] [IMP] website_slides: improve slide.embed feature Currently, the slide.embed statistics only work if a 'Referer' header is passed in the request object. The issue with that strategy is that it's not very reliable, indeed some modern browsers now completely avoid passing the referer header (mostly for security and privacy reasons). This commit refactors the slide.embed feature to work even without a referer header. When the header is missing, the incremented count will be stored under a common slide.embed record with a url field = False. This count is still useful because the end-user will want to know if the slide is popular externally, even if we don't know on which website it has been embedded exactly. In addition, we add more visibility to the slide.embed feature by introducing a stat-button on the slide.slide form view, that redirects to the list of slide.embeds for this slide. Finally, we also renamed the 'embedcount_ids' field to 'embed_ids' to get a better / clearer naming. A few tests about embed detection were added to ensure this behavior. (Tests are based on the work of Nathan Marotte ). Task-2654009 Part-of: odoo/odoo#77736 --- addons/website_slides/__manifest__.py | 1 + addons/website_slides/controllers/main.py | 10 ++-- addons/website_slides/models/slide_embed.py | 26 +++------ addons/website_slides/models/slide_slide.py | 54 ++++++++++++++++++- addons/website_slides/tests/__init__.py | 1 + .../tests/test_embed_detection.py | 51 ++++++++++++++++++ .../views/slide_embed_views.xml | 33 ++++++++++++ .../views/slide_slide_views.xml | 7 +++ 8 files changed, 159 insertions(+), 24 deletions(-) create mode 100644 addons/website_slides/tests/test_embed_detection.py create mode 100644 addons/website_slides/views/slide_embed_views.xml diff --git a/addons/website_slides/__manifest__.py b/addons/website_slides/__manifest__.py index a201a750463..97677eab787 100644 --- a/addons/website_slides/__manifest__.py +++ b/addons/website_slides/__manifest__.py @@ -31,6 +31,7 @@ Featuring 'views/res_config_settings_views.xml', 'views/res_partner_views.xml', 'views/rating_rating_views.xml', + 'views/slide_embed_views.xml', 'views/slide_question_views.xml', 'views/slide_slide_views.xml', 'views/slide_channel_partner_views.xml', diff --git a/addons/website_slides/controllers/main.py b/addons/website_slides/controllers/main.py index e4141799000..a75dae3f142 100644 --- a/addons/website_slides/controllers/main.py +++ b/addons/website_slides/controllers/main.py @@ -1153,12 +1153,12 @@ class WebsiteSlides(WebsiteProfile): try: slide = request.env['slide.slide'].browse(slide_id) + if not slide.exists(): + raise werkzeug.exceptions.NotFound() + referer_url = request.httprequest.headers.get('Referer', '') - if is_external_embed and referer_url: - request.env['slide.embed'].sudo()._add_embed_url( - slide.id, - referer_url - ) + if is_external_embed: + slide.sudo()._embed_increment(referer_url) values = self._get_slide_detail(slide) values['page'] = page diff --git a/addons/website_slides/models/slide_embed.py b/addons/website_slides/models/slide_embed.py index 7cbe5fbc56d..c1b028bdbcc 100644 --- a/addons/website_slides/models/slide_embed.py +++ b/addons/website_slides/models/slide_embed.py @@ -1,31 +1,21 @@ # -*- coding: utf-8 -*- # Part of Odoo. See LICENSE file for full copyright and licensing details. -from werkzeug import urls - -from odoo import fields, models +from odoo import _, api, fields, models class EmbeddedSlide(models.Model): """ Embedding in third party websites. Track view count, generate statistics. """ _name = 'slide.embed' _description = 'Embedded Slides View Counter' - _rec_name = 'slide_id' + _rec_name = 'website_name' slide_id = fields.Many2one('slide.slide', string="Presentation", required=True, index=True) - url = fields.Char('Third Party Website URL', required=True) + url = fields.Char('Third Party Website URL') + website_name = fields.Char('Website', compute='_compute_website_name') count_views = fields.Integer('# Views', default=1) - def _add_embed_url(self, slide_id, url): - baseurl = urls.url_parse(url).netloc - if not baseurl: - return 0 - embeds = self.search([('url', '=', baseurl), ('slide_id', '=', int(slide_id))], limit=1) - if embeds: - embeds.count_views += 1 - else: - embeds = self.create({ - 'slide_id': slide_id, - 'url': baseurl, - }) - return embeds.count_views + @api.depends('url') + def _compute_website_name(self): + for slide_embed in self: + slide_embed.website_name = slide_embed.url or _('Unknown Website') diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py index dd4e697aefd..8a70d94a409 100644 --- a/addons/website_slides/models/slide_slide.py +++ b/addons/website_slides/models/slide_slide.py @@ -145,7 +145,8 @@ class Slide(models.Model): embed_code_external = fields.Html('External Embed Code', readonly=True, compute='_compute_embed_code', sanitize=False, help="Same as 'Embed Code' but used to embed the content on an external website.") # views - embedcount_ids = fields.One2many('slide.embed', 'slide_id', string="Embed Count") + embed_ids = fields.One2many('slide.embed', 'slide_id', string="External Slide Embeds") + embed_count = fields.Integer('# of Embeds', compute='_compute_embed_counts') slide_views = fields.Integer('# of Website Views', store=True, compute="_compute_slide_views") public_views = fields.Integer('# of Public Views', copy=False) total_views = fields.Integer("# Total Views", default="0", compute='_compute_total', store=True) @@ -251,6 +252,24 @@ class Slide(models.Model): for slide in self: slide.slide_views = mapped_data.get(slide.id, 0) + @api.depends('embed_ids.slide_id') + def _compute_embed_counts(self): + mapped_data = {} + + if self.ids: + read_group_res = self.env['slide.embed'].read_group( + [('slide_id', 'in', self.ids)], + ['count_views'], + ['slide_id'] + ) + mapped_data = { + res['slide_id'][0]: res.get('count_views', 0) + for res in read_group_res + } + + for slide in self: + slide.embed_count = mapped_data.get(slide.id, 0) + @api.depends('slide_ids.sequence', 'slide_ids.slide_type', 'slide_ids.is_published', 'slide_ids.is_category') def _compute_slides_statistics(self): # Do not use dict.fromkeys(self.ids, dict()) otherwise it will use the same dictionnary for all keys. @@ -492,6 +511,32 @@ class Slide(models.Model): # Business Methods # --------------------------------------------------------- + def _embed_increment(self, url): + """ Increment the view count of the record we have based on the passed url. + If the url is empty, which typically happens if the browser does not pass the 'referer' + header properly, then we increment the entry that has 'False' as url value. """ + + self.ensure_one() + + url_entry = url + if not urls.url_parse(url).netloc: + url_entry = False + + embed_entry = self.env['slide.embed'].search([ + ('url', '=', url_entry), + ('slide_id', '=', self.id) + ], limit=1) + + if embed_entry: + embed_entry.count_views += 1 + else: + embed_entry = self.env['slide.embed'].create({ + 'slide_id': self.id, + 'url': url_entry, + }) + + return embed_entry + def _post_publication(self): for slide in self.filtered(lambda slide: slide.website_published and slide.channel_id.publish_template_id): publish_template = slide.channel_id.publish_template_id @@ -656,6 +701,13 @@ class Slide(models.Model): return self.env.user.sudo().add_karma(points) + def action_view_embeds(self): + self.ensure_one() + + action = self.env["ir.actions.actions"]._for_xml_id("website_slides.slide_embed_action") + action['context'] = {'search_default_slide_id': self.id} + return action + def _compute_quiz_info(self, target_partner, quiz_done=False): result = dict.fromkeys(self.ids, False) slide_partners = self.env['slide.slide.partner'].sudo().search([ diff --git a/addons/website_slides/tests/__init__.py b/addons/website_slides/tests/__init__.py index 9b7fc83ac65..3810dd0fad5 100644 --- a/addons/website_slides/tests/__init__.py +++ b/addons/website_slides/tests/__init__.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from . import common +from . import test_embed_detection from . import test_karma from . import test_resources from . import test_security diff --git a/addons/website_slides/tests/test_embed_detection.py b/addons/website_slides/tests/test_embed_detection.py new file mode 100644 index 00000000000..5c61ffaa00d --- /dev/null +++ b/addons/website_slides/tests/test_embed_detection.py @@ -0,0 +1,51 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from odoo.addons.website_slides.tests import common +from odoo.tests import HttpCase + + +class TestEmbedDetection(HttpCase, common.SlidesCase): + @classmethod + def setUpClass(cls): + super(TestEmbedDetection, cls).setUpClass() + + cls.website = cls.env['website'].create({ + 'name': 'Test Website', + 'domain': 'https://testwebsite.com' + }) + + cls.channel.website_id = cls.website.id + + def test_embed_external_no_referer(self): + """ When hitting the external URL without a referer header, the global embed record is + incremented. """ + self.url_open(f'/slides/embed_external/{self.slide.id}') + embed_views = self.env['slide.embed'].search([('slide_id', '=', self.slide.id)]) + self.assertEqual(len(embed_views), 1) + self.assertEqual(embed_views.website_name, 'Unknown Website') + + def test_embed_external_referer(self): + """ When hitting the external URL with a referer header, the embed record is incremented + based on the referer URL. """ + + self.assertFalse(bool(self.env['slide.embed'].search([ + ('slide_id', '=', self.slide.id) + ]))) + + self.url_open( + f'/slides/embed_external/{self.slide.id}', + headers={'Referer': 'https://someexternalwebsite.com'} + ) + + embed_views = self.env['slide.embed'].search([('slide_id', '=', self.slide.id)]) + self.assertEqual(len(embed_views), 1) + self.assertEqual(embed_views.count_views, 1) + self.assertEqual(embed_views.website_name, 'https://someexternalwebsite.com') + + def test_embed_not_external(self): + """ When hitting the non-external URL, we should not add a slide_embed record. """ + self.url_open(f'/slides/embed/{self.slide.id}') + self.assertFalse(bool(self.env['slide.embed'].search([ + ('slide_id', '=', self.slide.id) + ]))) diff --git a/addons/website_slides/views/slide_embed_views.xml b/addons/website_slides/views/slide_embed_views.xml new file mode 100644 index 00000000000..28190922c6f --- /dev/null +++ b/addons/website_slides/views/slide_embed_views.xml @@ -0,0 +1,33 @@ + + + + + slide.embed.view.tree + slide.embed + + + + + + + + + + + slide.embed.view.search + slide.embed + + + + + + + + + Embed Views + ir.actions.act_window + slide.embed + tree,search + + + diff --git a/addons/website_slides/views/slide_slide_views.xml b/addons/website_slides/views/slide_slide_views.xml index b35169518f5..04290773296 100644 --- a/addons/website_slides/views/slide_slide_views.xml +++ b/addons/website_slides/views/slide_slide_views.xml @@ -87,6 +87,13 @@ Comments +