Merge lp://qastaging/~camptocamp/openobject-addons/trunk-addons-account-reversal into lp://qastaging/openobject-addons

Being able to reverse an account_move is interesting (rather than the account_reverse module). But we should not have:
  to_be_reversed
  reverse_id

I reject for these reasons, please resubmit when cleaned.

Fabien,

I do not get the reason ? is it because we simply add new fields ? What is not clean?

Thanks

Frederic

Sorry Fabien, but these field are really important => perhaps we should
discuss this tooo.

Regards
*
Luc Maurer*
Directeur et associé

*Camptocamp SA*
PSE A
1015 Lausanne
Suisse

Mobile : +41 79 606 07 73
Direct : +41 21 619 10 12

*Camptocamp France SAS*
48 avenue du Lac du Bourget
73372 Le Bourget du Lac
France

Mobile : +41 79 606 07 73
Direct : +33 479 44 44 95
www.camptocamp.com

On Tue, Nov 8, 2011 at 12:16, Frederic Clementi - Camptocamp.com <
<email address hidden>> wrote:

> Fabien,
>
> I do not get the reason ? is it because we simply add new fields ? What is
> not clean?
>
> Thanks
>
> Frederic
>
>
> --
>
> https://code.launchpad.net/~c2c/openobject-addons/trunk-addons-account-reversal/+merge/75563<https://code.launchpad.net/%7Ec2c/openobject-addons/trunk-addons-account-reversal/+merge/75563>
> Your team Camptocamp is subscribed to branch
> lp:~c2c/openobject-addons/trunk-addons-account-reversal.
>

Hello,

I removed the fields, can you check again please ?

This features (how to know which moves have to be reversed, and the link between a move and its reversal) will be done in a module : lp:~c2c/c2c-addons/account_reversal_improvement)

Thanks

Hello all,

if you have finished your modifications and are ready for another review, please change the status of the merge proposal back to 'needs review' instead of 'work in progress' (as your 'resubmit' review comment is not enough to make the merge proposal appears again in our TODO list).

That's said, i don't have time to review it myself right now ;-), so let me just adjust the status properly.

Thanks,
Quentin

Hello Quentin,

Thanks for your guidance and adjustement of the status. I wasn't aware of that and didn't realize.

Guewen

Hello,

I've done the requested modifications, so why didn't have any news since then ?

Thanks
Guewen

Any news on this please?

Thanks

Frederic

Any news on this please?

Thanks

Frederic

This module is very useful to us, we use it for:
 * End of period FX revaluations and beginning of new period reversals
 * End of period accruals and beginning of new period reversals

Actually the to_be_reversed and reverse_id makes perfect sense to me and we've found it useful for viewing the reversed journals. OpenERP is missing this functionality and it would be great to get this into the core product.

Thanks

James

Hello.
@Fabien

Why we must lost the trace from the account move that generate the reversal?.

At least we must have a comment or something to say, @This move is due to reverse this other one@

I am not agreed about have not the fields, as it was originally (at least you say technically it can bring some problem i cant see ) was fine.

Thanks.

Conceptually, i approve, i will test

Hello Guewen.

Some technical comments that i think needs improvements.

In line 38 of your diff:

+ period_ctx['company_id'] = move.company_id.id

I think you must reset the company_in "only" if company_id is empty.
I mean:

+ period_ctx['company_id'] = not period_ctx.get('comapny_id') and move.company_id.id or period_ctx.get('comapny_id')

+ period_ctx['company_id'] = not period_ctx.get('comapany_id') and move.company_id.id or period_ctx.get('comapany_id')

Sorry for typo mistakes, it should left correct or delete message.

The good one.

+ period_ctx['company_id'] = not period_ctx.get('company_id') and move.company_id.id or period_ctx.get('company_id')

Other One.

In lines 182:

+ move_ids = context['active_ids']

If you use this method from other context this will fail, we avoid as a good proctice iter a dic with Square brackets, we can change:

+ move_ids = context.get('active_ids', False)
+ if not move_ids:
+ raise Controlled message!

The GTK client in some situations doesn't send this key and it fail ugly ;-)

It is happening here too!

187 + cr, uid,
188 + move_ids,
189 + form['date'],
190 + reversal_period_id=period_id,
191 + reversal_journal_id=journal_id,
192 + move_prefix=form['move_prefix'],
193 + move_line_prefix=form['move_line_prefix'],
194 + context=context)

Thanks....

For me it is approved already, im just helping to improve in a technical better way, to avoid unespected Exceptions.