[12.0][ENH] sql view - allow group operator #352
Conversation
Replaces OCA#345 Adds the possibility, for float and integer columns, to apply a group operator (average, min, max).
There was a problem hiding this comment.
Hi @richard-willdooit
Thanks a LOT, for this improvment !
That is for sure a big limitation of the current module bi_sql_editor.
What about to integrate this code to the main module. I don't see the interest to add a very important feature in an extra module. What do you think ?
(partial code review for the time being).
| ('max', 'Maximum'), | ||
| ] | ||
|
|
||
| _GRAPH_TYPE_SELECTION = [ |
| super()._add_manual_fields(model) | ||
| if 'bi.sql.view' in self.env: | ||
| Sql = self.env['bi.sql.view'] | ||
| if hasattr(Sql, 'check_manual_fields'): |
There was a problem hiding this comment.
why this check ?
I guess that it is allways true, if the module is installed ? or did I missed something ?
There was a problem hiding this comment.
@legalsylvain This check MAY be unnecessary now, but see my comment below....
|
@legalsylvain I first wrote this as an integrated part of the module - see #344 and #345 - however, due to the override of add_manual_fields, upgrading when adding the module turned in to a nightmare - there were times during the instanciation of the models when certain fields and attributes were not yet available or visible. As it is, we need to be careful if it gets merged in to the one module. I was much happier integrating it to the single module, but I had trouble upgrading two production instances for different issues (it depended on the order that models and fields were being added) - in the worse of the two, I had a maximum recursion every time the odoo server started, and I was only able to get the module installed by manipulating column definitions directly with PostgreSQL - not pretty. @CasVissers had a similar problem on #344 I am happy to re-propose the previous two PRs, but I cannot work out a clean and always reproducable upgrade 😕 |
|
@legalsylvain Do you know what we need to do to get this merged? |
|
This PR has the |
|
/ocabot merge minor |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at 481b205. Thanks a lot for contributing to OCA. ❤️ |
Replaces #345
Adds the possibility, for float and integer columns, to
apply a group operator (average, min, max).