[REF] link_tracker, mass_mailing: simplify add_click method and add tests

In this commit we rewrite a bit add_click in order to remove the inlined sudo
and ease inheritance and parameter management inside the method.

Access through controllers is sudo-ed as the main API method is now done
with current user access rights.

This commit is linked to task ID 1904277 and PR #28242.

7 files changed, 123 insertions, 61 deletions

diff --git a/addons/link_tracker/controller/main.py b/addons/link_tracker/controller/main.py
index 3b23c005446..d884967b5a6 100644
--- a/addons/link_tracker/controller/main.py
+++ b/addons/link_tracker/controller/main.py
@@ -8,9 +8,14 @@ from odoo.http import request
 
 
 class LinkTracker(http.Controller):
+
     @http.route('/r/<string:code>', type='http', auth='none', website=True)
     def full_url_redirect(self, code, **post):
         country_code = request.session.geoip and request.session.geoip.get('country_code') or False
-        request.env['link.tracker.click'].sudo().add_click(code, request.httprequest.remote_addr, country_code, stat_id=False)
+        request.env['link.tracker.click'].sudo().add_click(
+            code,
+            ip=request.httprequest.remote_addr,
+            country_code=country_code
+        )
         redirect_url = request.env['link.tracker'].get_url_from_code(code)
         return werkzeug.utils.redirect(redirect_url or '', 301)
diff --git a/addons/link_tracker/models/link_tracker.py b/addons/link_tracker/models/link_tracker.py
index c8f41c2eaca..7045e3ec3d2 100644
--- a/addons/link_tracker/models/link_tracker.py
+++ b/addons/link_tracker/models/link_tracker.py
@@ -240,32 +240,25 @@ class LinkTrackerClick(models.Model):
     ip = fields.Char(string='Internet Protocol')
     country_id = fields.Many2one('res.country', 'Country')
 
-    @api.model
-    def add_click(self, code, ip, country_code, stat_id=False):
-        self = self.sudo()
-        code_rec = self.env['link.tracker.code'].search([('code', '=', code)])
+    def _prepare_click_values_from_route(self, **route_values):
+        click_values = dict((fname, route_values[fname]) for fname in self._fields if fname in route_values)
+        if not click_values.get('country_id') and route_values.get('country_code'):
+            click_values['country_id'] = self.env['res.country'].search([('code', '=', route_values['country_code'])], limit=1).id
+        return click_values
 
-        if not code_rec:
+    @api.model
+    def add_click(self, code, **route_values):
+        """ Main API to add a click on a link. """
+        tracker_code = self.env['link.tracker.code'].search([('code', '=', code)])
+        if not tracker_code:
             return None
 
-        again = self.search_count([('link_id', '=', code_rec.link_id.id), ('ip', '=', ip)])
-
-        if not again:
-            self.create(
-                self._get_click_values_from_route(dict(
-                    code=code,
-                    ip=ip,
-                    country_code=country_code,
-                    stat_id=stat_id,
-                )))
+        ip = route_values.get('ip', False)
+        existing = self.search_count(['&', ('link_id', '=', tracker_code.link_id.id), ('ip', '=', ip)])
+        if existing:
+            return None
 
-    def _get_click_values_from_route(self, route_values):
-        code = self.env['link.tracker.code'].search([('code', '=', route_values['code'])], limit=1)
-        country = self.env['res.country'].search([('code', '=', route_values['country_code'])], limit=1)
+        route_values['link_id'] = tracker_code.link_id.id
+        click_values = self._prepare_click_values_from_route(**route_values)
 
-        return {
-            'link_id': code.link_id.id,
-            'create_date': datetime.date.today(),
-            'ip': route_values['ip'],
-            'country_id': country.id,
-        }
+        return self.create(click_values)
diff --git a/addons/mass_mailing/controllers/main.py b/addons/mass_mailing/controllers/main.py
index 440def20462..3c75c7934a3 100644
--- a/addons/mass_mailing/controllers/main.py
+++ b/addons/mass_mailing/controllers/main.py
@@ -107,7 +107,12 @@ class MassMailController(http.Controller):
         # which mass_mailing doesn't depend on
         country_code = request.session.get('geoip', False) and request.session.geoip.get('country_code', False)
 
-        request.env['link.tracker.click'].sudo().add_click(code, request.httprequest.remote_addr, country_code, stat_id=stat_id)
+        request.env['link.tracker.click'].sudo().add_click(
+            code,
+            ip=request.httprequest.remote_addr,
+            country_code=country_code,
+            mail_stat_id=stat_id
+        )
         return werkzeug.utils.redirect(request.env['link.tracker'].get_url_from_code(code), 301)
 
     @http.route('/mailing/blacklist/check', type='json', auth='none')
