Account - Financial Tools

Merge lp://staging/~acsone-openerp/account-financial-tools/account_partner_required-sbi into lp://staging/~account-core-editors/account-financial-tools/7.0

account_partner_required-sbi
Merge into 7.0

Proposed by Stéphane Bidoul (Acsone) on 2014-04-18

Status:	Merged
Merged at revision:	188
Proposed branch:	lp://staging/~acsone-openerp/account-financial-tools/account_partner_required-sbi
Merge into:	lp://staging/~account-core-editors/account-financial-tools/7.0
Diff against target:	382 lines (+351/-0) 6 files modified account_partner_required/__init__.py (+23/-0) account_partner_required/__openerp__.py (+46/-0) account_partner_required/account.py (+96/-0) account_partner_required/account_view.xml (+35/-0) account_partner_required/tests/__init__.py (+30/-0) account_partner_required/tests/test_account_partner_required.py (+121/-0)
To merge this branch:	bzr merge lp://staging/~acsone-openerp/account-financial-tools/account_partner_required-sbi
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Omar (Pexego)	code review		Approve on 2014-06-06
Joël Grand-Guillaume @ camptocamp	code review, no tests	2014-04-18	Approve on 2014-05-09
Lorenzo Battistini (community)	code review		Approve on 2014-04-29
Frederic Clementi - Camptocamp		2014-04-18	Pending
Review via email: mp+216442@code.staging.launchpad.net

Description of the change

New module to control the partner field in account move lines.

In many cases, it is desirable to enforce the presence of the
partner field in account move lines, but not always.

This module provides such a mechanism based on the account user type.

Cfr mailing list discussion for reference.

[1] https://lists.launchpad.net/openerp-community/msg05502.html

lp://staging/~acsone-openerp/account-financial-tools/account_partner_required-sbi updated on 2014-04-19

175. By Stéphane Bidoul (Acsone) on 2014-04-19: [IMP] add policy column in account type tree view
176. By Stéphane Bidoul (Acsone) on 2014-04-19: [IMP] local import
177. By Stéphane Bidoul (Acsone) on 2014-04-19: [IMP] header comments

Revision history for this message

Lorenzo Battistini (elbati) wrote on 2014-04-22:

Hello Stéphane,

why not using '_constraints' member instead of overriding create and write methods?

Is there any technical reason to avoid YAML tests?

Thanks!

review: Needs Information

Revision history for this message

Stéphane Bidoul (Acsone) (sbi) wrote on 2014-04-25:

Hello Lorenzo,

About the constraints. This module is heavily inspired by account_analytic_required which used the same technique. I must confess I did not question the approach. Initially account_analytic_required was working mainly on the vals dictionary so it would have been faster than constraints. While working on account_partner_required I identified a bug in account_analytic_required which required a refactoring [1] which possibly rendered the optimization less obvious. Now since there are really two constraints to be checked based on the policy, the current approach is probably still slightly faster.

Would you agree to keep the current approach and postpone a possible performance test / refactoring with constraints to another MP?

Regarding yml vs unittest, it's only a matter of personal taste in this case. I notice the unittest2 framework seems better documented [2]. But hey, at least I have tests, right :)

[1] https://code.launchpad.net/~acsone-openerp/account-analytic/account_analytic_required-test_suite-sbi
[2] https://doc.openerp.com/trunk/server/05_test_framework/