Merge lp:~vauxoo/openobject-addons/account_voucher_refactoring into lp:openobject-addons

[Javier Duran]

[MERGE] account_voucher refactoring

--- Hello we are Checking this merge proposal, can you reject to avoid noise please, we didn't considerate multi currency,

Regards.

Hi,

I think IMHO the action_move_line_create method need refactoring, it is too long and pretty confused, it has over 160 lines also there is a variable that doesn't follow the OpenERP Specific Guidelines (e.g. inv when you talk about voucher). I attached a patch file where this one becomes six smaller methods easier to understand and to use and to improve by others. I hope this can help a little to has a better readable/maintainable code.

Regards,

http://doc.openerp.com/v6.0/contribute/15_guidelines/coding_guidelines_framework.html
http://doc.openerp.com/v6.0/contribute/15_guidelines/coding_guidelines_framework.html#call-your-fish-a-fish
http://doc.openerp.com/v6.0/contribute/15_guidelines/coding_guidelines_framework.html#keep-your-methods-short-simple-when-possible

[Javier Duran (javieredm)]

For example, in Venezuela is a little different to reconcile accounting entries, this improvement allows not having to overwrite all over than 160 lines to change three or four lines, making it simpler and easier.

Regards,

[lp:~vauxoo/openobject-addons/account_voucher_refactoring]

4890. By Javier Duran on 2011-07-26

[REF] Refactoring the first_move_line_create and voucher_move_line_create method
in account_voucher module

[Nhomar - Vauxoo (nhomar)]

Hello.

When this merge proposal will be checked, we need this improvement to continue with pour localization works.

From it we will have a lot of refactor to do.

REgards.

[qdp (OpenERP) (qdp)]

i guess this is a duplicated merge proposal, and i'm rejecting it as you asked in the change description.

thanks

Unmerged revisions

4890. By Javier Duran on 2011-07-26

[REF] Refactoring the first_move_line_create and voucher_move_line_create method
in account_voucher module

4889. By Javier Duran on 2011-07-19

[REF] Refactoring the action_move_line_create method in account_voucher module

=== modified file 'account_voucher/account_voucher.py'
--- account_voucher/account_voucher.py	2011-07-04 13:50:50 +0000
+++ account_voucher/account_voucher.py	2011-07-26 21:17:31 +0000
@@ -621,6 +621,187 @@
             res['account_id'] = account_id
         return {'value':res}
 
