Bug In PaySelectionCreateLine - CreateFrom

[Nicolas Micoud]

Hi,

Open the Payment Selection window and create a new record.
Push the "CreateFrom" button.

It should retrieve records (invoices which should be paid).

Delete some records, but at least one record must remain in the sub tab and launch the process again : this time, invoices are not retrieved.

If you delete all PaySelectionLine, then the process will retrieve invoices

=> Reproducible on test.idempiere.org (see 'test nicolas')

Issue seems located at line 144 of PaySelectionCreateFrom :

.append(" AND i.C_Invoice_ID NOT IN (SELECT i.C_Invoice_ID FROM C_PaySelectionLine psl WHERE psl.C_PaySelection_ID=?)")

But as it is an old process, I guess it must have worked in the past ?

Any idea to resolve it ?

Thanks,

Nicolas

[Carlos Antonio Ruiz Gomez]

Hi Nicolas, I just fixed the error with commit 712f474

It was introduced by me 5 years ago in ticket IDEMPIERE-2134, seems like nobody tested that specific case during these years  :-)

Regards,

Carlos Ruiz

El 13/09/19 a las 12:13, Nicolas Micoud escribió:

--

[Nicolas Micoud]

Hi Carlos,

May I suggest another issue ?

Making some tests, I set the Payment date to '2019-10-31'. ATM, I have no currency rate stored in DB for that date.

So the PayAmt calculated by the SQL is null, and it throws a NPE on line 233

I would say that

if (C_Invoice_ID == 0 || Env.ZERO.compareTo(PayAmt) == 0)

should be replaced by

if (C_Invoice_ID == 0 || PayAmt == null || Env.ZERO.compareTo(PayAmt) == 0)

But... not sure if it's the good solution. As the invoices will not be retrieved just because of a missing currency rate.

Perhaps something like

if (PayAmt == null)
    return @Error@ @PayAmt@ can't be calculated for " + new MInvoice(getCtx(), C_Invoice_ID, get_TrxName()).getDocumentInfo();

would be better ?

wdyt ?

Thanks,

Nicolas

[Carlos Antonio Ruiz Gomez]

Well, NPE is not good, neither is hiding the invoices.

So, yes, I think a meaningful translatable user error is better.

That must be the same case in the Payment Selection (manual) form, in that form for usability is better to show that message when the invoices are selected, if there are invoices with PayAmt null show a message like "Could not find Payment Amount for some invoices, maybe currency rate is not configured" - and disable the process button.

Regards,

Carlos Ruiz

El 13/09/19 a las 15:32, Nicolas Micoud escribió:

--

[Nicolas Micoud]

I haven't really tested the manual form, I'll work on patches later :)

For the process, what about using a addLog ?

                if (PayAmt == null) {
                    addLog(C_Invoice_ID, null, null, new MInvoice(getCtx(), C_Invoice_ID, get_TrxName()).getDocumentInfo() + " : could not find Payment Amount, maybe currency rate is not configured");
                    continue;
                }

Tested locally, it will display this popup at the end of the process, listing all "not valid" invoices :

Other invoices are inserted - so is not blocking user from paying invoices because maybe one will have an issue.

wdyt ?
nb: message will be displayed using AD_Message

[Carlos Antonio Ruiz Gomez]

>Other invoices are inserted - so is not blocking user from paying invoices because maybe one will have an issue.

I think it's better not to generate anything when there is such misconfiguration error.

As the actual code doesn't generate partially, but throws an NPE, I would suggest to keep not generating partial, just fixing the message.

Regards,

Carlos Ruiz

El 13/09/19 a las 16:33, Nicolas Micoud escribió:

[Nicolas Micoud]

Hi,

Patch submitted https://idempiere.atlassian.net/browse/IDEMPIERE-2134

Nicolas