diff --git a/addons/mass_mailing/data/mass_mailing_demo.xml b/addons/mass_mailing/data/mass_mailing_demo.xml
index 215cb88bd7a..666d8050cac 100644
--- a/addons/mass_mailing/data/mass_mailing_demo.xml
+++ b/addons/mass_mailing/data/mass_mailing_demo.xml
@@ -210,49 +210,37 @@
 
         <!-- Generate some clicks -->
         <function model="link.tracker.click" name="add_click">
-            <!-- code, ip, country_code, stat_id -->
             <value model="link.tracker.code"
                 search="[('link_id.url', '=', 'http://www.example.com')]"
                 use="code"/>
-            <value eval="'100.01.02.03'"/>
-            <value eval="'BE'"/>
-            <value eval="ref('mass_mail_1_stat_0')"/>
+            <value eval="{'ip': '100.01.02.03', 'country_code': 'BE', 'mail_stat_id': ref('mass_mail_1_stat_0')}"/>
         </function>
         <function model="link.tracker.click" name="add_click">
-            <!-- code, ip, country_code, stat_id -->
             <value model="link.tracker.code"
                 search="[('link_id.url', '=', 'http://www.example.net/page/contactus')]"
                 use="code"/>
-            <value eval="'100.01.02.03'"/>
-            <value eval="'BE'"/>
-            <value eval="ref('mass_mail_1_stat_0')"/>
+            <value eval="{'ip': '100.01.02.03', 'country_code': 'BE', 'mail_stat_id': ref('mass_mail_1_stat_0')}"/>
         </function>
         <function model="link.tracker.click" name="add_click">
-            <!-- code, ip, country_code, stat_id -->
             <value model="link.tracker.code"
                 search="[('link_id.url', '=', 'http://www.example.com')]"
                 use="code"/>
-            <value eval="'100.01.02.04'"/>
-            <value eval="'BE'"/>
-            <value eval="ref('mass_mail_1_stat_1')"/>
+            <value eval="{'ip': '100.01.02.04', 'country_code': 'BE', 'mail_stat_id': ref('mass_mail_1_stat_1')}"/>
         </function>
         <function model="link.tracker.click" name="add_click">
-            <!-- code, ip, country_code, stat_id -->
             <value model="link.tracker.code"
                 search="[('link_id.url', '=', 'http://www.example.net/page/contactus')]"
                 use="code"/>
+            <value eval="{'ip': '100.01.02.04', 'country_code': 'BE', 'mail_stat_id': ref('mass_mail_1_stat_0')}"/>
             <value eval="'100.01.02.04'"/>
             <value eval="'BE'"/>
             <value eval="ref('mass_mail_1_stat_0')"/>
         </function>
         <function model="link.tracker.click" name="add_click">
-            <!-- code, ip, country_code, stat_id -->
             <value model="link.tracker.code"
                 search="[('link_id.url', '=', 'http://www.example.com')]"
                 use="code"/>
-            <value eval="'100.01.02.05'"/>
-            <value eval="'BE'"/>
-            <value eval="ref('mass_mail_1_stat_2')"/>
+            <value eval="{'ip': '100.01.02.05', 'country_code': 'BE', 'mail_stat_id': ref('mass_mail_1_stat_2')}"/>
         </function>
 
     </data>
diff --git a/addons/mass_mailing/models/link_tracker.py b/addons/mass_mailing/models/link_tracker.py
index c7077191dab..2b20b007a4b 100644
--- a/addons/mass_mailing/models/link_tracker.py
+++ b/addons/mass_mailing/models/link_tracker.py
@@ -18,22 +18,27 @@ class LinkTrackerClick(models.Model):
     mass_mailing_id = fields.Many2one('mail.mass_mailing', string='Mass Mailing')
     mass_mailing_campaign_id = fields.Many2one('mail.mass_mailing.campaign', string='Mass Mailing Campaign')
 
