Merge lp:~pedro.baeza/account-payment/7.0-account_payment_extension_store into lp:~account-payment-team/account-payment/7.0

Proposed by Pedro Manuel Baeza on 2014-01-28
Status: Merged
Merged at revision: 111
Proposed branch: lp:~pedro.baeza/account-payment/7.0-account_payment_extension_store
Merge into: lp:~account-payment-team/account-payment/7.0
Diff against target: 82 lines (+36/-22)
1 file modified
account_payment_extension/account_move_line.py (+36/-22)
To merge this branch: bzr merge lp:~pedro.baeza/account-payment/7.0-account_payment_extension_store
Reviewer Review Type Date Requested Status
Guewen Baconnier @ Camptocamp code review Approve on 2014-03-21
Yannick Vaucher @ Camptocamp code review 2014-01-28 Approve on 2014-03-17
Lorenzo Battistini (community) code review Approve on 2014-02-22
Review via email: mp+203594@code.launchpad.net

Description of the change

Methods for storing correctly the value amount_to_pay, that in 6.1 is stored, but not recomputed whenever its conditions changes, and for 7.0 is directly not stored, impacting on the performance on some operations.

To post a comment you must log in.
Lorenzo Battistini (elbati) wrote :

Since 'amount_to_pay' is stored, is '_to_pay_search' still needed?

review: Needs Information
108. By Pedro Manuel Baeza on 2014-01-30

[IMP] account_payment_extension: method _to_pay_search removed.

Pedro Manuel Baeza (pedro.baeza) wrote :

Indeed, it is not needed anymore.

I have remove it.

Thanks for the review.

Regards.

review: Approve (code review)
review: Approve (code review)

    result = {}
    line_obj = self.pool['payment.line']
    for line in line_obj.browse(cr, uid, ids, context=context):
        result[line.move_line_id.id] = True
        result[line.payment_move_id.id] = True
    return result.keys()

is a somewhat awkward manner to do:

    result = set()
    line_obj = self.pool['payment.line']
    for line in line_obj.browse(cr, uid, ids, context=context):
        result.add(line.move_line_id.id)
        result.add(line.payment_move_id.id)
    return list(result)

On line 72 of the diff:

    'account.move.line': (lambda self, cr, uid, ids, c={}: ids,

There is a mutable default argument, it should be replaced by None

Would you agree to change that?
Thanks

review: Needs Fixing
109. By Pedro Manuel Baeza on 2014-03-21

[IMP] account_payment_extension: dicts replaced by sets on store functions an mutable argument removed.

Pedro Manuel Baeza (pedro.baeza) wrote :

Guewen, thanks for your comments. They are done.

Regards.

Thanks!

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_payment_extension/account_move_line.py'
2--- account_payment_extension/account_move_line.py 2013-11-10 17:00:26 +0000
3+++ account_payment_extension/account_move_line.py 2014-03-21 11:20:36 +0000
4@@ -120,27 +120,6 @@
5 result[move_id] = debt
6 return result
7
8- def _to_pay_search(self, cr, uid, obj, name, args, context={}):
9- if not len(args):
10- return []
11- currency = self.pool.get('res.users').browse(
12- cr, uid, uid, context).company_id.currency_id
13-
14- # For searching we first discard reconciled moves
15- # because the filter is fast and discards most records quickly.
16- ids = self.pool.get('account.move.line').search(
17- cr, uid, [('reconcile_id', '=', False)], context=context)
18- records = self.pool.get('account.move.line').read(
19- cr, uid, ids, ['id', 'amount_to_pay'], context)
20- ids = []
21- for record in records:
22- if not self.pool.get('res.currency').is_zero(
23- cr, uid, currency, record['amount_to_pay']):
24- ids.append(record['id'])
25- if not ids:
26- return [('id', '=', False)]
27- return [('id', 'in', ids)]
28-
29 def _payment_type_get(self, cr, uid, ids, field_name, arg, context={}):
30 result = {}
31 invoice_obj = self.pool.get('account.invoice')
32@@ -184,6 +163,33 @@
33 result = [('id', 'in', [x[0] for x in res])]
34 return result
35
36+ def _get_move_lines(self, cr, uid, ids, context=None):
37+ result = set()
38+ line_obj = self.pool['payment.line']
39+ for line in line_obj.browse(cr, uid, ids, context=context):
40+ result.add(line.move_line_id.id)
41+ result.add(line.payment_move_id.id)
42+ return list(result)
43+
44+ def _get_move_lines_order(self, cr, uid, ids, context=None):
45+ result = set()
46+ order_obj = self.pool['payment.order']
47+ for order in order_obj.browse(cr, uid, ids, context=context):
48+ for line in order.line_ids:
49+ result.add(line.move_line_id.id)
50+ result.add(line.payment_move_id.id)
51+ return list(result)
52+
53+ def _get_reconcile(self, cr, uid, ids, context=None):
54+ result = set()
55+ reconcile_obj = self.pool['account.move.reconcile']
56+ for reconcile in reconcile_obj.browse(cr, uid, ids, context=context):
57+ for line in reconcile.line_id:
58+ result.add(line.id)
59+ for line in reconcile.line_partial_ids:
60+ result.add(line.id)
61+ return list(result)
62+
63 _columns = {
64 'received_check': fields.boolean('Received check',
65 help="""To write down that a check in paper support has
66@@ -191,7 +197,15 @@
67 'partner_bank_id': fields.many2one('res.partner.bank', 'Bank Account'),
68 'amount_to_pay': fields.function(
69 amount_to_pay, method=True, type='float', string='Amount to pay',
70- fnct_search=_to_pay_search),
71+ store={
72+ 'account.move.line': (lambda self, cr, uid, ids,
73+ context=None: ids, None, 20),
74+ 'payment.order': (_get_move_lines_order, ['line_ids'], 20),
75+ 'payment.line': (_get_move_lines,
76+ ['type', 'move_line_id', 'payment_move_id'], 20),
77+ 'account.move.reconcile': (_get_reconcile,
78+ ['line_id', 'line_partial_ids'], 20)
79+ }),
80 'payment_type': fields.function(_payment_type_get,
81 type="many2one", relation="payment.type", method=True,
82 string="Payment type", fnct_search=_payment_type_search),