Problem adding partially reconciled moves to a payment order

[sbidoul]

Hello,

When adding invoices to a payment order, the algorithm in account_banking_payment_export (amount_to_pay) takes the credit minus the existing payment lines in valid payment orders.
In a scenario where the original invoice has been partially reconciled with a credit note (and therefore not paid through a payment order), the proposed amount is still the original invoice.

The scenario goes as follow:

• create a supplier invoice for amount 600€
• create a supplier refund for amount 200€ and reconcile partially it with the supplier invoice
• the residual amount on the supplier invoice is 400€
• create a payment order and add the invoice: the system proposes 600€ where the user expects 400€

In official 8.0, the amount to pay is the residual amount (which is now correct in the presence of partial reconciliations). This does not take into existing payment orders anymore, though. IMO, this is fine especially when people work with transfer accounts, because as soon as the payment order is confirmed, the invoice is reconciled and the residual updated.

So I would suggest

• dropping this computation of amount_to_pay
• when searching invoices to pay, filtering on unreconciled entries where credit > 0 instead of amout_to_pay
• when adding to the payment order, using the residual amount

@alexis-via @StefanRijnhart @pedrobaeza what is your opinion?

[alexis-via]

@sbidoul I agree with your proposal, because I always use the module "account_banking_payment_transfer" that generate account moves when the payment.order is validated, so that the residual amount is updated as soon as the payment order is validated.

[pedrobaeza]

Well, I see that a potential user problem, because they can add the invoice to two or more payment orders that are not validated, and then validate both, creating a negative balance and the user will be unnoticed.

It's better to fix the amout to amount_residual field in amount_to_pay than to completely remove it.

[sbidoul]

@pedrobaeza using amount_residual in amount_to_pay won't work because if the paid amount is both reconciled and in a payment order it will be deduced twice from the amount to pay: first in amount residual and second from the payment line.

[sbidoul]

@pedrobaeza your use case could be taken care of by not showing at all any invoice that is in a payment order that is not sent.

[pedrobaeza]

OK, so what do you propose then?

[sbidoul]

@pedrobaeza my proposal is just above your last comment. What do you think?

[alexis-via]

@siboul I like your idea of "not showing at all any invoice that is in a payment order that is not sent." It is simple and reliable.

[StefanRijnhart]

@sbidoul excellent analysis, I agree in principle with your solution + hide invoices from pending payment orders. The situation for setups that don't use clearing accounts could be a bit messy, but I understand from you that Odoo 8.0 natively dropped the check for pending payment orders so I would be happy to follow.

[pedrobaeza]

OK for me too.

[sbidoul]

@alexis-via @pedrobaeza @StefanRijnhart thanks for the feedback.
PR acsone#5 is ready for your review.

[matthieuchoplin]

I have an other use case that has an issue regarding the partial payment:

• I create and validate an invoice with 1000 EUR
• I create a payment order for this invoice but change the amount_currency to 100 EUR
• I confirm it
• I click OK and the state of the payment order is "Sent" and a payment_ids[0] has been created on the invoice with 100 EUR and the residual amount is 900 EUR

So far so good, but when I create an other payment, it is not taken into the payment_ids of the invoice.

• I create a 2nd partial payment, I check that the amount_currency is 900 EUR (the remaining), I change the amount to pay to 200 EUR
• I confirm the payement order and click OK
• The payment order is "Sent"

And when I check the invoice, this is where I think something is wrong:

• len(payment_ids) is 1 instead of 2 (because 2 payments have been made)
• sum of debit should be 300 (100 from the 1st payment and 200 from the 2nd payment) but it is still 100 from the 1st payment
• the residual should be 700 but it is still 900
  => I have a yml test for this one, is it relevant to add it in this PR (through the acsone:8.0-bank-payment-fix93-sbi PR) or create an other one?

[matthieuchoplin]

Sorry, my bad: the last use case is actually working in this PR (#110) but not in the current 8.0 OCA branch, so I add this test as well (acsone#6) 👍

[matthieuchoplin]

I have an interesting behaviour when I test the button "Partial Reconcile Counter" on the payment order.

I can see in the code that we have a domain on the action_window that we return (with the method partial) but strangely it seems to not take it into account when we edit the lines (just click on the line if you were to edit it).

I made this video to illustrate: http://youtu.be/S0FHnnNkMJM

In the use case of the video:

• I click the button and I know that I have made 2 partial payments for an invoice so I expect to see 3 account_move_line: 2 in debit and one in credit. This is fine at first.
• but then I click on the line and the counterpart of each line is displayed (debit for credit and vice versa)

Is it what is expected?

[sbidoul]

That's weird indeed. That's similar to the behaviour of the Journal Items view, but there it occurs only when you change a line. Here it does not look like you change anything, and you just click on different lines, right?

[matthieuchoplin]

yes. I just click on the line.

[sbidoul]

Before we investigate, could you try if you have the same behaviour when going to a Customer or Supplier form and clicking on the Journal Item button on that form?

[matthieuchoplin]

No, I do not have the same behaviour when I go to a supplier form view and click on the button "Journal Items".
I think the issue is with the widget on_write="on_create_write" in the view account.view_move_line_tree because if I remove it I do not have this behaviour.

Yeah, we need to override that method as well I believe:

def on_create_write(self, cr, uid, id, context=None):
    if not id:
        return []
    ml = self.browse(cr, uid, id, context=context)
    return map(lambda x: x.id, ml.move_id.line_id)

What do you think?

[sbidoul]

I must admit I don't think much about that particular issue. I'll ask my colleague @adrienpeiffer.

[adrienpeiffer]

Hi,

I can reproduce this behavior on the Journal Items view.
I added a filter on a partner and one on an account.
When the focus is on a line and I click on another line, all lines of the journal entries of the first line is loaded into the view without considering the filter due to the on_write option on the tree view (And to the on_create_write method).
Personally, I don't really understand the functional aspect of this option and I think it can cause performance problems if we have large journal entries.

Anyone can clarify this? Otherwise, I propose to do a PR to remove this part of code. This is the only place in the core where it is used.

What do you think ?

[matthieuchoplin]

This issue has actually already been reported to Odoo on the 18th Oct 2014: odoo/odoo#3161
I would suggest to leave the code like that and wait for the fix or to not change the core view but create a new one that will not include the on_write="on_create_write" to return the "Partial Reconciles".
Is this the only last blocker for merging the PR #110?

[sbidoul]

@matthieuchoplin from my point of view #110 is ready to merge. Your approval there will help. I'd like to have @pedrobaeza's approval because he had concerns about the selection of customer credit notes.