Merge lp:~therp-nl/therp-backports/addons-6.1_lp882036_account_rounding into lp:therp-backports

Proposed by Ronald Portier (Therp)
Status: Merged
Merged at revision: 6714
Proposed branch: lp:~therp-nl/therp-backports/addons-6.1_lp882036_account_rounding
Merge into: lp:therp-backports
Diff against target: 20 lines (+2/-1)
1 file modified
account/account.py (+2/-1)
To merge this branch: bzr merge lp:~therp-nl/therp-backports/addons-6.1_lp882036_account_rounding
Reviewer Review Type Date Requested Status
Stefan Rijnhart (Opener) (community) Approve
Review via email: mp+144439@code.launchpad.net

Description of the change

Include fix for lp882306, to correctly round invoice lines.

To post a comment you must log in.
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Hi Ronald.

According to lp:882036 this issue affects openobject-server and the patch was already included in openobject-server 6.1. Should the problem that is solved by this branch in the account module not be filed as a separate issue for openobject-addons?

Cheers,
Stefan.

review: Needs Information
Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

Why a seprate issue? It is the same issue, only the solution needs fixes in both the server (that has been done), and in the addons (done in this merge proposal).

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

Well, the bug description does not mention the account module, and it is not marked for affecting openobject-addons.

In fact, I now notice that one of the last bug comments before yours mentions that lp:1025649 is opened specifically for the addons part and that bug has a rather larger change set attached to it (which is shot down by the reviewer).

So to answer my own question, this branch does not fix lp:882036 but instead partially fixes lp:1025469. Please update the linked bugs and have a look at the suggested fix for that bug and its review.

review: Needs Fixing
Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

I changed the link to the other bug. The merge request already linked and mine do not seem to overlap.

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

> I changed the link to the other bug. The merge request already linked and mine do not seem to overlap.

But if it is that particular issue the solutions to upstream and
backports should be equivalent. How about adopting a fixed version of
the upstream branch in addition to your part?

Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

My correction was developed and tested on 6.1. The other branch is for trunk. So simply merging them is no option.

As my fix solves the problem with rounding taxes, I do not think it a good idea to stall this for a long winding review of all other places where rounding problems might occur, just because somebody did this for trunk.

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

I see that the proposed branch for lp:1025649 has a misleadingly simple diff. When I look at the individual revisions, I see that many more changes have been made, which are already merged. That is out of scope for this branch I admit.

Of course, the title of lp:1025649 does not match the exhaustive scope of the official MP. Furthermore, the reason that your change does not feature in the official MP for lp:1025649 is because a verbatim patch was already merged as a fix for lp:977300.

Nothing out of this mess should do any harm to the merit of your patch so I approve.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account/account.py'
2--- account/account.py 2012-11-16 09:16:59 +0000
3+++ account/account.py 2013-01-23 06:46:24 +0000
4@@ -29,6 +29,7 @@
5 from osv import fields, osv
6 import decimal_precision as dp
7 from tools.translate import _
8+from openerp.tools.float_utils import float_round
9
10 def check_cycle(self, cr, uid, ids, context=None):
11 """ climbs the ``self._table.parent_id`` chains for 100 levels or
12@@ -2091,7 +2092,7 @@
13 tax_compute_precision = precision
14 if taxes and taxes[0].company_id.tax_calculation_rounding_method == 'round_globally':
15 tax_compute_precision += 5
16- totalin = totalex = round(price_unit * quantity, precision)
17+ totalin = totalex = float_round(price_unit * quantity, precision)
18 tin = []
19 tex = []
20 for tax in taxes:

Subscribers

People subscribed via source and target branches

to all changes: