Review: Approve
--
https://code.launchpad.net/~camptocamp/account-budgeting/7.0-fix-store-fields-mdh/+merge/216849
Your team Account Core Editors is subscribed to branch lp:account-budgeting.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to :
Review: Approve code review, no test
--
https://code.launchpad.net/~camptocamp/account-budgeting/7.0-fix-store-fields-mdh/+merge/216849
Your team Account Core Editors is subscribed to branch lp:account-budgeting.
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to :
The proposal to merge lp:~camptocamp/account-budgeting/7.0-fix-store-fields-mdh
into lp:account-budgeting has been updated.
Description changed to:
The object in the store condition was the wrong one. I fixed it for the correct
one + fixed the now-called function.
For more details, see:
Matthieu Dietrich @ camptocamp has proposed merging
lp:~camptocamp/account-budgeting/7.0-fix-store-fields-mdh into
lp:account-budgeting.
Commit message:
[FIX] fixed store condition on fields 'analytic_amount', 'analytic_real_amount'
and 'analytic_diff_amount' + fixed function called by the
Leonardo,
thanks for the review. For your questions:
1. yes, that field needs to be stored. The initial commit mentions that a sum
of the lines is used in a groupby view, which is the default view for budget
lines, coming from a project.
2. I think the optimal solution would be to retrieve
@Leonardo: here is a SQL version of the request.
Tell me if that's okay for you.
Regards,
Matthieu Dietrich
--
https://code.launchpad.net/~camptocamp/account-budgeting/7.0-fix-store-fields-mdh/+merge/216849
Your team Account Core Editors is requested to review the proposed merge of
Review: Needs Fixing code review, no test
Your SQL request maybe works but it is not written in an efficient way (2
DISTINCT, 1 subquery). This should be written with EXISTS.
And, each time you bypass the ORM to write raw SQL, $DEITY kills a kitten.
$DEITY also maims a puppy if you don't
@Alexandre, Leonardo:
I went back for a classical ORM approach. I don't think a store condition's
function is called by another user than the SUPERUSER, but it's better to be
safe.
I'm still refining a YAML test as well to add to this MP.
Regards,
Matthieu
--
thanks Matthieu.
I'm also not a fan of the SQL if I don't have a gun pointed at me. Hopefully
with the test it will be easier to improve that.
--
https://code.launchpad.net/~camptocamp/account-budgeting/7.0-fix-store-fields-mdh/+merge/216849
Your team Account Core Editors is subscribed to
Hello,
I added the YAML test, in order to verify that the amounts are updated after a
new analytic line on the same analytic account was added.
Regards,
Matthieu
--
https://code.launchpad.net/~camptocamp/account-budgeting/7.0-fix-store-fields-mdh/+merge/216849
Your team Account Core Editors is
Review: Approve code review, no test
Thanks, you saved some kittens
--
https://code.launchpad.net/~camptocamp/account-budgeting/7.0-fix-store-fields-mdh/+merge/216849
Your team Account Core Editors is subscribed to branch lp:account-budgeting.
--
Mailing list:
11 matches
Mail list logo
