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

[Guewen Baconnier @ Camptocamp]

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

[Fabien (Open ERP) (fp-tinyerp)]

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

[Frederic Clementi - Camptocamp (frederic-clementi)]

Fabien,

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

Thanks

Frederic

[Luc Maurer @ Camptocamp (lmaurer-c2c)]

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 : [phone]
Direct : [phone]

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

Mobile : [phone]
Direct : [phone]
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.
>

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

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

[qdp (OpenERP) (qdp)]

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

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hello Quentin,

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

Guewen

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Hello,

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

Thanks
Guewen

[Frederic Clementi - Camptocamp (frederic-clementi)]

Any news on this please?

Thanks

Frederic

[Frederic Clementi - Camptocamp (frederic-clementi)]

Any news on this please?

Thanks

Frederic

[James Jesudason (jamesj)]

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

[Nhomar - Vauxoo (nhomar)]

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

[Nhomar - Vauxoo (nhomar)]

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

[Nhomar - Vauxoo (nhomar)]

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

[Nhomar - Vauxoo (nhomar)]

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

[Nhomar - Vauxoo (nhomar)]

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

=== modified file 'account/__openerp__.py'
--- account/__openerp__.py	2011-11-23 14:16:16 +0000
+++ account/__openerp__.py	2011-11-24 15:19:29 +0000
@@ -98,6 +98,7 @@
         'wizard/account_reconcile_view.xml',
         'wizard/account_reconcile_partner_process_view.xml',
         'wizard/account_automatic_reconcile_view.xml',
+        'wizard/account_move_reverse_view.xml',
         'wizard/account_financial_report_view.xml',
         'project/wizard/project_account_analytic_line_view.xml',
         'account_end_fy.xml',
 
=== modified file 'account/account.py'
--- account/account.py	2011-11-23 14:16:16 +0000
+++ account/account.py	2011-11-24 15:19:29 +0000
@@ -1582,6 +1582,85 @@
         valid_moves = [move.id for move in valid_moves]
         return len(valid_moves) > 0 and valid_moves or False
 
+    def _move_reversal(self, cr, uid, move, reversal_date, reversal_period_id=False, reversal_journal_id=False,
+                       move_prefix=False, move_line_prefix=False, context=None):
+        """
+        Create the reversal of a move
+
+        @param move: browse instance of the move to reverse
+        @param reversal_date: when the reversal must be input
+        @param reversal_period_id: facultative period to write on the move (use the period of the date if empty
+        @param reversal_journal_id: facultative journal in which create the move
+        @param move_prefix: prefix for the move's name
+        @param move_line_prefix: prefix for the move line's names
+
+        @return: Returns the id of the created reversal move
+        """
+        if context is None: context = {}
+        move_line_obj = self.pool.get('account.move.line')
+        period_obj = self.pool.get('account.period')
+        period_ctx = context.copy()
+        period_ctx['company_id'] = move.company_id.id
+        if not reversal_period_id:
+            reversal_period_id = period_obj.find(cr, uid, reversal_date, context=period_ctx)[0]
+
+        if not reversal_journal_id:
+            reversal_journal_id = move.journal_id.id
+        reversal_ctx = context.copy()
+        reversal_ctx['reversal'] = True
+        reversal_ref = ''.join([x for x in [move_prefix, move.ref] if x])
+        reversal_move_id = self.copy(cr, uid, move.id,
+                                     default={
+                                         'date': reversal_date,
+                                         'period_id': reversal_period_id,
+                                         'ref': reversal_ref,
+                                         'journal_id': reversal_journal_id,
+                                     },
+                                     context=reversal_ctx)
+
+        reversal_move = self.browse(cr, uid, reversal_move_id, context=context)
+        for reversal_move_line in reversal_move.line_id:
+            reversal_ml_name = ' '.join([x for x in [move_line_prefix, reversal_move_line.name] if x])
+            move_line_obj.write(cr, uid, [reversal_move_line.id],
+                                {
+                                    'debit': reversal_move_line.credit,
+                                    'credit': reversal_move_line.debit,
+                                    'amount_currency': reversal_move_line.amount_currency * -1,
+                                    'name': reversal_ml_name,
+                                },
+                                context=reversal_ctx,
+                                check=True, update_check=True
+                                )
+
+        self.validate(cr, uid, [reversal_move_id], context=reversal_ctx)
+        return reversal_move_id
+
+    def create_reversals(self, cr, uid, ids, reversal_date, reversal_period_id=False, reversal_journal_id=False,
+                move_prefix=False, move_line_prefix=False, context=None):
+        """
+        Create the reversal of one or  multiple moves
+
+        @param reversal_date: when the reversal must be input
+        @param reversal_period_id: facultative period to write on the move (use the period of the date if empty
+        @param reversal_journal_id: facultative journal in which create the move
+        @param move_prefix: prefix for the move's name
+        @param move_line_prefix: prefix for the move line's names
+
+        @return: Returns a list of ids of the created reversal moves
+        """
+        if isinstance(ids, (int, long)):
+            ids = [ids]
+
+        reversed_move_ids = []
+        for src_move in self.browse(cr, uid, ids, context=context):
+            reversal_move_id = self._move_reversal(cr, uid, src_move, reversal_date, reversal_period_id=reversal_period_id,
+                                                   reversal_journal_id=reversal_journal_id, move_prefix=move_prefix,
+                                                   move_line_prefix=move_line_prefix, context=context)
+            if reversal_move_id:
+                reversed_move_ids.append(reversal_move_id)
+
+        return reversed_move_ids
+
 account_move()
 
 class account_move_reconcile(osv.osv):
 