+    def move_create(self, cr, uid, voucher_brw):
+        move_obj = self.pool.get('account.move')    
+        seq_obj = self.pool.get('ir.sequence')
+        
+        if voucher_brw.number:
+            name = voucher_brw.number
+        elif voucher_brw.journal_id.sequence_id:
+            name = seq_obj.get_id(cr, uid, voucher_brw.journal_id.sequence_id.id)
+        else:
+            raise osv.except_osv(_('Error !'), _('Please define a sequence on the journal !'))
+        if not voucher_brw.reference:
+            ref = name.replace('/','')
+        else:
+            ref = voucher_brw.reference
+
+        move = {
+            'name': name,
+            'journal_id': voucher_brw.journal_id.id,
+            'narration': voucher_brw.narration,
+            'date': voucher_brw.date,
+            'ref': ref,
+            'period_id': voucher_brw.period_id and voucher_brw.period_id.id or False
+        }
+        move_id = move_obj.create(cr, uid, move)
+        return move_id
+
+
+    def first_move_line_get_item(self, cr, uid, voucher, move_id, company_currency,\
+             current_currency, debit, credit, sign):
+        return {
+            'name': voucher.name or '/',
+            'debit': debit,
+            'credit': credit,
+            'account_id': voucher.account_id.id,
+            'move_id': move_id,
+            'journal_id': voucher.journal_id.id,
+            'period_id': voucher.period_id.id,
+            'partner_id': voucher.partner_id.id,
+            'currency_id': company_currency <> current_currency and  current_currency or False,
+            'amount_currency': company_currency <> current_currency and sign * voucher.amount or 0.0,
+            'date': voucher.date,
+            'date_maturity': voucher.date_due
+        }
+
+        
+    def first_move_line_create(self, cr, uid, voucher_brw, move_id, company_currency, current_currency, context_multi_currency):
+        move_line_obj = self.pool.get('account.move.line')
+        currency_obj = self.pool.get('res.currency')
+
+        debit = 0.0
+        credit = 0.0
+        # TODO: is there any other alternative then the voucher type ??
+        # -for sale, purchase we have but for the payment and receipt we do not have as based on the bank/cash journal we can not know its payment or receipt
+        if voucher_brw.type in ('purchase', 'payment'):
+            credit = currency_obj.compute(cr, uid, current_currency, company_currency, voucher_brw.amount, context=context_multi_currency)
+        elif voucher_brw.type in ('sale', 'receipt'):
+            debit = currency_obj.compute(cr, uid, current_currency, company_currency, voucher_brw.amount, context=context_multi_currency)
+        if debit < 0:
+            credit = -debit
+            debit = 0.0
+        if credit < 0:
+            debit = -credit
+            credit = 0.0
+        sign = debit - credit < 0 and -1 or 1
+        #create the first line of the voucher
+        move_line = self.first_move_line_get_item(cr, uid, voucher_brw, move_id, company_currency, current_currency, debit, credit, sign)
+        move_line_id = move_line_obj.create(cr, uid, move_line)
+        return move_line_id
+
+
+    def voucher_move_line_get_item(self, cr, uid, voucher, line, move_id, company_currency, current_currency):
+        return {
+            'journal_id': voucher.journal_id.id,
+            'period_id': voucher.period_id.id,
+            'name': line.name and line.name or '/',
+            'account_id': line.account_id.id,
+            'move_id': move_id,
+            'partner_id': voucher.partner_id.id,
+            'currency_id': company_currency <> current_currency and current_currency or False,
+            'analytic_account_id': line.account_analytic_id and line.account_analytic_id.id or False,
+            'quantity': 1,
+            'credit': 0.0,
+            'debit': 0.0,
+            'date': voucher.date
+        }
+
+        
+    def voucher_move_line_create(self, cr, uid, voucher_brw, line_total, move_id,\
+            company_currency, current_currency, context_multi_currency, context=None):
+        move_line_obj = self.pool.get('account.move.line')    
+        currency_obj = self.pool.get('res.currency')
+        tot_line = line_total
+        rec_lst_ids = []
+
+        if context is None:
+            context = {}
+        
+        for line in voucher_brw.line_ids:
+            #create one move line per voucher line where amount is not 0.0
+            if not line.amount:
+                continue
+            #we check if the voucher line is fully paid or not and create a move line to balance the payment and initial invoice if needed
+            if line.amount == line.amount_unreconciled:
+                amount = line.move_line_id.amount_residual #residual amount in company currency
+            else:
+                amount = currency_obj.compute(cr, uid, current_currency, company_currency, line.untax_amount or line.amount, context=context_multi_currency)
+            move_line = self.voucher_move_line_get_item(cr, uid, voucher_brw, line, move_id, company_currency, current_currency)
+            if amount < 0:
+                amount = -amount
+                if line.type == 'dr':
+                    line.type = 'cr'
+                else:
+                    line.type = 'dr'
+
+            if (line.type=='dr'):
+                tot_line += amount
+                move_line['debit'] = amount
+            else:
+                tot_line -= amount
+                move_line['credit'] = amount
+
+            if voucher_brw.tax_id and voucher_brw.type in ('sale', 'purchase'):
+                move_line.update({
+                    'account_tax_id': voucher_brw.tax_id.id,
+                })
+            if move_line.get('account_tax_id', False):
+                tax_data = tax_obj.browse(cr, uid, [move_line['account_tax_id']], context=context)[0]
+                if not (tax_data.base_code_id and tax_data.tax_code_id):
+                    raise osv.except_osv(_('No Account Base Code and Account Tax Code!'),_("You have to configure account base code and account tax code on the '%s' tax!") % (tax_data.name))
+            sign = (move_line['debit'] - move_line['credit']) < 0 and -1 or 1
+            move_line['amount_currency'] = company_currency <> current_currency and sign * line.amount or 0.0
+            voucher_line = move_line_obj.create(cr, uid, move_line)
+            if line.move_line_id.id:
+                rec_ids = [voucher_line, line.move_line_id.id]
+                rec_lst_ids.append(rec_ids)
+
+        return (tot_line, rec_lst_ids)
+
+
+    def writeoff_move_line_create(self, cr, uid, voucher_brw, line_total, move_id, name,\
+            company_currency, current_currency, context_multi_currency):
+        move_line_obj = self.pool.get('account.move.line')
+        currency_obj = self.pool.get('res.currency')
+        move_line_id = False
+
+        if not currency_obj.is_zero(cr, uid, voucher_brw.currency_id, line_total):
+            diff = line_total
+            account_id = False
+            write_off_name = ''
+            if voucher_brw.payment_option == 'with_writeoff':
+                account_id = voucher_brw.writeoff_acc_id.id
+                write_off_name = voucher_brw.comment
+            elif voucher_brw.type in ('sale', 'receipt'):
+                account_id = voucher_brw.partner_id.property_account_receivable.id
+            else:
+                account_id = voucher_brw.partner_id.property_account_payable.id
+            move_line = {
+                'name': write_off_name or name,
+                'account_id': account_id,
+                'move_id': move_id,
+                'partner_id': voucher_brw.partner_id.id,
+                'date': voucher_brw.date,
+                'credit': diff > 0 and diff or 0.0,
+                'debit': diff < 0 and -diff or 0.0,
+                #'amount_currency': company_currency <> current_currency and currency_obj.compute(cr, uid, company_currency, current_currency, diff * -1, context=context_multi_currency) or 0.0,
+                #'currency_id': company_currency <> current_currency and current_currency or False,
+            }
+            move_line_id = move_line_obj.create(cr, uid, move_line)
+                
+        return move_line_id
+
+
+    def voucher_reconcile(self, cr, uid, rec_list_ids):
+        move_line_obj = self.pool.get('account.move.line')
+        for rec_ids in rec_list_ids:
+            if len(rec_ids) >= 2:
+                move_line_obj.reconcile_partial(cr, uid, rec_ids)
+                    
+        return True
+        
+
     def action_move_line_create(self, cr, uid, ids, context=None):
 
         def _get_payment_term_lines(term_id, amount):
