From f462bde3544f7d7afe76bfb59981dbb2b687a474 Mon Sep 17 00:00:00 2001
From: Enric Tobella <etobella@creublanca.es>
Date: Tue, 28 Apr 2020 19:10:30 +0200
Subject: [PATCH] [12.0][IMP] kpi_dashboard: Compute the KPI on a separate
 cursor and rollback it

---
 kpi_dashboard/README.rst                    | 20 ++++++---
 kpi_dashboard/__manifest__.py               |  3 +-
 kpi_dashboard/demo/demo_dashboard.xml       |  1 -
 kpi_dashboard/i18n/kpi_dashboard.pot        | 29 ++++++++++++
 kpi_dashboard/models/kpi_kpi.py             | 22 ++++++++-
 kpi_dashboard/static/description/index.html | 19 +++++---
 kpi_dashboard/tests/test_formula.py         | 50 ++++++++++++++++-----
 7 files changed, 117 insertions(+), 27 deletions(-)

diff --git a/kpi_dashboard/README.rst b/kpi_dashboard/README.rst
index db74bc809..a8dc09cf3 100644
--- a/kpi_dashboard/README.rst
+++ b/kpi_dashboard/README.rst
@@ -51,12 +51,14 @@ Configure KPIs
 Using KPI with code
 ~~~~~~~~~~~~~~~~~~~
 
-Define the code directly on the code field. You can use:
-
-* `self` and `model` as the kpi element
-* The script should create a variable called `result` as a dictionary that
-  will be stored as the value
+Define the code directly on the code field. You can use `self` and `model` as the kpi element
+The script should create a variable called `result` as a dictionary that
+will be stored as the value.
+For example, we can use::
 
+    result = {}
+    result['value'] = len(model.search([('id', '=', %s)]))
+    result['previous'] = len(model.search([('id', '!=', %s)]))
 
 Configure dashboards
 ~~~~~~~~~~~~~~~~~~~~
@@ -103,6 +105,14 @@ OCA, or the Odoo Community Association, is a nonprofit organization whose
 mission is to support the collaborative development of Odoo features and
 promote its widespread use.
 
+.. |maintainer-etobella| image:: https://github.com/etobella.png?size=40px
+    :target: https://github.com/etobella
+    :alt: etobella
+
+Current `maintainer <https://odoo-community.org/page/maintainer-role>`__:
+
+|maintainer-etobella| 
+
 This module is part of the `OCA/reporting-engine <https://github.com/OCA/reporting-engine/tree/12.0/kpi_dashboard>`_ project on GitHub.
 
 You are welcome to contribute. To learn how please visit https://odoo-community.org/page/Contribute.
diff --git a/kpi_dashboard/__manifest__.py b/kpi_dashboard/__manifest__.py
index 7f45bb92b..52b594aca 100644
--- a/kpi_dashboard/__manifest__.py
+++ b/kpi_dashboard/__manifest__.py
@@ -5,7 +5,7 @@
     "name": "Kpi Dashboard",
     "summary": """
         Create Dashboards using kpis""",
-    "version": "12.0.1.0.1",
+    "version": "12.0.1.1.0",
     "license": "AGPL-3",
     "author": "Creu Blanca,Odoo Community Association (OCA)",
     "website": "https://github.com/reporting-engine",
@@ -21,4 +21,5 @@
         "views/kpi_dashboard.xml",
     ],
     "demo": ["demo/demo_dashboard.xml"],
+    "maintainers": ["etobella"],
 }
