Merge lp:~pedro.baeza/ocb-addons/6.1-fix-1223243 into lp:ocb-addons/6.1

Proposed by Pedro Manuel Baeza
Status: Work in progress
Proposed branch: lp:~pedro.baeza/ocb-addons/6.1-fix-1223243
Merge into: lp:ocb-addons/6.1
Diff against target: 75 lines (+8/-22)
2 files modified
account/account.py (+5/-7)
account/account_move_line.py (+3/-15)
To merge this branch: bzr merge lp:~pedro.baeza/ocb-addons/6.1-fix-1223243
Reviewer Review Type Date Requested Status
Stefan Rijnhart (Opener) Needs Fixing
Review via email: mp+198291@code.launchpad.net

Description of the change

Improvement made on 7.0 when posting account move lines. I have to admit that I haven't fully tested it, but overall checks are OK and the improvement deserves to make the MP.

Regards.

To post a comment you must log in.
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

It is a cherrypicking from revisions 9458 and 9679 of 7.0 branch, manually merging conflicting lines.

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Pedro,

I know you do a lot of good work, but I am a little unhappy with a proposal that you did not even fully test yourself (thank you for your honesty though!). That's enough to warrant a disapproval as far as I'm concerned, as you can't really expect a reviewer to spend valuable time on that.

Also, I am having a hard time trying to find out what this change really does. There is no bug report, and it even looks like there are several changes. In that case, you really should put in a detailed description of what each change does, if only to convince the reviewers that these changes are a good thing. And if upstream revision numbers are the only pointers, at least make them links to the revisions on Launchpad!

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

Hi, Stefan, thanks for your prudence, and sorry for the mistakes on this MP. First of all, I thought that I linked the corresponding bug, but I wasn't. Now, it is linked. In this bug are explained all the changes, but I paste here again for reference:

- revision 9458: reduce the number of time validate() function is called on account move (worst case was one per line, now should be one per move)
- revision 9679: reduce the number of time analytic lines are created and dropped as mentioned Frederic (only end of validation, unbalanced move creates no analytic lines)

I'm going to set this as WIP until I have some time to fully tested.

Regards.

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks for clarifying!

Unmerged revisions

6822. By Pedro Manuel Baeza

[IMP] Backport of 7.0 rev. 9458 and 9679

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'account/account.py'
--- account/account.py 2013-11-20 01:17:39 +0000
+++ account/account.py 2013-12-09 17:06:05 +0000
@@ -1422,11 +1422,14 @@
1422 l[2]['period_id'] = default_period1422 l[2]['period_id'] = default_period
1423 context['period_id'] = default_period1423 context['period_id'] = default_period
14241424
1425 if 'line_id' in vals:1425 if vals.get('line_id', False):
1426 c = context.copy()1426 c = context.copy()
1427 c['novalidate'] = True1427 c['novalidate'] = True
1428 result = super(account_move, self).create(cr, uid, vals, c)1428 result = super(account_move, self).create(cr, uid, vals, c)
1429 self.validate(cr, uid, [result], context)1429 tmp = self.validate(cr, uid, [result], context)
1430 journal = self.pool.get('account.journal').browse(cr, uid, vals['journal_id'], context)
1431 if journal.entry_posted and tmp:
1432 self.button_validate(cr,uid, [result], context)
1430 else:1433 else:
1431 result = super(account_move, self).create(cr, uid, vals, context)1434 result = super(account_move, self).create(cr, uid, vals, context)
1432 return result1435 return result
@@ -1569,11 +1572,6 @@
1569 obj_analytic_line = self.pool.get('account.analytic.line')1572 obj_analytic_line = self.pool.get('account.analytic.line')
1570 obj_move_line = self.pool.get('account.move.line')1573 obj_move_line = self.pool.get('account.move.line')
1571 for move in self.browse(cr, uid, ids, context):1574 for move in self.browse(cr, uid, ids, context):
1572 # Unlink old analytic lines on move_lines
1573 for obj_line in move.line_id:
1574 for obj in obj_line.analytic_lines:
1575 obj_analytic_line.unlink(cr,uid,obj.id)
1576
1577 journal = move.journal_id1575 journal = move.journal_id
1578 amount = 01576 amount = 0
1579 line_ids = []1577 line_ids = []
15801578
=== modified file 'account/account_move_line.py'
--- account/account_move_line.py 2012-09-06 14:35:17 +0000
+++ account/account_move_line.py 2013-12-09 17:06:05 +0000
@@ -171,6 +171,8 @@
171 if obj_line.analytic_account_id:171 if obj_line.analytic_account_id:
172 if not obj_line.journal_id.analytic_journal_id:172 if not obj_line.journal_id.analytic_journal_id:
173 raise osv.except_osv(_('No Analytic Journal !'),_("You have to define an analytic journal on the '%s' journal!") % (obj_line.journal_id.name, ))173 raise osv.except_osv(_('No Analytic Journal !'),_("You have to define an analytic journal on the '%s' journal!") % (obj_line.journal_id.name, ))
174 if obj_line.analytic_lines:
175 acc_ana_line_obj.unlink(cr,uid,[obj.id for obj in obj_line.analytic_lines])
174 amt = (obj_line.credit or 0.0) - (obj_line.debit or 0.0)176 amt = (obj_line.credit or 0.0) - (obj_line.debit or 0.0)
175 vals_lines = {177 vals_lines = {
176 'name': obj_line.name,178 'name': obj_line.name,
@@ -1316,20 +1318,6 @@
1316 if not ok:1318 if not ok:
1317 raise osv.except_osv(_('Bad account !'), _('You can not use this general account in this journal, check the tab \'Entry Controls\' on the related journal !'))1319 raise osv.except_osv(_('Bad account !'), _('You can not use this general account in this journal, check the tab \'Entry Controls\' on the related journal !'))
13181320
1319 if vals.get('analytic_account_id',False):
1320 if journal.analytic_journal_id:
1321 vals['analytic_lines'] = [(0,0, {
1322 'name': vals['name'],
1323 'date': vals.get('date', time.strftime('%Y-%m-%d')),
1324 'account_id': vals.get('analytic_account_id', False),
1325 'unit_amount': vals.get('quantity', 1.0),
1326 'amount': vals.get('debit', 0.0) or vals.get('credit', 0.0),
1327 'general_account_id': vals.get('account_id', False),
1328 'journal_id': journal.analytic_journal_id.id,
1329 'ref': vals.get('ref', False),
1330 'user_id': uid
1331 })]
1332
1333 result = super(account_move_line, self).create(cr, uid, vals, context=context)1321 result = super(account_move_line, self).create(cr, uid, vals, context=context)
1334 # CREATE Taxes1322 # CREATE Taxes
1335 if vals.get('account_tax_id', False):1323 if vals.get('account_tax_id', False):
@@ -1391,7 +1379,7 @@
1391 self.create(cr, uid, data, context)1379 self.create(cr, uid, data, context)
1392 del vals['account_tax_id']1380 del vals['account_tax_id']
13931381
1394 if check and ((not context.get('no_store_function')) or journal.entry_posted):1382 if check and not context.get('novalidate') and ((not context.get('no_store_function')) or journal.entry_posted):
1395 tmp = move_obj.validate(cr, uid, [vals['move_id']], context)1383 tmp = move_obj.validate(cr, uid, [vals['move_id']], context)
1396 if journal.entry_posted and tmp:1384 if journal.entry_posted and tmp:
1397 move_obj.button_validate(cr,uid, [vals['move_id']], context)1385 move_obj.button_validate(cr,uid, [vals['move_id']], context)

Subscribers

People subscribed via source and target branches