@@ -635,159 +816,42 @@
         move_line_pool = self.pool.get('account.move.line')
         currency_pool = self.pool.get('res.currency')
         tax_obj = self.pool.get('account.tax')
-        seq_obj = self.pool.get('ir.sequence')
-        for inv in self.browse(cr, uid, ids, context=context):
-            if inv.move_id:
+        
+        for voucher in self.browse(cr, uid, ids, context=context):
+            if voucher.move_id:
                 continue
             context_multi_currency = context.copy()
-            context_multi_currency.update({'date': inv.date})
-
-            if inv.number:
-                name = inv.number
-            elif inv.journal_id.sequence_id:
-                name = seq_obj.get_id(cr, uid, inv.journal_id.sequence_id.id)
-            else:
-                raise osv.except_osv(_('Error !'), _('Please define a sequence on the journal !'))
-            if not inv.reference:
-                ref = name.replace('/','')
-            else:
-                ref = inv.reference
-
-            move = {
-                'name': name,
-                'journal_id': inv.journal_id.id,
-                'narration': inv.narration,
-                'date': inv.date,
-                'ref': ref,
-                'period_id': inv.period_id and inv.period_id.id or False
-            }
-            move_id = move_pool.create(cr, uid, move)
-
-            #create the first line manually
-            company_currency = inv.journal_id.company_id.currency_id.id
-            current_currency = inv.currency_id.id
-            debit = 0.0
-            credit = 0.0
-            # TODO: is there any other alternative then the voucher type ??
-            # -for sale, purchase we have but for the payment and receipt we do not have as based on the bank/cash journal we can not know its payment or receipt
-            if inv.type in ('purchase', 'payment'):
-                credit = currency_pool.compute(cr, uid, current_currency, company_currency, inv.amount, context=context_multi_currency)
-            elif inv.type in ('sale', 'receipt'):
-                debit = currency_pool.compute(cr, uid, current_currency, company_currency, inv.amount, context=context_multi_currency)
-            if debit < 0:
-                credit = -debit
-                debit = 0.0
-            if credit < 0:
-                debit = -credit
-                credit = 0.0
-            sign = debit - credit < 0 and -1 or 1
-            #create the first line of the voucher
-            move_line = {
-                'name': inv.name or '/',
-                'debit': debit,
-                'credit': credit,
-                'account_id': inv.account_id.id,
-                'move_id': move_id,
-                'journal_id': inv.journal_id.id,
-                'period_id': inv.period_id.id,
-                'partner_id': inv.partner_id.id,
-                'currency_id': company_currency <> current_currency and  current_currency or False,
-                'amount_currency': company_currency <> current_currency and sign * inv.amount or 0.0,
-                'date': inv.date,
-                'date_maturity': inv.date_due
-            }
-            move_line_pool.create(cr, uid, move_line)
+            context_multi_currency.update({'date': voucher.date})
+            company_currency = voucher.journal_id.company_id.currency_id.id
+            current_currency = voucher.currency_id.id
+            #create the move
+            move_id = self.move_create(cr, uid, voucher)
+            name = move_pool.browse(cr, uid, move_id, context=context).name
+            #create the first line manually            
+            move_line_id = self.first_move_line_create(cr, uid, voucher, move_id, company_currency, current_currency, context_multi_currency)
+            mov_line_brw = move_line_pool.browse(cr, uid, move_line_id, context=context)
+            
             rec_list_ids = []
