[stable 6.0.2] Profit and Loss parser logic error leads to incorrect totals

Bug #785427 reported by Graeme Gellatly
18
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Odoo Addons (MOVED TO GITHUB)
Confirmed
Wishlist
OpenERP R&D Addons Team 3
credativ-openerp
New
Undecided
Unassigned
Addons-6.0
Fix Released
Undecided
Unassigned

Bug Description

In the P&L parser you have the following code

                    if typ == 'expense' and account.type <> 'view' and (account.debit <> account.credit):
                        self.result_sum_dr += abs(account.debit - account.credit)
                    if typ == 'income' and account.type <> 'view' and (account.debit <> account.credit):
                        self.result_sum_cr += abs(account.debit - account.credit)

It is clearly incorrect - while it will work fine for those accounts that have the expected - or + balance imagine this situation (or any 1 of a 100 other normal business or accounts setups).

Lets say I offer rebates, but those rebates are not an expense, really they are a reduction in sales revenue, so I declare them as income (quite correctly, at least in my country), now instead of subtracting those rebates from the income total, I add them. My income increases? By offering rebates? My Sales Manager just retired very rich.

Take another really common example - any salespeople reading - salespeople jam their accounts full of product at the end of a fiscal period to maximise their commission. Then in the first month of the period, all that product comes back and sales are actually negative for a while.

What about FX Gains/Losses account or writeoff accounts. Typically these are expense accounts, but sometimes we get lucky and they go in our favour. But now we don't realise the benefit it is subtracted from our profit (well added to our expenses, which reduces our profit)

You cannot use absolute value in any reports or functions relating to account.balances, you must either use the value, or the inverse of the value depending on whether it is defined as a debit or credit account. Or alternatively for debit accounts go dr-cr, and for credit, go cr-dr. In fact, anytime abs is used in relation to anything accounting this should be raising a big red flag.

Revision history for this message
Vinay Rana (OpenERP) (vra-openerp) wrote :

Currently we are not calculating GP or GL separately in Profit and Loss report.
So I am setting this issue for future road map.

Thanks.

Changed in openobject-addons:
assignee: nobody → OpenERP R&D Addons Team 3 (openerp-dev-addons3)
importance: Undecided → Wishlist
status: New → Confirmed
Revision history for this message
Graeme Gellatly (gdgellatly) wrote : Re: [Bug 785427] Re: [stable 6.0.2] Profit and Loss parser logic error leads to incorrect totals
Download full text (3.2 KiB)

Forget Gross Profit, You are still calculating net profit. So because both
sum_dr and sum_cr have the potential to be incorrect, this value has the
potential to be incorrect. Follow this thinking through and now the balance
sheet too is incorrect (although another error in the way balance sheet
parser calls P&L already makes it highly likely be incorrect.

On Fri, May 20, 2011 at 5:01 PM, Vinay Rana (openerp) <
<email address hidden>> wrote:

> Currently we are not calculating GP or GL separately in Profit and Loss
> report.
> So I am setting this issue for future road map.
>
> Thanks.
>
> ** Changed in: openobject-addons
> Importance: Undecided => Wishlist
>
> ** Changed in: openobject-addons
> Status: New => Confirmed
>
> ** Changed in: openobject-addons
> Assignee: (unassigned) => OpenERP R&D Addons Team 3
> (openerp-dev-addons3)
>
> --
> You received this bug notification because you are a direct subscriber
> of the bug.
> https://bugs.launchpad.net/bugs/785427
>
> Title:
> [stable 6.0.2] Profit and Loss parser logic error leads to incorrect
> totals
>
> Status in OpenERP Modules (addons):
> Confirmed
>
> Bug description:
> In the P&L parser you have the following code
>
> if typ == 'expense' and account.type <> 'view' and
> (account.debit <> account.credit):
> self.result_sum_dr += abs(account.debit -
> account.credit)
> if typ == 'income' and account.type <> 'view' and
> (account.debit <> account.credit):
> self.result_sum_cr += abs(account.debit -
> account.credit)
>
> It is clearly incorrect - while it will work fine for those accounts
> that have the expected - or + balance imagine this situation (or any 1
> of a 100 other normal business or accounts setups).
>
> Lets say I offer rebates, but those rebates are not an expense, really
> they are a reduction in sales revenue, so I declare them as income
> (quite correctly, at least in my country), now instead of subtracting
> those rebates from the income total, I add them. My income increases?
> By offering rebates? My Sales Manager just retired very rich.
>
> Take another really common example - any salespeople reading -
> salespeople jam their accounts full of product at the end of a fiscal
> period to maximise their commission. Then in the first month of the
> period, all that product comes back and sales are actually negative
> for a while.
>
> What about FX Gains/Losses account or writeoff accounts. Typically
> these are expense accounts, but sometimes we get lucky and they go in
> our favour. But now we don't realise the benefit it is subtracted
> from our profit (well added to our expenses, which reduces our profit)
>
> You cannot use absolute value in any reports or functions relating to
> account.balances, you must either use the value, or the inverse of the
> value depending on whether it is defined as a debit or credit account.
> Or alternatively for debit accounts go dr-cr, and for credit, go cr-
> dr. In fact, anytime abs is used in relation to anything accounting
> this should be raising a big red flag.
>
> To unsubscribe from this...

Read more...

Revision history for this message
Bogdan Stanciu (bstanciu) wrote :

this is exactly what I meant in my previus comment on the bug #784339

bogdan

Revision history for this message
Dimitri John Ledkov (ex-credativ) (dle-credativ) wrote :

This has been fixed in trunk, with other improvements.

I'm cherry picking a patch for 6.0.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.