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

Proposed by Guewen Baconnier @ Camptocamp
Status: Needs review
Proposed branch: lp:~camptocamp/openobject-addons/trunk-addons-account-reversal
Merge into: lp:openobject-addons
Diff against target: 254 lines (+212/-0)
5 files modified
account/__openerp__.py (+1/-0)
account/account.py (+79/-0)
account/wizard/__init__.py (+1/-0)
account/wizard/account_move_reverse.py (+85/-0)
account/wizard/account_move_reverse_view.xml (+46/-0)
To merge this branch: bzr merge lp:~camptocamp/openobject-addons/trunk-addons-account-reversal
Reviewer Review Type Date Requested Status
Guewen Baconnier @ Camptocamp (community) Needs Resubmitting
Fabien (Open ERP) Disapprove
Review via email: mp+75563@code.launchpad.net

Description of the change

Hello,

This is the integration of a reversal function in the account module.

A flag is added on the account moves to know that they have to be reversed or are reversed.
A button in the search view let filter them.

To reverse entries, there is a wizard on the account moves which act on the selected moves.
Thanks to consider this merge proposal.

Best Regards
Guewen

To post a comment you must log in.
Revision history for this message
Fabien (Open ERP) (fp-tinyerp) wrote :

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.

review: Disapprove
Revision history for this message
Frederic Clementi - Camptocamp (frederic-clementi) wrote :

Fabien,

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

Thanks

Frederic

Revision history for this message
Luc Maurer @ Camptocamp (lmaurer-c2c) wrote :

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.
>

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

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

review: Needs Resubmitting
Revision history for this message
qdp (OpenERP) (qdp) wrote :

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

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hello Quentin,

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

Guewen

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hello,

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

Thanks
Guewen

Revision history for this message
Frederic Clementi - Camptocamp (frederic-clementi) wrote :

Any news on this please?

Thanks

Frederic

Revision history for this message
Frederic Clementi - Camptocamp (frederic-clementi) wrote :

Any news on this please?

Thanks

Frederic

Revision history for this message
James Jesudason (jamesj) wrote :

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

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

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

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

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')

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

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

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

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')

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

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.

Unmerged revisions

5001. By Guewen Baconnier @ Camptocamp <email address hidden> on 2011-11-24

[FIX] According to Fabien's request, removed the reversal_id and to_be_reverse fields. But splitted also the create reversal method to allow us to add these fields in a module.

5000. By Guewen Baconnier @ Camptocamp <email address hidden> on 2011-11-24

[MRG] from upstream

4999. By Guewen Baconnier @ Camptocamp <email address hidden> on 2011-09-29

