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
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
Yannick Vaucher @ Camptocamp code review Approve
Lorenzo Battistini (community) code review Approve
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.
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :
Revision history for this message
Lorenzo Battistini (elbati) wrote :

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

review: Needs Information
108. By Pedro Manuel Baeza

[IMP] account_payment_extension: method _to_pay_search removed.

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Indeed, it is not needed anymore.

I have remove it.

Thanks for the review.

Regards.

Revision history for this message
Lorenzo Battistini (elbati) :
review: Approve (code review)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) :
review: Approve (code review)
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

    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

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

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Guewen, thanks for your comments. They are done.

Regards.

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

Thanks!

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'account_payment_extension/account_move_line.py'
--- account_payment_extension/account_move_line.py 2013-11-10 17:00:26 +0000
+++ account_payment_extension/account_move_line.py 2014-03-21 11:20:36 +0000
@@ -120,27 +120,6 @@
120 result[move_id] = debt120 result[move_id] = debt
121 return result121 return result
122122
123 def _to_pay_search(self, cr, uid, obj, name, args, context={}):
124 if not len(args):
125 return []
126 currency = self.pool.get('res.users').browse(
127 cr, uid, uid, context).company_id.currency_id
128
129 # For searching we first discard reconciled moves
130 # because the filter is fast and discards most records quickly.
131 ids = self.pool.get('account.move.line').search(
132 cr, uid, [('reconcile_id', '=', False)], context=context)
133 records = self.pool.get('account.move.line').read(
134 cr, uid, ids, ['id', 'amount_to_pay'], context)
135 ids = []
136 for record in records:
137 if not self.pool.get('res.currency').is_zero(
138 cr, uid, currency, record['amount_to_pay']):
139 ids.append(record['id'])
140 if not ids:
141 return [('id', '=', False)]
142 return [('id', 'in', ids)]
143
144 def _payment_type_get(self, cr, uid, ids, field_name, arg, context={}):123 def _payment_type_get(self, cr, uid, ids, field_name, arg, context={}):
145 result = {}124 result = {}
146 invoice_obj = self.pool.get('account.invoice')125 invoice_obj = self.pool.get('account.invoice')
@@ -184,6 +163,33 @@
184 result = [('id', 'in', [x[0] for x in res])]163 result = [('id', 'in', [x[0] for x in res])]
185 return result164 return result
186165
166 def _get_move_lines(self, cr, uid, ids, context=None):
167 result = set()
168 line_obj = self.pool['payment.line']
169 for line in line_obj.browse(cr, uid, ids, context=context):
170 result.add(line.move_line_id.id)
171 result.add(line.payment_move_id.id)
172 return list(result)
173
174 def _get_move_lines_order(self, cr, uid, ids, context=None):
175 result = set()
176 order_obj = self.pool['payment.order']
177 for order in order_obj.browse(cr, uid, ids, context=context):
178 for line in order.line_ids:
179 result.add(line.move_line_id.id)
180 result.add(line.payment_move_id.id)
181 return list(result)
182
183 def _get_reconcile(self, cr, uid, ids, context=None):
184 result = set()
185 reconcile_obj = self.pool['account.move.reconcile']
186 for reconcile in reconcile_obj.browse(cr, uid, ids, context=context):
187 for line in reconcile.line_id:
188 result.add(line.id)
189 for line in reconcile.line_partial_ids:
190 result.add(line.id)
191 return list(result)
192
187 _columns = {193 _columns = {
188 'received_check': fields.boolean('Received check',194 'received_check': fields.boolean('Received check',
189 help="""To write down that a check in paper support has195 help="""To write down that a check in paper support has
@@ -191,7 +197,15 @@
191 'partner_bank_id': fields.many2one('res.partner.bank', 'Bank Account'),197 'partner_bank_id': fields.many2one('res.partner.bank', 'Bank Account'),
192 'amount_to_pay': fields.function(198 'amount_to_pay': fields.function(
193 amount_to_pay, method=True, type='float', string='Amount to pay',199 amount_to_pay, method=True, type='float', string='Amount to pay',
194 fnct_search=_to_pay_search),200 store={
201 'account.move.line': (lambda self, cr, uid, ids,
202 context=None: ids, None, 20),
203 'payment.order': (_get_move_lines_order, ['line_ids'], 20),
204 'payment.line': (_get_move_lines,
205 ['type', 'move_line_id', 'payment_move_id'], 20),
206 'account.move.reconcile': (_get_reconcile,
207 ['line_id', 'line_partial_ids'], 20)
208 }),
195 'payment_type': fields.function(_payment_type_get,209 'payment_type': fields.function(_payment_type_get,
196 type="many2one", relation="payment.type", method=True,210 type="many2one", relation="payment.type", method=True,
197 string="Payment type", fnct_search=_payment_type_search),211 string="Payment type", fnct_search=_payment_type_search),