[account] wizard_pay_invoice may calculate a wrong invoice total (5.0) patch included!

I can confirm this bug, specially with use of account_anglo_saxon module.
From functional point of view imho the pay invoice wizard should only check the lines related to the invoice, as suggested by Dukai. I didn't check the patch as others do have more programming experience.

Dukai,

You use invoice.account_id as a move line filter. I suggest instead that you check for each move if the account is a "receivable" or "payable" account. This is more consistent and allows other modules to split receivables/payables in more than one account (say "Receivables/Credit Card" for credit card payments, "Receivables/Cheque" for cheque payments.

Hi!
We use invoices _not_ only with receivable and payable accounts for example for products that we didn't sell but consumed ourselves. Your suggestion would break that use case.

This "Pay Invoice" wizard is an old wizard and can't be overloaded by modules. IMHO, the "right" course of action would be:

1) Refactor the old wizard "Pay Invoice" into a new wizard (osv.osv_memory)
2) Overload the relevant methods in account_anglo_saxon

This way, both solutions are possible.

osv_memory wizards cannot be inherited in 5.0 :(

So this bug may be targeted to 5.2

I agree with Almeida that such wizards need flexibility and hence need to be the new wizard.

I agree that this wizard also should be rewritten to osv_memory but this is not an option for 5.0.
So I propose a second version of the patch that should work in all cases discussed here.

Please review.

Thank you Jan and Dukai,

It has been applied into stable by revision 2540 <email address hidden>.

Hi !

!!! Just revert this please !!!

This make a huge regression in the wizard !!!! I realize that the debit/credit amount is wrongly computed in the payment lines....

And please, in the future, make some tests case to ensure no regression on this !

Here the test case from OERPScenario that breaks :

      Given I am loged as admin user with password admin used
      And the company currency is set to EUR
      And the following currency rate settings are:
      |code|rate|name|
      |EUR|1.000|01-01-2009|
      |CHF|1.644|01-01-2009|
      |CHF|1.500|09-09-2009|
      |CHF|0.6547|10-10-2009|
      |USD|1.3785|01-01-2009|
      And a cash journal in USD exists
      And a cash journal in CHF exists
      And a cash journal in EUR exists

    Scenario: make_and_validate_payments_with_pay_invoice_wizard
      Given I have recorded on the 1 jan 2009 a supplier invoice (in_invoice) of 1000,0 CHF without tax called MySupplierInvoicePayWizard
      When I press the validate button
      Then I should see the invoice MySupplierInvoicePayWizard open

      When I call the Pay invoice wizard
      And I partially pay 200.0 CHF.- on the 10 jan 2009
      Then I should see a residual amount of 800.0 CHF.-

      When I call the Pay invoice wizard
      And I partially pay 200.0 USD.- on the 11 jan 2009
      Then I should see a residual amount of 561.48 CHF.-

BREAKS HERE !!!
features/account/wizard/step_definitions/wizard_pay_invoice.rb:77
      expected: 561.48,
           got: 600.0 (using ==)

      When I call the Pay invoice wizard
      And I partially pay 200.0 EUR.- on the 12 jan 2009
      Then I should see a residual amount of 232.68 CHF.-

      When I call the Pay invoice wizard
      And I completely pay the residual amount in CHF on the 13 sep 2009
      Then I should see a residual amount of 0.0 CHF.-
      And I should see the invoice MySupplierInvoicePayWizard paid

Regards,

Forgot to tell you the revision to use for OERPScenario: trunk v. 58, with Ooor v. 1.2.2, if you want to try...

It has been reverted for the moment.

I will check this out with examples.

Thank you for your feedback.

Hello Dukai,

Can you please check this one?

If there is a problem, we should write this method on the account_anglo_saxon module, not ion account.

Thanks.

Hi,

I just tested more this one, and finally find that there is another regression somewhere which cause trouble I reported before.

So, may be this patch is good, and the trouble come from an other part.

I continue my investigation.

Regards,

Hi,

The regression is into the revision 2537, here the diff:

=== modified file 'account/wizard/wizard_pay_invoice.py'
--- account/wizard/wizard_pay_invoice.py 2009-11-02 11:24:47 +0000
+++ account/wizard/wizard_pay_invoice.py 2010-01-19 09:39:04 +0000
@@ -66,6 +66,13 @@
         currency_id = journal.currency.id
         # Put the paid amount in currency, and the currency, in the context if currency is different from company's currency
         context.update({'amount_currency':form['amount'],'currency_id':currency_id})
+
+ if invoice.company_id.currency_id.id<>invoice.currency_id.id:
+ ctx = {'date':data['form']['date']}
+ amount = cur_obj.compute(cr, uid, invoice.currency_id.id, invoice.company_id.currency_id.id, amount, context=ctx)
+ currency_id = invoice.currency_id.id
+ # Put the paid amount in currency, and the currency, in the context if currency is different from company's currency
+ context.update({'amount_currency':form['amount'],'currency_id':currency_id})

     # Take the choosen date
     if form.has_key('comment'):

This is what cause trouble with residual amount and so.

Conclusion : I open another bug report, and can somebody else confirm the patch from Dukai Gabor is ok ?

Regards,

Hello Joel,

It was committed for this reason: What if the journal currency is not specified, and company currency is different from invoice currency.

Have this patch and let us know:
--- account/wizard/wizard_pay_invoice.py 2010-01-20 11:40:27 +0000
+++ account/wizard/wizard_pay_invoice.py 2010-01-22 10:11:42 +0000
@@ -60,14 +60,16 @@
     journal = pool.get('account.journal').browse(cr, uid, data['form']['journal_id'], context)
     # Compute the amount in company's currency, with the journal currency (which is equal to payment currency)
     # when it is needed : If payment currency (according to selected journal.currency) is <> from company currency
+ cur_diff = False
     if journal.currency and invoice.company_id.currency_id.id<>journal.currency.id:
         ctx = {'date':data['form']['date']}
         amount = cur_obj.compute(cr, uid, journal.currency.id, invoice.company_id.currency_id.id, amount, context=ctx)
         currency_id = journal.currency.id
         # Put the paid amount in currency, and the currency, in the context if currency is different from company's currency
         context.update({'amount_currency':form['amount'],'currency_id':currency_id})
+ cur_diff = True

- if invoice.company_id.currency_id.id<>invoice.currency_id.id:
+ if not cur_diff and invoice.company_id.currency_id.id<>invoice.currency_id.id:
         ctx = {'date':data['form']['date']}
         amount = cur_obj.compute(cr, uid, invoice.currency_id.id, invoice.company_id.currency_id.id, amount, context=ctx)
         currency_id = invoice.currency_id.id

Hi Jay,

I applied this one (Just change the indentation from yours...):

=== modified file 'account/wizard/wizard_pay_invoice.py'
--- account/wizard/wizard_pay_invoice.py 2010-01-20 11:40:27 +0000
+++ account/wizard/wizard_pay_invoice.py 2010-01-22 11:03:01 +0000
@@ -60,14 +60,16 @@
     journal = pool.get('account.journal').browse(cr, uid, data['form']['journal_id'], context)
     # Compute the amount in company's currency, with the journal currency (which is equal to payment currency)
     # when it is needed : If payment currency (according to selected journal.currency) is <> from company currency
+ cur_diff = False
     if journal.currency and invoice.company_id.currency_id.id<>journal.currency.id:
         ctx = {'date':data['form']['date']}
         amount = cur_obj.compute(cr, uid, journal.currency.id, invoice.company_id.currency_id.id, amount, context=ctx)
         currency_id = journal.currency.id
         # Put the paid amount in currency, and the currency, in the context if currency is different from company's currency
         context.update({'amount_currency':form['amount'],'currency_id':currency_id})
-
- if invoice.company_id.currency_id.id<>invoice.currency_id.id:
+ cur_diff = True
+ # if invoice.company_id.currency_id.id<>invoice.currency_id.id:
+ if not cur_diff and invoice.company_id.currency_id.id<>invoice.currency_id.id:
         ctx = {'date':data['form']['date']}
         amount = cur_obj.compute(cr, uid, invoice.currency_id.id, invoice.company_id.currency_id.id, amount, context=ctx)
         currency_id = invoice.currency_id.id

This pass the last errors, but still get one on the next one:
  Background:
      Given I am loged as admin user with password admin used
      And the company currency is set to EUR
      And the following currency rate settings are:
      |code|rate|name|
      |EUR|1.000|01-01-2009|
      |CHF|1.644|01-01-2009|
      |CHF|1.500|09-09-2009|
      |CHF|0.6547|10-10-2009|
      |USD|1.3785|01-01-2009|
      And a cash journal in USD exists
      And a cash journal in CHF exists
      And a cash journal in EUR exists

    Scenario: make_and_validate_payments_with_pay_invoice_wizard
      Given I have recorded on the 1 jan 2009 a supplier invoice (in_invoice) of 1000,0 CHF without tax called MySupplierInvoicePayWizard
      When I press the validate button
      Then I should see the invoice MySupplierInvoicePayWizard open

      When I call the Pay invoice wizard
      And I partially pay 200.0 CHF.- on the 10 jan 2009
      Then I should see a residual amount of 800.0 CHF.-

      When I call the Pay invoice wizard
      And I partially pay 200.0 USD.- on the 11 jan 2009
      Then I should see a residual amount of 561.48 CHF.-

      When I call the Pay invoice wizard
      And I partially pay 200.0 EUR.- on the 12 jan 2009
      Then I should see a residual amount of 232.68 CHF.-

BREAKS HERE !!!
expected: 232.68,
           got: 361.48 (using ==)

      When I call the Pay invoice wizard
      And I completely pay the residual amount in CHF on the 13 sep 2009
      Then I should see a residual amount of 0.0 CHF.-
      And I should see the invoi...

Hello Joel,,

Can you please tell me, In which currency you had created your invoice mean invoice currency?

Thanks.

Hi !

Yes, sure, it's already in my test case :

Given I have recorded on the 1 jan 2009 a supplier invoice (in_invoice) of 1000,0 CHF without tax called MySupplierInvoicePayWizard

This mean, I recorded an invoice in CHF in this case. But please, pay attention to the following:

Currency rate are:

 |EUR|1.000|01-01-2009
 |CHF|1.644|01-01-2009|
 |CHF|1.500|09-09-2009|
 |CHF|0.6547|10-10-2009|

Company currency is : EUR (as define in background)

The date of the invoice is 1 jan 2009 !

The date of payment of course matter !

Thanks in advance for having a look !

Cheers,

Hello,

Please apply the attached new patch and notify me.

Thanks.

Hi,

I try it, but still get an error :

  Scenario: make_and_validate_payments_with_pay_invoice_wizard
    Given I have recorded on the 1 jan 2009 a supplier invoice (in_invoice) of 1000,0 CHF without tax called MySupplierInvoicePayWizard
    When I press the validate button
    Then I should see the invoice MySupplierInvoicePayWizard open
    When I call the Pay invoice wizard
    And I partially pay 200.0 CHF.- on the 10 jan 2009
    Then I should see a residual amount of 800.0 CHF.-
    When I call the Pay invoice wizard
    And I partially pay 200.0 USD.- on the 11 jan 2009
    Then I should see a residual amount of 561.48 CHF.-
      expected: 561.48,
           got: 561.0 (using ==)

       Diff:
      @@ -1,2 +1,2 @@
      -561.48
      +561.0
       (Spec::Expectations::ExpectationNotMetError)
      ./features/account/wizard/step_definitions/wizard_pay_invoice.rb:78:in `/^I should see a residual amount of (.*) (\w+)\.\-$/'
      features/account/wizard/pay_invoice.feature:26:in `Then I should see a residual amount of 561.48 CHF.-'

What is strange is that, doing the same test without your patch, get the following :

    Given I have recorded on the 1 jan 2009 a supplier invoice (in_invoice) of 1000,0 CHF without tax called MySupplierInvoicePayWizard
    When I press the validate button
    Then I should see the invoice MySupplierInvoicePayWizard open
    When I call the Pay invoice wizard
    And I partially pay 200.0 CHF.- on the 10 jan 2009
    Then I should see a residual amount of 800.0 CHF.-
    When I call the Pay invoice wizard
*********** OpenERP Server ERROR:
            Traceback (most recent call last):
  File "/Users/jgrandguillaume/c2c/Code/Official/5.0stable/server/bin/netsvc.py", line 244, in dispatch
    result = LocalService(service_name)(method, *params)
  File "/Users/jgrandguillaume/c2c/Code/Official/5.0stable/server/bin/netsvc.py", line 73, in __call__
    return getattr(self, method)(*params)
  File "/Users/jgrandguillaume/c2c/Code/Official/5.0stable/server/bin/service/web_services.py", line 633, i...

If I understand it right, you work on solving a different bug, not the one this bugreport was created for.

Could you open another bugreport for that problem? Then the original payment_wizard_fix_v2.patch could be reapplied again that was reverted by mistake.

Hi,

The thing is, we have probably 2 bugs here. But I won't allow a commit which introduce another...

I report now the bug which raise the following errors:

Couldn't create move with currency different from the secondary currency
of the account "1024 - Compte en devise A". Clear the secondary currency
field of the account definition if you want to accept all currencies.

But still, the commit suggested here by vra does not compute the residual amount correctly ...

See first error :
Then I should see a residual amount of 561.48 CHF.-
     expected: 561.48,
          got: 561.0 (using ==)

Regards,

I forgot I already reported it here :

https://bugs.launchpad.net/openobject-addons/+bug/511104

:)

Fixed by revision 2550 <email address hidden>.

Hi,

I confirm this is fixed.

Many thanks !

Regards