[IMP] account : account.period method find, add company_id filter (from Quentin's Voucher Branch)

4998. By Guewen Baconnier @ Camptocamp <email address hidden> on 2011-09-29

[MRG] from trunk

4997. By Guewen Baconnier @ Camptocamp <email address hidden> on 2011-09-27

[MRG] merge from trunk addons

4996. By Guewen Baconnier @ Camptocamp <email address hidden> on 2011-09-27

[FIX] removed hooks

4995. By Guewen Baconnier @ Camptocamp <email address hidden> on 2011-09-15

[ADD] account: reversal feature

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account/__openerp__.py'
2--- account/__openerp__.py 2011-11-23 14:16:16 +0000
3+++ account/__openerp__.py 2011-11-24 15:19:29 +0000
4@@ -98,6 +98,7 @@
5 'wizard/account_reconcile_view.xml',
6 'wizard/account_reconcile_partner_process_view.xml',
7 'wizard/account_automatic_reconcile_view.xml',
8+ 'wizard/account_move_reverse_view.xml',
9 'wizard/account_financial_report_view.xml',
10 'project/wizard/project_account_analytic_line_view.xml',
11 'account_end_fy.xml',
12
13=== modified file 'account/account.py'
14--- account/account.py 2011-11-23 14:16:16 +0000
15+++ account/account.py 2011-11-24 15:19:29 +0000
16@@ -1582,6 +1582,85 @@
17 valid_moves = [move.id for move in valid_moves]
18 return len(valid_moves) > 0 and valid_moves or False
19
20+ def _move_reversal(self, cr, uid, move, reversal_date, reversal_period_id=False, reversal_journal_id=False,
21+ move_prefix=False, move_line_prefix=False, context=None):
22+ """
23+ Create the reversal of a move
24+
25+ @param move: browse instance of the move to reverse
26+ @param reversal_date: when the reversal must be input
27+ @param reversal_period_id: facultative period to write on the move (use the period of the date if empty
28+ @param reversal_journal_id: facultative journal in which create the move
29+ @param move_prefix: prefix for the move's name
30+ @param move_line_prefix: prefix for the move line's names
31+
32+ @return: Returns the id of the created reversal move
33+ """
34+ if context is None: context = {}
35+ move_line_obj = self.pool.get('account.move.line')
36+ period_obj = self.pool.get('account.period')
37+ period_ctx = context.copy()
38+ period_ctx['company_id'] = move.company_id.id
39+ if not reversal_period_id:
40+ reversal_period_id = period_obj.find(cr, uid, reversal_date, context=period_ctx)[0]
41+
42+ if not reversal_journal_id:
43+ reversal_journal_id = move.journal_id.id
44+ reversal_ctx = context.copy()
45+ reversal_ctx['reversal'] = True
46+ reversal_ref = ''.join([x for x in [move_prefix, move.ref] if x])
47+ reversal_move_id = self.copy(cr, uid, move.id,
48+ default={
49+ 'date': reversal_date,
50+ 'period_id': reversal_period_id,
51+ 'ref': reversal_ref,
52+ 'journal_id': reversal_journal_id,
53+ },
54+ context=reversal_ctx)
55+
56+ reversal_move = self.browse(cr, uid, reversal_move_id, context=context)
57+ for reversal_move_line in reversal_move.line_id:
58+ reversal_ml_name = ' '.join([x for x in [move_line_prefix, reversal_move_line.name] if x])
59+ move_line_obj.write(cr, uid, [reversal_move_line.id],
60+ {
61+ 'debit': reversal_move_line.credit,
62+ 'credit': reversal_move_line.debit,
63+ 'amount_currency': reversal_move_line.amount_currency * -1,
64+ 'name': reversal_ml_name,
65+ },
66+ context=reversal_ctx,
67+ check=True, update_check=True
68+ )
69+
70+ self.validate(cr, uid, [reversal_move_id], context=reversal_ctx)
71+ return reversal_move_id
72+
73+ def create_reversals(self, cr, uid, ids, reversal_date, reversal_period_id=False, reversal_journal_id=False,
74+ move_prefix=False, move_line_prefix=False, context=None):
75+ """
76+ Create the reversal of one or multiple moves
77+
78+ @param reversal_date: when the reversal must be input
79+ @param reversal_period_id: facultative period to write on the move (use the period of the date if empty
80+ @param reversal_journal_id: facultative journal in which create the move
81+ @param move_prefix: prefix for the move's name
82+ @param move_line_prefix: prefix for the move line's names
83+
84+ @return: Returns a list of ids of the created reversal moves
85+ """
86+ if isinstance(ids, (int, long)):
87+ ids = [ids]
88+
89+ reversed_move_ids = []
90+ for src_move in self.browse(cr, uid, ids, context=context):
91+ reversal_move_id = self._move_reversal(cr, uid, src_move, reversal_date, reversal_period_id=reversal_period_id,
92+ reversal_journal_id=reversal_journal_id, move_prefix=move_prefix,
93+ move_line_prefix=move_line_prefix, context=context)
94+ if reversal_move_id:
95+ reversed_move_ids.append(reversal_move_id)
96+
97+ return reversed_move_ids
98+
99 account_move()
100
101 class account_move_reconcile(osv.osv):
102
103=== modified file 'account/wizard/__init__.py'
104--- account/wizard/__init__.py 2011-09-29 11:08:56 +0000
105+++ account/wizard/__init__.py 2011-11-24 15:19:29 +0000
106@@ -43,6 +43,7 @@
107 import account_fiscalyear_close_state
108 import account_vat
109 import account_open_closed_fiscalyear
110+import account_move_reverse
111
112 import account_invoice_state
113 import account_chart
114
115=== added file 'account/wizard/account_move_reverse.py'
116--- account/wizard/account_move_reverse.py 1970-01-01 00:00:00 +0000
117+++ account/wizard/account_move_reverse.py 2011-11-24 15:19:29 +0000
118@@ -0,0 +1,85 @@
119+# -*- coding: utf-8 -*-
120+##############################################################################
121+#
122+# OpenERP, Open Source Management Solution
123+# Copyright (C) 2004-2010 Tiny SPRL (<http://tiny.be>).
124+#
125+# This program is free software: you can redistribute it and/or modify
126+# it under the terms of the GNU Affero General Public License as
127+# published by the Free Software Foundation, either version 3 of the
128+# License, or (at your option) any later version.
129+#
130+# This program is distributed in the hope that it will be useful,
131+# but WITHOUT ANY WARRANTY; without even the implied warranty of
132+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
133+# GNU Affero General Public License for more details.
134+#
135+# You should have received a copy of the GNU Affero General Public License
136+# along with this program. If not, see <http://www.gnu.org/licenses/>.
137+#
138+##############################################################################
139+
140+from osv import osv, fields
141+from tools.translate import _
142+
143+
144+class account_move_reversal(osv.osv_memory):
145+ _name = "account.move.reverse"
146+ _description = "Create reversal of account moves"
147+
148+ _columns = {
149+ 'date': fields.date('Reversal Date', required=True, help="Enter the date of the reversal account entries. By default, OpenERP proposes the first day of the next period."),
150+ 'period_id': fields.many2one('account.period', 'Reversal Period', help="If empty, take the period of the date."),
151+ 'journal_id': fields.many2one('account.journal', 'Reversal Journal', help='If empty, uses the journal of the journal entry to be reversed.'),
152+ 'move_prefix': fields.char('Entries Ref. Prefix', size=32, help="Prefix that will be added to the 'Ref' of the journal entry to be reversed to create the 'Ref' of the reversal journal entry (no space added after the prefix)."),
153+ 'move_line_prefix': fields.char('Items Name Prefix', size=32, help="Prefix that will be added to the name of the journal item to be reversed to create the name of the reversal journal item (a space is added after the prefix)."),
154+ }
155+
156+ def _next_period_first_date(self, cr, uid, context=None):
157+ if context is None:
158+ context = {}
159+ period_obj = self.pool.get('account.period')
160+ current_period_id = period_obj.find(cr, uid, context=context)[0]
161+ current_period = period_obj.browse(cr, uid, current_period_id, context=context)
162+ next_period_id = period_obj.next(cr, uid, current_period, 1, context=context)
163+ next_period = period_obj.browse(cr, uid, next_period_id, context=context)
164+ return next_period.date_start
165+
166+ _defaults = {
167+ 'date': _next_period_first_date,
168+ 'move_line_prefix': lambda *a: 'REV -',
169+ }
170+
171+ def action_reverse(self, cr, uid, ids, context=None):
172+ if context is None:
173+ context = {}
174+ form = self.read(cr, uid, ids, context=context)[0]
175+
176+ action = {'type': 'ir.actions.act_window_close'}
177+
178+ if context.get('active_ids', False):
179+ mod_obj = self.pool.get('ir.model.data')
180+ act_obj = self.pool.get('ir.actions.act_window')
181+ move_obj = self.pool.get('account.move')
182+ move_ids = context['active_ids']
183+
184+ period_id = form.get('period_id', False) and form['period_id'][0] or False
185+ journal_id = form.get('journal_id', False) and form['journal_id'][0] or False
186+ reversed_move_ids = move_obj.create_reversals(
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)
195+
196+ action_ref = mod_obj.get_object_reference(cr, uid, 'account', 'action_move_journal_line')
197+ action_id = action_ref and action_ref[1] or False
198+ action = act_obj.read(cr, uid, [action_id], context=context)[0]
199+ action['domain'] = str([('id', 'in', reversed_move_ids)])
200+ action['name'] = _('Reversal Entries')
201+ return action
202+
203+account_move_reversal()
204
205=== added file 'account/wizard/account_move_reverse_view.xml'
206--- account/wizard/account_move_reverse_view.xml 1970-01-01 00:00:00 +0000
207+++ account/wizard/account_move_reverse_view.xml 2011-11-24 15:19:29 +0000
208@@ -0,0 +1,46 @@
209+<?xml version="1.0" encoding="utf-8"?>
210+<openerp>
211+ <data>
212+
213+ <record id="view_account_move_reverse" model="ir.ui.view">
214+ <field name="name">account.move.reverse.form</field>
215+ <field name="model">account.move.reverse</field>
216+ <field name="type">form</field>
217+ <field name="arch" type="xml">
218+ <form string="Create reversal journal entries">
219+ <field name="date"/>
220+ <field name="period_id"/>
221+ <field name="journal_id"/>
222+ <newline/>
223+ <field name="move_prefix" />
224+ <field name="move_line_prefix" />
225+
226+ <separator string="" colspan="4" />
227+ <group colspan="4" col="6">
228+ <button icon="gtk-cancel" special="cancel" string="Cancel"/>
229+ <button icon="gtk-ok" string="Reverse Entries" name="action_reverse" type="object"/>
230+ </group>
231+ </form>
232+ </field>
233+ </record>
234+
235+ <record id="act_account_move_reverse" model="ir.actions.act_window">
236+ <field name="name">Reverse Entries</field>
237+ <field name="res_model">account.move.reverse</field>
238+ <field name="view_type">form</field>
239+ <field name="view_mode">form</field>
240+ <field name="view_id" ref="view_account_move_reverse"/>
241+ <field name="target">new</field>
242+ <field name="help">This will create reversal for all selected entries whether checked "to be reversed" or not.</field>
243+ </record>
244+
245+ <record id="ir_account_move_reverse" model="ir.values">
246+ <field name="name">Reverse Entries</field>
247+ <field name="key2">client_action_multi</field>
248+ <field name="model">account.move</field>
249+ <field name="value" eval="'ir.actions.act_window,%d'%act_account_move_reverse" />
250+ <field name="object" eval="True" />
251+ </record>
252+
253+ </data>
254+</openerp>

Subscribers

People subscribed via source and target branches

to all changes: