ADD sql_export module

[florian-dacosta]

This module is usefull when a user knows how to make sql request. He can extract data from the database easily.
He also can choose wich user is able to use the sql resquest he creates.

[lmignon]

Do you depend on 'mrp'? IMO this should be 'base'.

[florian-dacosta]

Of course! I'll change that now, it is a mistake.

[lmignon]

IMO the second if is too restrictive. It's sometimes useful to build query using the with clause (http://www.postgresql.org/docs/9.1/static/queries-with.html). Moreover, if you want to test that the query starts with select you should write

            if not obj.query.lower().startswith('select'):
                return False

IMO, checking for prohibited works is enough.

[florian-dacosta]

Thanks for your comment!
You are right, checking first word is a bit restrictive.
But I am not sure checking for prohibited words is enought.
I could forget one or two important since I am not a huge expert in sql
Also, I did not include some word like update or create in this list.
So someone could update any tables, which we do not want!

I chose not to put create because of the field create_date that we may want to extract.
Also update, is easily used in fields name. (like in prestashop or magento connector)
That is why I restrict on the word SELECT.

Of course, I am open to idea to make it better.

[lmignon]

You should use a regular expression to search for the whole word and include update and create in your list of prohibited words.

[lmignon]

@florian-dacosta Thank you for this PR: nice functionality for advanced user only. Your module works fine (tested in my env). Some additional remarks.

• If you don't provides an icon, use the one provided by OCA (https://github.com/OCA/maintainer-tools/tree/master/template/module/static/description)
• As it's a new functionality, you should also provide a unittest for your module.
• You should rename your file sql.py to sql_export.py since it only provides 1 model (and sql_view.xml -> sql_export_view.xml).

lmi

[lmignon]

you should query ir.model.data with the xmlid

def _get_editor_group(self, cr, uid, *args):
    model_obj = self.pool['ir.model.data']
    return model_obj.get_object_reference(
        cr, uid, 'sql_export', 'group_sql_request_editor')[1]

[florian-dacosta]

@lmignon Thanks for your feedback.
It should be ok now!

[lmignon]

the url is https://github.com/OCA/server-tools/issues 😏
and version is the one of the branch 8.0

[lmignon]

@florian-dacosta Thank you, good job! A few comments remain but the module looks fine.

[florian-dacosta]

@lmignon No problem, I took your last comments into consideration

[lmignon]

You should repeat the 'with' statement before each call to self.sql_model.create
Currently, the assertion is true once one call fails.

def test_prohibited_queries_creation(self):
    prohibited_queries = [
        "upDaTe sale_order SET partner_id = 1 WHERE id = 1",
        "DELETE FROM sale_order;",
        "  DELETE FROM sale_order   ;",
        """DELETE
         FROM
         sale_order
         """,
    ]
    for query in prohibited_queries:
        with self.assertRaises(except_orm):
            self.sql_model.create(self.cr, self.uid, {'name': 'test_prohibited',
                                                      'query': query})
    wrong_syntax_query = {
        'name': 'test delete3',
        'query': """SELCT id FROM sale_order"""
    }
    with self.assertRaises(except_orm):
        self.sql_model.create(self.cr, self.uid, wrong_syntax_query)

    ok_query = {
       'name': 'test ok',
        'query': "SELECT create_date FROM sale_order"
    }
    query_id = self.sql_model.create(self.cr, self.uid, ok_query)
    self.assertIsNotNone(query_id)

[lmignon]

👍 once travis is green

[lmignon]

Your tests are never launched by odoo .
You need to put your test class into a checks list

checks = [
    test_sql_query
]

[florian-dacosta]

I did not know that! I did unit test the same way very recently for a module in v8, and odoo was launching the tests without any troubles.
Is it specific to the v7?

Thanks once again for your help!

[lmignon]

in v8, tests are now automatically launched if they are written under the module 'tests' in a submodule with a the name starting with test_
But before V8, we need to add the mentioned lines to say to Odoo which tests must be launched

[florian-dacosta]

Well, I can't see if it ran the tests.
Strange, because in localhost, my odoo did run the tests without any problem.

[laetitia-gangloff]

👍 (functionnal use)
nice module ! thank you !

[dreispt]

If you leave this description key here, it will override the README. Maybe it's best to remove it.
Sorry - not true for v7.

[dreispt]

IIMO we're polluting the top menu with yet another option.
Can we fit it in an existing top menu?
Since creating the SQL is quite a reserved feature, I suggest having it under Reporting\Configuration (base.menu_reporting_config).

[florian-dacosta]

I agree it is annoying to add one more option on the top menu.
The problem is that this feature can be used by almost every user.
Yes, just one or two user should be able to create a new sql request, but the requests can be executed by users in sales, purchases, warehouse, etc...
For each request, you can add groups or users, that will determine who can read this request. That is why we need to put the menu somewhere every users can have access.

[dreispt]

AFAIK the Reporting menu is available for all, only it's option are limited per security group. Can you try that?

[florian-dacosta]

You are right, it should be ok in the reporting menu.
I'll just add a menu under Reporting/
I don't think it belongs to the configuration submenu

[dreispt]

Agreed. Use "SQL" in uppercase in the menu labels?

[laetitia-gangloff]

I think it is possible to merge.

[gurneyalex]

👍