From dfe2fff6cde3e1a184f2cb59c2b4fb3901cf40aa Mon Sep 17 00:00:00 2001 From: hieplq Date: Thu, 1 Jun 2023 12:15:48 +0700 Subject: [PATCH] IDEMPIERE-5723 NPE when allocate has both AR and AP invoice (#1842) * IDEMPIERE-5723 NPE when allocate has both AR and AP invoice base on patch from gauravsontakke * IDEMPIERE-5723 NPE when allocate has both AR and AP invoice (fix code review) https://github.com/idempiere/idempiere/pull/1842#pullrequestreview-1429618616 * IDEMPIERE-5723 NPE when allocate has both AR and AP invoice (check Accrual) createInvoiceGainLoss isn't yet test for non accrual so at check for accrual to consistent with above code --- .../org/compiere/acct/Doc_AllocationHdr.java | 87 +++++++++++-------- .../idempiere/test/model/AllocationTest.java | 52 +++++++++++ 2 files changed, 103 insertions(+), 36 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java b/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java index e172e5848b..755c4a5fba 100644 --- a/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java +++ b/org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java @@ -21,8 +21,9 @@ import java.math.RoundingMode; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.util.ArrayList; -import java.util.Hashtable; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.logging.Level; import org.compiere.model.MAccount; @@ -190,6 +191,8 @@ public class Doc_AllocationHdr extends Doc Fact factForRGL = new Fact(this, as, Fact.POST_Actual); // dummy fact (not posted) to calculate Realized Gain & Loss boolean isInterOrg = isInterOrg(as); MAccount bpAcct = null; // Liability/Receivables + MAccount bpAcctAr = null; + MAccount bpAcctAp = null; for (int i = 0; i < p_lines.length; i++) { @@ -324,7 +327,9 @@ public class Doc_AllocationHdr extends Doc // AR Invoice Amount CR if (as.isAccrual()) { - bpAcct = getAccount(Doc.ACCTTYPE_C_Receivable, as); + if (bpAcctAr == null) + bpAcctAr = getAccount(Doc.ACCTTYPE_C_Receivable, as); + bpAcct = bpAcctAr; fl = fact.createLine (line, bpAcct, getC_Currency_ID(), null, allocationSource); // payment currency if (fl != null) @@ -377,7 +382,9 @@ public class Doc_AllocationHdr extends Doc // AP Invoice Amount DR if (as.isAccrual()) { - bpAcct = getAccount(Doc.ACCTTYPE_V_Liability, as); + if (bpAcctAp == null) + bpAcctAp = getAccount(Doc.ACCTTYPE_V_Liability, as); + bpAcct = bpAcctAp; fl = fact.createLine (line, bpAcct, getC_Currency_ID(), allocationSource, null); // payment currency if (fl != null) @@ -456,7 +463,7 @@ public class Doc_AllocationHdr extends Doc } // Realized Gain & Loss - if (invoice != null + if (invoice != null && as.isAccrual() && (getC_Currency_ID() != as.getC_Currency_ID() // payment allocation in foreign currency || getC_Currency_ID() != line.getInvoiceC_Currency_ID())) // allocation <> invoice currency { @@ -479,7 +486,7 @@ public class Doc_AllocationHdr extends Doc // rounding adjustment if (getC_Currency_ID() != as.getC_Currency_ID()) { - p_Error = createInvoiceRoundingCorrection (as, fact, bpAcct); + p_Error = createInvoiceRoundingCorrection (as, fact, bpAcctAr, bpAcctAp); if (p_Error != null) return null; p_Error = createPaymentRoundingCorrection (as, fact); @@ -1065,26 +1072,30 @@ public class Doc_AllocationHdr extends Doc * @param acct account * @return Error Message or null if OK */ - private String createInvoiceRoundingCorrection (MAcctSchema as, Fact fact, MAccount acct) + private String createInvoiceRoundingCorrection (MAcctSchema as, Fact fact, MAccount acctAr, MAccount acctAp) { - ArrayList invList = new ArrayList(); - Hashtable htInvAllocLine = new Hashtable(); + Map invList = new HashMap<>(); + Map htInvAllocLine = new HashMap<>(); for (int i = 0; i < p_lines.length; i++) { MInvoice invoice = null; - DocLine_Allocation line = (DocLine_Allocation)p_lines[i]; - if (line.getC_Invoice_ID() != 0) - { + DocLine_Allocation line = (DocLine_Allocation)p_lines[i]; + + if (line.getC_Invoice_ID() == 0) + continue; + + if (invList.containsKey(line.getC_Invoice_ID())){ + log.severe(line.getC_Invoice_ID() + ":same invoice included in more than one allocation line"); + }else { invoice = new MInvoice (getCtx(), line.getC_Invoice_ID(), getTrxName()); - if (!invList.contains(invoice)) - invList.add(invoice); + invList.put(invoice.getC_Invoice_ID(), invoice); htInvAllocLine.put(invoice.getC_Invoice_ID(), line.get_ID()); } } - Hashtable htInvSource = new Hashtable(); - Hashtable htInvAccounted = new Hashtable(); - for (MInvoice invoice : invList) + Map htInvSource = new HashMap<>(); + Map htInvAccounted = new HashMap<>(); + for (MInvoice invoice : invList.values()) { StringBuilder sql = new StringBuilder() .append("SELECT SUM(AmtSourceDr), SUM(AmtAcctDr), SUM(AmtSourceCr), SUM(AmtAcctCr)") @@ -1093,7 +1104,7 @@ public class Doc_AllocationHdr extends Doc .append(" AND C_AcctSchema_ID=?") .append(" AND Account_ID=?") .append(" AND PostingType='A'"); - + MAccount acct = invoice.isSOTrx() ? acctAr : acctAp; // For Invoice List valuesInv = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MInvoice.Table_ID, invoice.getC_Invoice_ID(), as.getC_AcctSchema_ID(), acct.getAccount_ID()); @@ -1128,10 +1139,10 @@ public class Doc_AllocationHdr extends Doc MAccount gain = MAccount.get (as.getCtx(), as.getAcctSchemaDefault().getRealizedGain_Acct()); MAccount loss = MAccount.get (as.getCtx(), as.getAcctSchemaDefault().getRealizedLoss_Acct()); - Hashtable htTotalAmtSourceDr = new Hashtable(); - Hashtable htTotalAmtAcctDr = new Hashtable(); - Hashtable htTotalAmtSourceCr = new Hashtable(); - Hashtable htTotalAmtAcctCr = new Hashtable(); + Map htTotalAmtSourceDr = new HashMap<>(); + Map htTotalAmtAcctDr = new HashMap<>(); + Map htTotalAmtSourceCr = new HashMap<>(); + Map htTotalAmtAcctCr = new HashMap<>(); FactLine[] factlines = fact.getLines(); for (FactLine factLine : factlines) { @@ -1140,6 +1151,8 @@ public class Doc_AllocationHdr extends Doc MAllocationLine allocationLine = new MAllocationLine(getCtx(), factLine.getLine_ID(), getTrxName()); if (allocationLine.getC_Invoice_ID() > 0) { + MInvoice invoice = invList.get(allocationLine.getC_Invoice_ID()); + MAccount acct = invoice.isSOTrx() ? acctAr : acctAp; if (factLine.getAccount_ID() == acct.getAccount_ID()) { BigDecimal totalAmtSourceDr = htTotalAmtSourceDr.get(allocationLine.getC_Invoice_ID()); @@ -1187,9 +1200,9 @@ public class Doc_AllocationHdr extends Doc } } - Hashtable htAllocInvSource = new Hashtable(); - Hashtable htAllocInvAccounted = new Hashtable(); - for (MInvoice invoice : invList) + Map htAllocInvSource = new HashMap<>(); + Map htAllocInvAccounted = new HashMap<>(); + for (MInvoice invoice : invList.values()) { BigDecimal allocateSource = Env.ZERO; BigDecimal allocateAccounted = Env.ZERO; @@ -1247,6 +1260,7 @@ public class Doc_AllocationHdr extends Doc .append(" AND Account_ID=?") .append(" AND Line_ID IN (SELECT C_AllocationLine_ID FROM C_AllocationLine WHERE C_AllocationHdr_ID=? AND C_Invoice_ID=?)"); + MAccount acct = invoice.isSOTrx() ? acctAr : acctAp; // For Allocation List valuesAlloc = DB.getSQLValueObjectsEx(getTrxName(), sql.toString(), MAllocationHdr.Table_ID, alloc.get_ID(), as.getC_AcctSchema_ID(), acct.getAccount_ID(), alloc.get_ID(), invoice.getC_Invoice_ID()); @@ -1328,8 +1342,9 @@ public class Doc_AllocationHdr extends Doc htAllocInvAccounted.put(invoice.getC_Invoice_ID(), allocateAccounted); } - for (MInvoice invoice : invList) + for (MInvoice invoice : invList.values()) { + MAccount acct = invoice.isSOTrx() ? acctAr : acctAp; BigDecimal invSource = htInvSource.get(invoice.getC_Invoice_ID()); if (invSource == null) invSource = Env.ZERO; @@ -1444,8 +1459,8 @@ public class Doc_AllocationHdr extends Doc */ private String createPaymentRoundingCorrection (MAcctSchema as, Fact fact) { - ArrayList payList = new ArrayList(); - Hashtable htPayAllocLine = new Hashtable(); + List payList = new ArrayList(); + Map htPayAllocLine = new HashMap<>(); for (int i = 0; i < p_lines.length; i++) { MPayment payment = null; @@ -1459,9 +1474,9 @@ public class Doc_AllocationHdr extends Doc } } - Hashtable htPayAcct = new Hashtable(); - Hashtable htPaySource = new Hashtable(); - Hashtable htPayAccounted = new Hashtable(); + Map htPayAcct = new HashMap<>(); + Map htPaySource = new HashMap<>(); + Map htPayAccounted = new HashMap<>(); for (MPayment payment : payList) { htPayAcct.put(payment.getC_Payment_ID(), getPaymentAcct(as, payment.getC_Payment_ID())); @@ -1494,10 +1509,10 @@ public class Doc_AllocationHdr extends Doc MAccount gain = MAccount.get (as.getCtx(), as.getAcctSchemaDefault().getRealizedGain_Acct()); MAccount loss = MAccount.get (as.getCtx(), as.getAcctSchemaDefault().getRealizedLoss_Acct()); - Hashtable htTotalAmtSourceDr = new Hashtable(); - Hashtable htTotalAmtAcctDr = new Hashtable(); - Hashtable htTotalAmtSourceCr = new Hashtable(); - Hashtable htTotalAmtAcctCr = new Hashtable(); + Map htTotalAmtSourceDr = new HashMap<>(); + Map htTotalAmtAcctDr = new HashMap<>(); + Map htTotalAmtSourceCr = new HashMap<>(); + Map htTotalAmtAcctCr = new HashMap<>(); FactLine[] factlines = fact.getLines(); for (FactLine factLine : factlines) { @@ -1553,8 +1568,8 @@ public class Doc_AllocationHdr extends Doc } } - Hashtable htAllocPaySource = new Hashtable(); - Hashtable htAllocPayAccounted = new Hashtable(); + Map htAllocPaySource = new HashMap<>(); + Map htAllocPayAccounted = new HashMap<>(); for (MPayment payment : payList) { BigDecimal allocateSource = Env.ZERO; diff --git a/org.idempiere.test/src/org/idempiere/test/model/AllocationTest.java b/org.idempiere.test/src/org/idempiere/test/model/AllocationTest.java index ae648f37b4..c7e9af81d7 100644 --- a/org.idempiere.test/src/org/idempiere/test/model/AllocationTest.java +++ b/org.idempiere.test/src/org/idempiere/test/model/AllocationTest.java @@ -170,6 +170,58 @@ public class AllocationTest extends AbstractTestCase { rollback(); } + @Test + /** + * https://idempiere.atlassian.net/browse/IDEMPIERE-5723 + */ + public void testAllocateInvoiceArAp() { + MBPartner bpartner = MBPartner.get(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id); + + Timestamp date = TimeUtil.getDay(null); + MCurrency usd = MCurrency.get(DictionaryIDs.C_Currency.USD.id); // USD + + int payterm = DictionaryIDs.C_PaymentTerm.TWO_PERCENT_10_NET_30.id; //(2%10 Net 30) + int taxid = DictionaryIDs.C_Tax.CT_SALES.id; // (CT Sales, Rate 6) + + MInvoice invoiceAr = createInvoice(true, false, date, date, + bpartner.getC_BPartner_ID(), payterm, taxid, Env.ONEHUNDRED); + assertEquals(invoiceAr.getTotalLines().setScale(2, RoundingMode.HALF_UP), new BigDecimal("100.00")); + assertEquals(invoiceAr.getGrandTotal().setScale(2, RoundingMode.HALF_UP), new BigDecimal("106.00")); + + completeDocument(invoiceAr); + postDocument(invoiceAr); + + MInvoice invoiceAp = createInvoice(false, false, date, date, + bpartner.getC_BPartner_ID(), payterm, taxid, Env.ONEHUNDRED); + + completeDocument(invoiceAp); + postDocument(invoiceAp); + + + MAllocationHdr alloc = new MAllocationHdr(Env.getCtx(), true, date, usd.getC_Currency_ID(), Env.getContext(Env.getCtx(), "#AD_User_Name"), getTrxName()); + alloc.setAD_Org_ID(invoiceAr.getAD_Org_ID()); + int doctypeAlloc = MDocType.getDocType(MDocType.DOCBASETYPE_PaymentAllocation); + alloc.setC_DocType_ID(doctypeAlloc); + //alloc.setDescription(alloc.getDescriptionForManualAllocation(payment.getC_BPartner_ID(), getTrxName())); + alloc.saveEx(); + + MAllocationLine aLine1 = new MAllocationLine(alloc, invoiceAp.getOpenAmt(), Env.ZERO, Env.ZERO, Env.ZERO); + aLine1.setDocInfo(invoiceAp.getC_BPartner_ID(), 0, invoiceAp.getC_Invoice_ID()); + aLine1.saveEx(); + + MAllocationLine aLine2 = new MAllocationLine(alloc, invoiceAr.getOpenAmt(), Env.ZERO, Env.ZERO, Env.ZERO); + aLine2.setDocInfo(invoiceAr.getC_BPartner_ID(), 0, invoiceAr.getC_Invoice_ID()); + aLine2.saveEx(); + + completeDocument(alloc); + postDocument(alloc); + + alloc.load(getTrxName()); + + assertTrue(alloc.isPosted(), "Allocation not posted"); + + } + @Test public void testAllocateCustomerInvoice() { int severeCount = 0;