Account - Financial Tools

Merge lp://staging/~camptocamp/account-financial-tools/account-constrinat-parent-object into lp://staging/~account-core-editors/account-financial-tools/7.0

account-constrinat-parent-object
Merge into 7.0

Proposed by Joël Grand-Guillaume @ camptocamp on 2013-01-21

Status:	Merged
Merge reported by:	Joël Grand-Guillaume @ camptocamp
Merged at revision:	not available
Proposed branch:	lp://staging/~camptocamp/account-financial-tools/account-constrinat-parent-object
Merge into:	lp://staging/~account-core-editors/account-financial-tools/7.0
Diff against target:	157 lines (+124/-1) 2 files modified account_constraints/__openerp__.py (+7/-0) account_constraints/account_constraints.py (+117/-1)
To merge this branch:	bzr merge lp://staging/~camptocamp/account-financial-tools/account-constrinat-parent-object
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Guewen Baconnier @ Camptocamp			Approve on 2013-01-28
Alexandre Fayolle - camptocamp	code review, no test	2013-01-21	Needs Fixing on 2013-01-21
Review via email: mp+144113@code.staging.launchpad.net

Description of the change

On Account constraints:

  Remove the possibility to modify or delete a move line related to an
  invoice or a bank statement, no matter what the status of the move
  (draft, validated or posted). This is usefule in standard context but
  moreover if you're using : account_default_draft_move. This way you ensure
  user cannot make mistake even in draft, he must pass through the
  parent object to make his modification.

lp://staging/~camptocamp/account-financial-tools/account-constrinat-parent-object updated on 2013-01-21

100. By Joël Grand-Guillaume @ camptocamp on 2013-01-21: [FIX] Add the key 'from_parent_object' in context when creating move from invoice.

Revision history for this message

Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote on 2013-01-21:

in description: s/usefule/useful/
                s/in standard/in a standard/
                s/moreover/even more/
                s/using : account_default_draft_move/using `account_default_draft_move`/
                s/ensure user/ensure that the user/
                s/mistake/mistakes/
                s/in draft/in draft state/

line 42: s/Error!/Error/

line 58, line 71: s/Add the verification of:/Add the following checks:/

line 63, line 76: s/if you use the all move in/if you use the module setting all moves in/

line 99, line 120: s/buton cancel/"Cancel" button/

review: Needs Fixing (code review, no test)

lp://staging/~camptocamp/account-financial-tools/account-constrinat-parent-object updated on 2013-01-21

101. By Joël Grand-Guillaume @ camptocamp on 2013-01-21: [FIX] Typo regarding MP comments

Revision history for this message

Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote on 2013-01-22:

104, 112, 125, 132: the context is mutable, I think it would be safer to propagate a modified copy of the context.

review: Needs Fixing

Revision history for this message

Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote on 2013-01-22:

> 104, 112, 125, 132: the context is mutable, I think it would be safer to
> propagate a modified copy of the context.

The only risk is if some other module uses the key we are adding to the context for some other purpose, or am I missing something.

Revision history for this message

Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote on 2013-01-23:

> > 104, 112, 125, 132: the context is mutable, I think it would be safer to
> > propagate a modified copy of the context.
>
> The only risk is if some other module uses the key we are adding to the
> context for some other purpose, or am I missing something.

If a module implements a method which:

1. call create_move_from_st_line on an account.bank.statement
2. then call unlink directly on a move line

The 'from_parent_object' will be propagated to the second call, which is not what we want.

I admit this scenario is extremely improbable, but if such a bug happens, it will also be extremely hard to find.

Am I wrong?

Revision history for this message

Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote on 2013-01-23:

I agree with you Guewen... Would you or Alex please make the changes till end of week ? I will not be able to do them right now and we really need those constraints to be put in place...

Revision history for this message

Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote on 2013-01-23:

On mer. 23 janv. 2013 15:36:18 CET, Joël Grand-Guillaume @ camptocamp
wrote:
> I agree with you Guewen... Would you or Alex please make the changes till end of week ? I will not be able to do them right now and we really need those constraints to be put in place...

Not sure I'll be able to make before friday, because I have a lot of
work for a customer meeting on monday.

Should be able to tackle it next tuesday, though.

--
Alexandre Fayolle
Chef de Projet
Tel : + 33 (0)4 79 26 57 94

Camptocamp France SAS
Savoie Technolac, BP 352
73377 Le Bourget du Lac Cedex
http://www.camptocamp.com

Revision history for this message

Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote on 2013-01-23:

Alright ! Thank you !

camptocamp
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTS

Joël Grand-Guillaume
Division Manager
Business Solutions

+41 21 619 10 28

www.camptocamp.com

lp://staging/~camptocamp/account-financial-tools/account-constrinat-parent-object updated on 2013-01-28

102. By Guewen Baconnier @ Camptocamp on 2013-01-28: [FIX] copy the context to avoid to propagate the 'from_parent_object' key to callers' context
103. By Guewen Baconnier @ Camptocamp on 2013-01-28: [FIX] initialize context to a dict if it is None as we want to get a key
104. By Guewen Baconnier @ Camptocamp on 2013-01-28: [FIX] cut some long lines, remove trailing whitespaces, indentation