-    @api.model
-    def add_click(self, code, ip, country_code, stat_id=False):
-        res = super(LinkTrackerClick, self).add_click(code, ip, country_code, stat_id=stat_id)
-        if stat_id:
-            stat_sudo = self.env['mail.mail.statistics'].sudo().browse(stat_id)
-            stat_sudo.set_opened()
-            stat_sudo.set_clicked()
-        return res
-
-    def _get_click_values_from_route(self, route_values):
-        click_values = super(LinkTrackerClick, self)._get_click_values_from_route(route_values)
-        if route_values['stat_id']:
-            mail_stat = self.env['mail.mail.statistics'].browse(route_values['stat_id'])
-            click_values['mail_stat_id'] = mail_stat.id
-            if mail_stat.mass_mailing_campaign_id:
-                click_values['mass_mailing_campaign_id'] = mail_stat.mass_mailing_campaign_id.id
-            if mail_stat.mass_mailing_id:
-                click_values['mass_mailing_id'] = mail_stat.mass_mailing_id.id
+    def _prepare_click_values_from_route(self, **route_values):
+        click_values = super(LinkTrackerClick, self)._prepare_click_values_from_route(**route_values)
+
+        if click_values.get('mail_stat_id'):
+            stat_sudo = self.env['mail.mail.statistics'].sudo().browse(route_values['mail_stat_id']).exists()
+            if not stat_sudo:
+                click_values['mail_stat_id'] = False
+            else:
+                if not click_values.get('mass_mailing_campaign_id'):
+                    click_values['mass_mailing_campaign_id'] = stat_sudo.mass_mailing_campaign_id.id
+                if not click_values.get('mass_mailing_id'):
+                    click_values['mass_mailing_id'] = stat_sudo.mass_mailing_id.id
+
         return click_values
+
+    @api.model
+    def add_click(self, code, **route_values):
+        click = super(LinkTrackerClick, self).add_click(code, **route_values)
+
+        if click and click.mail_stat_id:
+            click.mail_stat_id.set_opened()
+            click.mail_stat_id.set_clicked()
+
+        return click
diff --git a/addons/test_mass_mailing/tests/__init__.py b/addons/test_mass_mailing/tests/__init__.py
index 4283fdd0861..652f56e3078 100644
--- a/addons/test_mass_mailing/tests/__init__.py
+++ b/addons/test_mass_mailing/tests/__init__.py
@@ -4,6 +4,7 @@
 from . import test_blacklist
 from . import test_blacklist_mixin
 from . import test_composer
+from . import test_link
 from . import test_mail_auto_blacklist
 from . import test_mail_channel
 from . import test_mass_mailing
diff --git a/addons/test_mass_mailing/tests/test_link.py b/addons/test_mass_mailing/tests/test_link.py
new file mode 100644
index 00000000000..99819944d82
--- /dev/null
+++ b/addons/test_mass_mailing/tests/test_link.py
@@ -0,0 +1,65 @@
+# -*- coding: utf-8 -*-
+# Part of Odoo. See LICENSE file for full copyright and licensing details.
+
+from odoo.tests.common import users
+from odoo.addons.test_mass_mailing.tests import common
+
+
+class TestLinkTracker(common.MassMailingCase):
+
+    def setUp(self):
+        super(TestLinkTracker, self).setUp()
+
+        self.link = self.env['link.tracker'].create({
+            'url': 'https://www.example.com'
+        })
+
+        self.click = self.env['link.tracker.click'].create({
+            'link_id': self.link.id,
+            'ip': '100.00.00.00',
+            'country_id': self.env.ref('base.fr').id,
+        })
+
+    def test_add_link(self):
+        code = self.link.code
+        self.assertEqual(self.link.count, 1)
+
+        # click from a new IP should create a new entry
+        click = self.env['link.tracker.click'].sudo().add_click(
+            code,
+            ip='100.00.00.01',
+            country_code='BEL'
+        )
+        self.assertEqual(click.ip, '100.00.00.01')
+        self.assertEqual(click.country_id, self.env.ref('base.be'))
+        self.assertEqual(self.link.count, 2)
+
+        # click from same IP (even another country) does not create a new entry
+        click = self.env['link.tracker.click'].sudo().add_click(
+            code,
+            ip='100.00.00.01',
+            country_code='FRA'
+        )
+        self.assertEqual(click, None)
+        self.assertEqual(self.link.count, 2)
+
+    @users('marketing')
+    def test_add_link_mail_stat(self):
+        mailing = self.env['mail.mass_mailing'].create({'name': 'Test Mailing'})
+        code = self.link.code
+        self.assertEqual(self.link.count, 1)
+        stat = self.env['mail.mail.statistics'].create({'mass_mailing_id': mailing.id})
+        self.assertFalse(stat.opened)
+        self.assertFalse(stat.clicked)
+
+        # click from a new IP should create a new entry and update stat when provided
+        click = self.env['link.tracker.click'].sudo().add_click(
+            code,
+            ip='100.00.00.01',
+            country_code='BEL',
+            mail_stat_id=stat.id
+        )
+        self.assertEqual(self.link.count, 2)
+        self.assertEqual(click.mass_mailing_id, mailing)
+        self.assertTrue(stat.opened)
+        self.assertTrue(stat.clicked)