diff --git a/kpi_dashboard/demo/demo_dashboard.xml b/kpi_dashboard/demo/demo_dashboard.xml
index e86f457b0..958e15806 100644
--- a/kpi_dashboard/demo/demo_dashboard.xml
+++ b/kpi_dashboard/demo/demo_dashboard.xml
@@ -87,7 +87,6 @@ result = {"graphs": [
     <function model="kpi.kpi" name="compute"
               eval="[[ref('widget_graph')]]"/>
 
-
     <record id="dashboard_widget_text" model="kpi.dashboard.item">
         <field name="name">Dashboard title</field>
         <field name="dashboard_id" ref="demo_dashboard"/>
diff --git a/kpi_dashboard/i18n/kpi_dashboard.pot b/kpi_dashboard/i18n/kpi_dashboard.pot
index 18e81f2d0..3974c4164 100644
--- a/kpi_dashboard/i18n/kpi_dashboard.pot
+++ b/kpi_dashboard/i18n/kpi_dashboard.pot
@@ -13,6 +13,12 @@ msgstr ""
 "Content-Transfer-Encoding: \n"
 "Plural-Forms: \n"
 
+#. module: kpi_dashboard
+#: code:addons/kpi_dashboard/models/kpi_kpi.py:106
+#, python-format
+msgid " or "
+msgstr ""
+
 #. module: kpi_dashboard
 #. openerp-web
 #: code:addons/kpi_dashboard/static/src/js/dashboard_controller.js:46
@@ -60,6 +66,11 @@ msgstr ""
 msgid "Add to Dashboard"
 msgstr ""
 
+#. module: kpi_dashboard
+#: selection:kpi.kpi,widget:0
+msgid "Altair"
+msgstr ""
+
 #. module: kpi_dashboard
 #: model:ir.model.fields,field_description:kpi_dashboard.field_kpi_kpi__args
 msgid "Args"
@@ -81,6 +92,13 @@ msgstr ""
 msgid "Cancel"
 msgstr ""
 
+#. module: kpi_dashboard
+#: model:ir.model.fields,field_description:kpi_dashboard.field_kpi_kpi__code
+#: model_terms:ir.ui.view,arch_db:kpi_dashboard.kpi_kpi_form_view
+#: selection:kpi.kpi,computation_method:0
+msgid "Code"
+msgstr ""
+
 #. module: kpi_dashboard
 #: model:ir.model.fields,field_description:kpi_dashboard.field_kpi_dashboard_item__color
 msgid "Color"
@@ -192,6 +210,11 @@ msgstr ""
 msgid "End Row"
 msgstr ""
 
+#. module: kpi_dashboard
+#: model_terms:ir.ui.view,arch_db:kpi_dashboard.kpi_kpi_form_view
+msgid "Enter Python code here."
+msgstr ""
+
 #. module: kpi_dashboard
 #. openerp-web
 #: code:addons/kpi_dashboard/static/src/js/dashboard_controller.js:31
@@ -471,6 +494,12 @@ msgstr ""
 msgid "Suffix"
 msgstr ""
 
+#. module: kpi_dashboard
+#: code:addons/kpi_dashboard/models/kpi_kpi.py:108
+#, python-format
+msgid "The code cannot contain the following terms: %s."
+msgstr ""
+
 #. module: kpi_dashboard
 #: selection:ir.actions.act_window.view,view_mode:0
 #: selection:ir.ui.view,type:0
diff --git a/kpi_dashboard/models/kpi_kpi.py b/kpi_dashboard/models/kpi_kpi.py
index b6c7c030a..275818d98 100644
--- a/kpi_dashboard/models/kpi_kpi.py
+++ b/kpi_dashboard/models/kpi_kpi.py
@@ -1,9 +1,11 @@
 # Copyright 2020 Creu Blanca
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
-from odoo import api, fields, models
+from odoo import api, fields, models, _
+from odoo.exceptions import ValidationError
 import ast
 from odoo.tools.safe_eval import safe_eval
+import re
 
 
 class KpiKpi(models.Model):
@@ -89,12 +91,28 @@ class KpiKpi(models.Model):
     def _get_code_input_dict(self):
         return {
             "self": self,
-            "model": self,
+            "model": self.browse(),
         }
 
+    def _forbidden_code(self):
+        return ["commit", "rollback", "getattr", "execute"]
+
     def _compute_value_code(self):
+        forbidden = self._forbidden_code()
+        search_terms = "(" + ("|".join(forbidden)) + ")"
+        if re.search(search_terms, (self.code or "").lower()):
+            message = ", ".join(forbidden[:-1]) or ""
+            if len(message) > 0:
+                message += _(" or ")
+            message += forbidden[-1]
+            raise ValidationError(_(
+                "The code cannot contain the following terms: %s."
+            ) % message)
         results = self._get_code_input_dict()
+        savepoint = "kpi_formula_%s" % self.id
+        self.env.cr.execute("savepoint %s" % savepoint)
         safe_eval(self.code or "", results, mode="exec", nocopy=True)
+        self.env.cr.execute("rollback to %s" % savepoint)
         return results.get("result", {})
 
 
diff --git a/kpi_dashboard/static/description/index.html b/kpi_dashboard/static/description/index.html
index bde4304d4..09f8b1f9d 100644
--- a/kpi_dashboard/static/description/index.html
+++ b/kpi_dashboard/static/description/index.html
@@ -3,7 +3,7 @@
 <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
 <head>
 <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
-<meta name="generator" content="Docutils 0.14: http://docutils.sourceforge.net/" />
+<meta name="generator" content="Docutils 0.15.1: http://docutils.sourceforge.net/" />
 <title>Kpi Dashboard</title>
 <style type="text/css">
 
@@ -405,12 +405,15 @@ use the code to directly compute it.</li>
 </div>
 <div class="section" id="using-kpi-with-code">
 <h2><a class="toc-backref" href="#id3">Using KPI with code</a></h2>
-<p>Define the code directly on the code field. You can use:</p>
-<ul class="simple">
-<li><cite>self</cite> and <cite>model</cite> as the kpi element</li>
-<li>The script should create a variable called <cite>result</cite> as a dictionary that
-will be stored as the value</li>
-</ul>
+<p>Define the code directly on the code field. You can use <cite>self</cite> and <cite>model</cite> as the kpi element
+The script should create a variable called <cite>result</cite> as a dictionary that
+will be stored as the value.
+For example, we can use:</p>
+<pre class="literal-block">
+result = {}
+result['value'] = len(model.search([('id', '=', %s)]))
+result['previous'] = len(model.search([('id', '!=', %s)]))
+</pre>
 </div>
 <div class="section" id="configure-dashboards">
 <h2><a class="toc-backref" href="#id4">Configure dashboards</a></h2>
@@ -452,6 +455,8 @@ If you spotted it first, help us smashing it by providing a detailed and welcome
 <p>OCA, or the Odoo Community Association, is a nonprofit organization whose
 mission is to support the collaborative development of Odoo features and
 promote its widespread use.</p>
+<p>Current <a class="reference external" href="https://odoo-community.org/page/maintainer-role">maintainer</a>:</p>
+<p><a class="reference external" href="https://github.com/etobella"><img alt="etobella" src="https://github.com/etobella.png?size=40px" /></a></p>
 <p>This module is part of the <a class="reference external" href="https://github.com/OCA/reporting-engine/tree/12.0/kpi_dashboard">OCA/reporting-engine</a> project on GitHub.</p>
 <p>You are welcome to contribute. To learn how please visit <a class="reference external" href="https://odoo-community.org/page/Contribute">https://odoo-community.org/page/Contribute</a>.</p>
 </div>
diff --git a/kpi_dashboard/tests/test_formula.py b/kpi_dashboard/tests/test_formula.py
index 4f37d9990..abe73d4ee 100644
--- a/kpi_dashboard/tests/test_formula.py
+++ b/kpi_dashboard/tests/test_formula.py
@@ -2,30 +2,58 @@
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
 from odoo.tests.common import TransactionCase
+from odoo.exceptions import ValidationError
 
 
 class TestFormula(TransactionCase):
-    def test_computation(self):
-        kpi = self.env["kpi.kpi"].create(
+    def setUp(self):
+        super().setUp()
+        self.kpi = self.env["kpi.kpi"].create(
             {
                 "name": "DEMO KPI",
                 "widget": "number",
                 "computation_method": "code",
             }
         )
-        self.assertFalse(kpi.value)
-        kpi.compute()
-        self.assertEqual(kpi.value, {})
-        kpi.code = """
+
+    def test_forbidden_words_01(self):
+        self.kpi.code = """
+result = {"value": 0}
+self.env.cr.commit()
+"""
+        with self.assertRaises(ValidationError):
+            self.kpi.compute()
+
+    def test_forbidden_words_02(self):
+        self.kpi.code = """
+result = {"value": 0}
+self.env.cr.rollback()
+"""
+        with self.assertRaises(ValidationError):
+            self.kpi.compute()
+
+    def test_forbidden_words_03(self):
+        self.kpi.code = """
+result = {"value": 0}
+self.env.cr.execute("CoMMiT")
+"""
+        with self.assertRaises(ValidationError):
+            self.kpi.compute()
+
+    def test_computation(self):
+        self.assertFalse(self.kpi.value)
+        self.kpi.compute()
+        self.assertEqual(self.kpi.value, {})
+        self.kpi.code = """
 result = {}
 result['value'] = len(model.search([('id', '=', %s)]))
 result['previous'] = len(model.search([('id', '!=', %s)]))
         """ % (
-            kpi.id,
-            kpi.id,
+            self.kpi.id,
+            self.kpi.id,
         )
-        kpi.compute()
-        value = kpi.value
+        self.kpi.compute()
+        value = self.kpi.value
         self.assertTrue(value.get("value"))
         self.assertEqual(value.get("value"), 1)
-        self.assertEqual(value.get("previous"), kpi.search_count([]) - 1)
+        self.assertEqual(value.get("previous"), self.kpi.search_count([]) - 1)