=== modified file 'account/wizard/__init__.py'
--- account/wizard/__init__.py	2011-09-29 11:08:56 +0000
+++ account/wizard/__init__.py	2011-11-24 15:19:29 +0000
@@ -43,6 +43,7 @@
 import account_fiscalyear_close_state
 import account_vat
 import account_open_closed_fiscalyear
+import account_move_reverse
 
 import account_invoice_state
 import account_chart
 
=== added file 'account/wizard/account_move_reverse.py'
--- account/wizard/account_move_reverse.py	1970-01-01 00:00:00 +0000
+++ account/wizard/account_move_reverse.py	2011-11-24 15:19:29 +0000
@@ -0,0 +1,85 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2004-2010 Tiny SPRL (<http://tiny.be>).
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+from osv import osv, fields
+from tools.translate import _
+
+
+class account_move_reversal(osv.osv_memory):
+    _name = "account.move.reverse"
+    _description = "Create reversal of account moves"
+
+    _columns = {
+        '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."),
+        'period_id': fields.many2one('account.period', 'Reversal Period', help="If empty, take the period of the date."),
+        'journal_id': fields.many2one('account.journal', 'Reversal Journal', help='If empty, uses the journal of the journal entry to be reversed.'),
+        '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)."),
+        '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)."),
+        }
+
+    def _next_period_first_date(self, cr, uid, context=None):
+        if context is None:
+            context = {}
+        period_obj = self.pool.get('account.period')
+        current_period_id = period_obj.find(cr, uid, context=context)[0]
+        current_period = period_obj.browse(cr, uid, current_period_id, context=context)
+        next_period_id = period_obj.next(cr, uid, current_period, 1, context=context)
+        next_period = period_obj.browse(cr, uid, next_period_id, context=context)
+        return next_period.date_start
+
+    _defaults = {
+        'date': _next_period_first_date,
+        'move_line_prefix': lambda *a: 'REV -',
+        }
+
+    def action_reverse(self, cr, uid, ids, context=None):
+        if context is None:
+            context = {}
+        form = self.read(cr, uid, ids, context=context)[0]
+
+        action = {'type': 'ir.actions.act_window_close'}
+
+        if context.get('active_ids', False):
+            mod_obj = self.pool.get('ir.model.data')
+            act_obj = self.pool.get('ir.actions.act_window')
+            move_obj = self.pool.get('account.move')
+            move_ids = context['active_ids']
+
+            period_id = form.get('period_id', False) and form['period_id'][0] or False
+            journal_id = form.get('journal_id', False) and form['journal_id'][0] or False
+            reversed_move_ids = move_obj.create_reversals(
+                cr, uid,
+                move_ids,
+                form['date'],
+                reversal_period_id=period_id,
+                reversal_journal_id=journal_id,
+                move_prefix=form['move_prefix'],
+                move_line_prefix=form['move_line_prefix'],
+                context=context)
+
+            action_ref = mod_obj.get_object_reference(cr, uid, 'account', 'action_move_journal_line')
+            action_id = action_ref and action_ref[1] or False
+            action = act_obj.read(cr, uid, [action_id], context=context)[0]
+            action['domain'] = str([('id', 'in', reversed_move_ids)])
+            action['name'] = _('Reversal Entries')
+        return action
+
+account_move_reversal()
 
=== added file 'account/wizard/account_move_reverse_view.xml'
--- account/wizard/account_move_reverse_view.xml	1970-01-01 00:00:00 +0000
+++ account/wizard/account_move_reverse_view.xml	2011-11-24 15:19:29 +0000
@@ -0,0 +1,46 @@
+<?xml version="1.0" encoding="utf-8"?>
+<openerp>
+    <data>
+
+        <record id="view_account_move_reverse" model="ir.ui.view">
+            <field name="name">account.move.reverse.form</field>
+            <field name="model">account.move.reverse</field>
+            <field name="type">form</field>
+            <field name="arch" type="xml">
+                <form string="Create reversal journal entries">
+                    <field name="date"/>
+                    <field name="period_id"/>
+                    <field name="journal_id"/>
+                    <newline/>
+                    <field name="move_prefix" />
+                    <field name="move_line_prefix" />
+                    
+                    <separator string="" colspan="4" />
+                    <group colspan="4" col="6">
+                        <button icon="gtk-cancel" special="cancel" string="Cancel"/>
+                        <button icon="gtk-ok" string="Reverse Entries" name="action_reverse" type="object"/>
+                   </group>
+                </form>
+            </field>
+        </record>
+
+        <record id="act_account_move_reverse" model="ir.actions.act_window">
+            <field name="name">Reverse Entries</field>
+            <field name="res_model">account.move.reverse</field>
+            <field name="view_type">form</field>
+            <field name="view_mode">form</field>
+            <field name="view_id" ref="view_account_move_reverse"/>
+            <field name="target">new</field>
+            <field name="help">This will create reversal for all selected entries whether checked "to be reversed" or not.</field>
+        </record>
+
+        <record id="ir_account_move_reverse" model="ir.values">
+            <field name="name">Reverse Entries</field>
+            <field name="key2">client_action_multi</field>
+            <field name="model">account.move</field>
+            <field name="value" eval="'ir.actions.act_window,%d'%act_account_move_reverse" />
+            <field name="object" eval="True" />
+        </record>
+
+    </data>
+</openerp>