From acb5cabccdf5d610853845d147dcd61932728020 Mon Sep 17 00:00:00 2001 From: SimoRubi Date: Thu, 23 Apr 2020 11:35:52 +0200 Subject: [PATCH 1/2] bi_view_editor: window functions without ORDER BY clause do not have reliable behavior --- bi_view_editor/models/bve_view.py | 18 ++++++++++++++++-- bi_view_editor/readme/ROADMAP.rst | 1 + bi_view_editor/readme/USAGE.rst | 3 +++ bi_view_editor/views/bve_view.xml | 3 ++- 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/bi_view_editor/models/bve_view.py b/bi_view_editor/models/bve_view.py index 3039da31..1c10716f 100644 --- a/bi_view_editor/models/bve_view.py +++ b/bi_view_editor/models/bve_view.py @@ -98,6 +98,19 @@ class BveView(models.Model): compute='_compute_users', store=True) query = fields.Text(compute='_compute_sql_query') + over_condition = fields.Text( + states={ + 'draft': [ + ('readonly', False), + ], + }, + readonly=True, + help="Condition to be inserted in the OVER part " + "of the ID's row_number function.\n" + "For instance, 'ORDER BY t1.id' would create " + "IDs ordered in the same way as t1's IDs; otherwise " + "IDs are assigned with no specific order.", + ) er_diagram_image = fields.Binary(compute='_compute_er_diagram_image') _sql_constraints = [ @@ -285,11 +298,12 @@ class BveView(models.Model): self.env.cr.execute('CREATE or REPLACE VIEW %s as (%s)', ( AsIs(view_name), AsIs(query), )) - @api.depends('line_ids', 'state') + @api.depends('line_ids', 'state', 'over_condition') def _compute_sql_query(self): for bve_view in self: tables_map = {} - select_str = '\n CAST(row_number() OVER () as integer) AS id' + select_str = '\n CAST(row_number() OVER ({}) as integer) AS id' \ + .format(bve_view.over_condition or '') for line in bve_view.field_ids: table = line.table_alias select = line.field_id.name diff --git a/bi_view_editor/readme/ROADMAP.rst b/bi_view_editor/readme/ROADMAP.rst index 32e4e2d0..b7e7bd0f 100644 --- a/bi_view_editor/readme/ROADMAP.rst +++ b/bi_view_editor/readme/ROADMAP.rst @@ -5,3 +5,4 @@ * Data the user has no access to (e.g. in a multi company situation) can be viewed by making a view. Would be nice if models available to select when creating a view are limited to the ones that have intersecting groups. +* Raise a warning in case the SQL query is not correct. diff --git a/bi_view_editor/readme/USAGE.rst b/bi_view_editor/readme/USAGE.rst index 986ce305..b243f0ef 100644 --- a/bi_view_editor/readme/USAGE.rst +++ b/bi_view_editor/readme/USAGE.rst @@ -21,3 +21,6 @@ To access the created BI View with a dedicated menu: A more advanced UI is also available under the "Details" tab. It provides extra possibilities for more advanced users, like to use LEFT JOIN instead of the default INNER JOIN. + +It also possible to improve the IDs generation for new views by adding an `Over Condition` in the "SQL" tab, see https://www.postgresql.org/docs/current/sql-expressions.html#SYNTAX-WINDOW-FUNCTIONS for further details. +For instance, an ORDER BY clause helps preventing unreliable behavior when filtering the generated views. diff --git a/bi_view_editor/views/bve_view.xml b/bi_view_editor/views/bve_view.xml index 2599fc67..101fa0e9 100644 --- a/bi_view_editor/views/bve_view.xml +++ b/bi_view_editor/views/bve_view.xml @@ -85,8 +85,9 @@ + - + From b9f77fb79164cf4df646c9f16b21907cf17dc0fb Mon Sep 17 00:00:00 2001 From: SimoRubi Date: Thu, 7 May 2020 14:39:04 +0200 Subject: [PATCH 2/2] bi_view_editor: Better error message if the query is not correct --- bi_view_editor/models/bve_view.py | 12 ++++++++++-- bi_view_editor/readme/ROADMAP.rst | 1 - bi_view_editor/tests/test_bi_view.py | 22 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/bi_view_editor/models/bve_view.py b/bi_view_editor/models/bve_view.py index 1c10716f..58ac76a8 100644 --- a/bi_view_editor/models/bve_view.py +++ b/bi_view_editor/models/bve_view.py @@ -295,8 +295,16 @@ class BveView(models.Model): self._cr.execute('DROP TABLE IF EXISTS %s', (AsIs(view_name), )) # create postgres view - self.env.cr.execute('CREATE or REPLACE VIEW %s as (%s)', ( - AsIs(view_name), AsIs(query), )) + try: + with self.env.cr.savepoint(): + self.env.cr.execute('CREATE or REPLACE VIEW %s as (%s)', ( + AsIs(view_name), AsIs(query), )) + except Exception as e: + raise UserError( + _("Error creating the view '{query}':\n{error}") + .format( + query=query, + error=e)) @api.depends('line_ids', 'state', 'over_condition') def _compute_sql_query(self): diff --git a/bi_view_editor/readme/ROADMAP.rst b/bi_view_editor/readme/ROADMAP.rst index b7e7bd0f..32e4e2d0 100644 --- a/bi_view_editor/readme/ROADMAP.rst +++ b/bi_view_editor/readme/ROADMAP.rst @@ -5,4 +5,3 @@ * Data the user has no access to (e.g. in a multi company situation) can be viewed by making a view. Would be nice if models available to select when creating a view are limited to the ones that have intersecting groups. -* Raise a warning in case the SQL query is not correct. diff --git a/bi_view_editor/tests/test_bi_view.py b/bi_view_editor/tests/test_bi_view.py index 38e303b8..7404dcd9 100644 --- a/bi_view_editor/tests/test_bi_view.py +++ b/bi_view_editor/tests/test_bi_view.py @@ -5,6 +5,7 @@ import json import odoo from odoo.tests.common import TransactionCase +from odoo.tools import mute_logger from odoo.exceptions import UserError, ValidationError from ..hooks import post_load, uninstall_hook @@ -407,3 +408,24 @@ class TestBiViewEditor(TransactionCase): bi_view1 = self.env['bve.view'].create(vals) bi_view1.action_create() self.assertEqual(len(bi_view1.line_ids), 4) + + @mute_logger('odoo.sql_db') + def test_20_broken_view(self): + """ + Create a broken query, a nice UserError should be raised. + odoo.sql_db logger is muted to avoid the + ERROR: bad_query line in the logs. + """ + vals = self.bi_view1_vals + vals.update({ + 'name': 'Test View broken', + 'over_condition': 'bad SQL code', + }) + bi_view = self.env['bve.view'].create(vals) + with self.assertRaises(UserError) as ue: + bi_view.action_create() + + self.assertEqual(bi_view.state, 'draft') + self.assertIn(bi_view.over_condition, str(ue.exception)) + # remove view + bi_view.unlink()