-            line_total = debit - credit
-            if inv.type == 'sale':
-                line_total = line_total - currency_pool.compute(cr, uid, inv.currency_id.id, company_currency, inv.tax_amount, context=context_multi_currency)
-            elif inv.type == 'purchase':
-                line_total = line_total + currency_pool.compute(cr, uid, inv.currency_id.id, company_currency, inv.tax_amount, context=context_multi_currency)
-
-            for line in inv.line_ids:
-                #create one move line per voucher line where amount is not 0.0
-                if not line.amount:
-                    continue
-                #we check if the voucher line is fully paid or not and create a move line to balance the payment and initial invoice if needed
-                if line.amount == line.amount_unreconciled:
-                    amount = line.move_line_id.amount_residual #residual amount in company currency
-                else:
-                    amount = currency_pool.compute(cr, uid, current_currency, company_currency, line.untax_amount or line.amount, context=context_multi_currency)
-                move_line = {
-                    'journal_id': inv.journal_id.id,
-                    'period_id': inv.period_id.id,
-                    'name': line.name or '/',
-                    'account_id': line.account_id.id,
-                    'move_id': move_id,
-                    'partner_id': inv.partner_id.id,
-                    'currency_id': company_currency <> current_currency and current_currency or False,
-                    'analytic_account_id': line.account_analytic_id and line.account_analytic_id.id or False,
-                    'quantity': 1,
-                    'credit': 0.0,
-                    'debit': 0.0,
-                    'date': inv.date
-                }
-                if amount < 0:
-                    amount = -amount
-                    if line.type == 'dr':
-                        line.type = 'cr'
-                    else:
-                        line.type = 'dr'
-                if (line.type=='dr'):
-                    line_total += amount
-                    move_line['debit'] = amount
-                else:
-                    line_total -= amount
-                    move_line['credit'] = amount
-
-                if inv.tax_id and inv.type in ('sale', 'purchase'):
-                    move_line.update({
-                        'account_tax_id': inv.tax_id.id,
-                    })
-                if move_line.get('account_tax_id', False):
-                    tax_data = tax_obj.browse(cr, uid, [move_line['account_tax_id']], context=context)[0]
-                    if not (tax_data.base_code_id and tax_data.tax_code_id):
-                        raise osv.except_osv(_('No Account Base Code and Account Tax Code!'),_("You have to configure account base code and account tax code on the '%s' tax!") % (tax_data.name))
-                sign = (move_line['debit'] - move_line['credit']) < 0 and -1 or 1
-                move_line['amount_currency'] = company_currency <> current_currency and sign * line.amount or 0.0
-                voucher_line = move_line_pool.create(cr, uid, move_line)
-                if line.move_line_id.id:
-                    rec_ids = [voucher_line, line.move_line_id.id]
-                    rec_list_ids.append(rec_ids)
-
-            if not currency_pool.is_zero(cr, uid, inv.currency_id, line_total):
-                diff = line_total
-                account_id = False
-                write_off_name = ''
-                if inv.payment_option == 'with_writeoff':
-                    account_id = inv.writeoff_acc_id.id
-                    write_off_name = inv.comment
-                elif inv.type in ('sale', 'receipt'):
-                    account_id = inv.partner_id.property_account_receivable.id
-                else:
-                    account_id = inv.partner_id.property_account_payable.id
-                move_line = {
-                    'name': write_off_name or name,
-                    'account_id': account_id,
-                    'move_id': move_id,
-                    'partner_id': inv.partner_id.id,
-                    'date': inv.date,
-                    'credit': diff > 0 and diff or 0.0,
-                    'debit': diff < 0 and -diff or 0.0,
-                    #'amount_currency': company_currency <> current_currency and currency_pool.compute(cr, uid, company_currency, current_currency, diff * -1, context=context_multi_currency) or 0.0,
-                    #'currency_id': company_currency <> current_currency and current_currency or False,
-                }
-                move_line_pool.create(cr, uid, move_line)
-            self.write(cr, uid, [inv.id], {
+            line_total = mov_line_brw.debit - mov_line_brw.credit
+            if voucher.type == 'sale':
+                line_total = line_total - currency_pool.compute(cr, uid, voucher.currency_id.id, company_currency, voucher.tax_amount, context=context_multi_currency)
+            elif voucher.type == 'purchase':
+                line_total = line_total + currency_pool.compute(cr, uid, voucher.currency_id.id, company_currency, voucher.tax_amount, context=context_multi_currency)
+
+            #create one move line per voucher line where amount is not 0.0
+            line_total, rec_list_ids = self.voucher_move_line_create(cr, uid, voucher, line_total, move_id, company_currency, current_currency, context_multi_currency, context=context)
+            #create the writeoff line if needed
+            ml_writeoff_id = self.writeoff_move_line_create(cr, uid, voucher, line_total, move_id, name, company_currency, current_currency, context_multi_currency)                
+            self.write(cr, uid, [voucher.id], {
                 'move_id': move_id,
                 'state': 'posted',
                 'number': name,
             })
             if inv.journal_id.entry_posted:
                 move_pool.post(cr, uid, [move_id], context={})
-            for rec_ids in rec_list_ids:
-                if len(rec_ids) >= 2:
-                    move_line_pool.reconcile_partial(cr, uid, rec_ids)
+            #reconcile move line
+            self.voucher_reconcile(cr, uid, rec_list_ids)
+
         return True
 
     def copy(self, cr, uid, id, default={}, context=None):