Merge lp://qastaging/~camptocamp/ocb-addons/7.0-bugfix-1189480-with-perf-mdh into lp://qastaging/ocb-addons
Proposed by
Matthieu Dietrich @ camptocamp
| Status: | Merged |
|---|---|
| Merged at revision: | 9864 |
| Proposed branch: | lp://qastaging/~camptocamp/ocb-addons/7.0-bugfix-1189480-with-perf-mdh |
| Merge into: | lp://qastaging/ocb-addons |
| Diff against target: |
330 lines (+243/-20) 3 files modified
project/__openerp__.py (+1/-0) project/project.py (+68/-20) project/test/hours_process.yml (+174/-0) |
| To merge this branch: | bzr merge lp://qastaging/~camptocamp/ocb-addons/7.0-bugfix-1189480-with-perf-mdh |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Yannick Vaucher @ Camptocamp | code review | Approve | |
| Holger Brunn (Therp) | code review | Approve | |
| Matthieu Dietrich @ camptocamp (community) | Needs Resubmitting | ||
| Alexandre Fayolle - camptocamp | code review, no test | Pending | |
| Nicolas Bessi - Camptocamp | Pending | ||
|
Review via email:
|
|||
This proposal supersedes a proposal from 2013-11-29.
Description of the change
The quick fix proposed in lp:~camptocamp/openobject-addons/7.0-fixes-bug-1189480 works, but the multiple read() severely impact performance. This solution uses a WITH RECURSIVE SQL statement in order to 1) avoid any extraneous read() 2) get all the information in one request.
I also added a YAML test to test both this new method and a hierarchical computation, since all YAML tests in addons are for a single level of projects.
To post a comment you must log in.
Revision history for this message
| Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote | # |
review:
Needs Fixing
(code review, no test)
Revision history for this message
| Matthieu Dietrich @ camptocamp (mdietrich-c2c) wrote | # |
review:
Needs Resubmitting
Revision history for this message
| Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) | # |
review:
Approve
(code review, no test)
Revision history for this message
| Holger Brunn (Therp) (hbrunn) | # |
review:
Approve
(code review)
Revision history for this message
| Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote | # |
review:
Approve
(functionnal + code review)
Revision history for this message
| Yannick Vaucher @ Camptocamp (yvaucher-c2c) | # |
review:
Approve
(code review)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
I would like to see a multicompany test case for this as using raw SQL always has a chance of bypassing security (or a comment in the code explaining why it does not bypass multicompany rules)
add space after ':' in litteral dictionaries
I'm concerned about the use of "==" to compare floats in the tests, since this can lead to assertion failures due to the binary representation of floats.