From af10c3a67463f5adf959b3ce7d2da399a8c609dc Mon Sep 17 00:00:00 2001 From: hengsin Date: Sun, 31 Mar 2024 19:00:09 +0800 Subject: [PATCH] IDEMPIERE-5847 Wrong Posting in Shipment (#2280) * IDEMPIERE-5847 Wrong Posting in Shipment * IDEMPIERE-5847 Wrong Posting in Shipment - minor unit test refinement * IDEMPIERE-5847 Wrong Posting in Shipment - Fix intermittent unit test error --- .../src/org/compiere/acct/Doc_InOut.java | 6 -- .../src/org/compiere/model/MFactAcct.java | 16 +++++ .../org/idempiere/test/base/InOutTest.java | 50 +++++++------- .../test/costing/AveragePOCostingTest.java | 68 +++++++++++++++++++ 4 files changed, 111 insertions(+), 29 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/acct/Doc_InOut.java b/org.adempiere.base/src/org/compiere/acct/Doc_InOut.java index f934e9c83e..ad07ee9e4a 100644 --- a/org.adempiere.base/src/org/compiere/acct/Doc_InOut.java +++ b/org.adempiere.base/src/org/compiere/acct/Doc_InOut.java @@ -219,8 +219,6 @@ public class Doc_InOut extends Doc { MInOutLineMA ma = mas[j]; BigDecimal QtyMA = ma.getMovementQty(); - if (QtyMA.signum() != line.getQty().signum()) - QtyMA = QtyMA.negate(); ProductCost pc = line.getProductCost(); pc.setQty(QtyMA); pc.setM_M_AttributeSetInstance_ID(ma.getM_AttributeSetInstance_ID()); @@ -451,8 +449,6 @@ public class Doc_InOut extends Doc { MInOutLineMA ma = mas[j]; BigDecimal QtyMA = ma.getMovementQty(); - if (QtyMA.signum() != line.getQty().signum()) - QtyMA = QtyMA.negate(); ProductCost pc = line.getProductCost(); pc.setQty(QtyMA); pc.setM_M_AttributeSetInstance_ID(ma.getM_AttributeSetInstance_ID()); @@ -964,8 +960,6 @@ public class Doc_InOut extends Doc { MInOutLineMA ma = mas[j]; BigDecimal QtyMA = ma.getMovementQty(); - if (QtyMA.signum() != line.getQty().signum()) - QtyMA = QtyMA.negate(); ProductCost pc = line.getProductCost(); pc.setQty(QtyMA); pc.setM_M_AttributeSetInstance_ID(ma.getM_AttributeSetInstance_ID()); diff --git a/org.adempiere.base/src/org/compiere/model/MFactAcct.java b/org.adempiere.base/src/org/compiere/model/MFactAcct.java index bc18bf5be4..1f179fbe8b 100644 --- a/org.adempiere.base/src/org/compiere/model/MFactAcct.java +++ b/org.adempiere.base/src/org/compiere/model/MFactAcct.java @@ -23,6 +23,7 @@ import java.util.logging.Level; import org.adempiere.exceptions.DBException; import org.compiere.util.CLogger; import org.compiere.util.DB; +import org.compiere.util.Env; /** * Accounting Fact Model @@ -147,4 +148,19 @@ public class MFactAcct extends X_Fact_Acct return acct; } // getMAccount + private final static String recordIdWhereClause = "AD_Table_ID=? AND Record_ID=? AND C_AcctSchema_ID=?"; + + /** + * Create Fact_Acct query for table and record id + * @param AD_Table_ID + * @param Record_ID + * @param C_AcctSchema_ID + * @param trxName + * @return query + */ + public static final Query createRecordIdQuery(int AD_Table_ID, int Record_ID, int C_AcctSchema_ID, String trxName) { + Query query = new Query(Env.getCtx(), Table_Name, recordIdWhereClause, trxName); + return query.setParameters(AD_Table_ID, Record_ID, C_AcctSchema_ID); + } + } // MFactAcct diff --git a/org.idempiere.test/src/org/idempiere/test/base/InOutTest.java b/org.idempiere.test/src/org/idempiere/test/base/InOutTest.java index 6e82e2d970..ea63d3a786 100644 --- a/org.idempiere.test/src/org/idempiere/test/base/InOutTest.java +++ b/org.idempiere.test/src/org/idempiere/test/base/InOutTest.java @@ -31,6 +31,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.math.BigDecimal; import java.math.RoundingMode; import java.sql.Timestamp; +import java.util.List; import org.compiere.acct.Doc; import org.compiere.acct.DocManager; @@ -56,6 +57,7 @@ import org.compiere.model.MShippingProcessor; import org.compiere.model.MWarehouse; import org.compiere.model.PO; import org.compiere.model.ProductCost; +import org.compiere.model.Query; import org.compiere.model.SystemIDs; import org.compiere.model.X_C_BP_ShippingAcct; import org.compiere.model.X_M_ShippingProcessorCfg; @@ -274,12 +276,9 @@ public class InOutTest extends AbstractTestCase { doc.setC_BPartner_ID(receipt.getC_BPartner_ID()); MAccount acctNIR = doc.getAccount(Doc.ACCTTYPE_NotInvoicedReceipts, as); - String whereClause = MFactAcct.COLUMNNAME_AD_Table_ID + "=" + MInOut.Table_ID - + " AND " + MFactAcct.COLUMNNAME_Record_ID + "=" + receipt.get_ID() - + " AND " + MFactAcct.COLUMNNAME_C_AcctSchema_ID + "=" + as.getC_AcctSchema_ID(); - int[] ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName()); - for (int id : ids) { - MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName()); + Query query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, receipt.get_ID(), as.getC_AcctSchema_ID(), getTrxName()); + List fas = query.list(); + for (MFactAcct fa : fas) { if (acctNIR.getAccount_ID() == fa.getAccount_ID()) { if (receiptLine.get_ID() == fa.getLine_ID()) { BigDecimal acctSource = orderLine.getPriceActual().multiply(receiptLine.getMovementQty()) @@ -344,12 +343,9 @@ public class InOutTest extends AbstractTestCase { doc.setC_BPartner_ID(delivery.getC_BPartner_ID()); MAccount acctNIR = doc.getAccount(Doc.ACCTTYPE_NotInvoicedReceipts, as); - String whereClause = MFactAcct.COLUMNNAME_AD_Table_ID + "=" + MInOut.Table_ID - + " AND " + MFactAcct.COLUMNNAME_Record_ID + "=" + delivery.get_ID() - + " AND " + MFactAcct.COLUMNNAME_C_AcctSchema_ID + "=" + as.getC_AcctSchema_ID(); - int[] ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName()); - for (int id : ids) { - MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName()); + Query query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, delivery.get_ID(), as.getC_AcctSchema_ID(), getTrxName()); + List fas = query.list(); + for (MFactAcct fa : fas) { if (acctNIR.getAccount_ID() == fa.getAccount_ID()) { if (deliveryLine.get_ID() == fa.getLine_ID()) { BigDecimal acctSource = orderLine.getPriceActual().multiply(deliveryLine.getMovementQty()) @@ -419,7 +415,10 @@ public class InOutTest extends AbstractTestCase { orderLine.setLine(line); orderLine.setProduct(product); orderLine.setQty(qty); - orderLine.setPrice(price); + if (price != null) + orderLine.setPrice(price); + else + orderLine.setPrice(); orderLine.saveEx(); return orderLine; } @@ -583,6 +582,15 @@ public class InOutTest extends AbstractTestCase { MProduct product = MProduct.get(Env.getCtx(), DictionaryIDs.M_Product.AZALEA_BUSH.id); Timestamp currentDate = Env.getContextAsDate(Env.getCtx(), "#Date"); + // make sure there's cost for AZALEA_BUSH + MBPartner vendor = MBPartner.get(Env.getCtx(), DictionaryIDs.C_BPartner.SEED_FARM.id); + MOrder purchaseOrder = createPurchaseOrder(vendor, currentDate, DictionaryIDs.M_PriceList.PURCHASE.id, DictionaryIDs.C_ConversionType.SPOT.id); + MOrderLine poLine = createOrderLine(purchaseOrder, 10, product, new BigDecimal("1"), null); + completeDocument(purchaseOrder); + MInOut receipt = createMMReceipt(purchaseOrder, currentDate); + createInOutLine(receipt, poLine, new BigDecimal("1")); + completeDocument(receipt); + MOrder order = createSalseOrder(bpartner, currentDate, DictionaryIDs.M_PriceList.STANDARD.id, DictionaryIDs.C_ConversionType.SPOT.id); int plv = MPriceList.get(DictionaryIDs.M_PriceList.STANDARD.id).getPriceListVersion(currentDate).get_ID(); BigDecimal price = MProductPrice.get(Env.getCtx(), plv, product.get_ID(), getTrxName()).getPriceStd(); @@ -600,15 +608,12 @@ public class InOutTest extends AbstractTestCase { MAccount cogs = pc.getAccount(ProductCost.ACCTTYPE_P_Cogs, as); MAccount asset = pc.getAccount(ProductCost.ACCTTYPE_P_Asset, as); - String whereClause = MFactAcct.COLUMNNAME_AD_Table_ID + "=" + MInOut.Table_ID - + " AND " + MFactAcct.COLUMNNAME_Record_ID + "=" + delivery.get_ID() - + " AND " + MFactAcct.COLUMNNAME_C_AcctSchema_ID + "=" + as.getC_AcctSchema_ID(); - int[] ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName()); - assertTrue(ids.length > 0, "Failed to retrieve fact posting entries for shipment document"); + Query query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, delivery.get_ID(), as.getC_AcctSchema_ID(), getTrxName()); + List fas = query.list(); + assertTrue(fas.size() > 0, "Failed to retrieve fact posting entries for shipment document"); boolean cogsFound = false; boolean assetFound = false; - for (int id : ids) { - MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName()); + for (MFactAcct fa : fas) { if (cogs.getAccount_ID() == fa.getAccount_ID()) { if (deliveryLine.get_ID() == fa.getLine_ID()) { assertEquals(fa.getAmtSourceDr().abs().toPlainString(), fa.getAmtSourceDr().toPlainString(), "Not DR COGS"); @@ -628,11 +633,10 @@ public class InOutTest extends AbstractTestCase { //re-post repostDocument(delivery); - ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName()); + fas = query.list(); cogsFound = false; assetFound = false; - for (int id : ids) { - MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName()); + for (MFactAcct fa : fas) { if (cogs.getAccount_ID() == fa.getAccount_ID()) { if (deliveryLine.get_ID() == fa.getLine_ID()) { assertEquals(fa.getAmtSourceDr().abs().toPlainString(), fa.getAmtSourceDr().toPlainString(), "Not DR COGS"); diff --git a/org.idempiere.test/src/org/idempiere/test/costing/AveragePOCostingTest.java b/org.idempiere.test/src/org/idempiere/test/costing/AveragePOCostingTest.java index 266a5008db..5d3fc2fb0b 100644 --- a/org.idempiere.test/src/org/idempiere/test/costing/AveragePOCostingTest.java +++ b/org.idempiere.test/src/org/idempiere/test/costing/AveragePOCostingTest.java @@ -35,6 +35,9 @@ import java.math.RoundingMode; import java.sql.Timestamp; import java.util.List; +import org.compiere.acct.Doc; +import org.compiere.acct.DocManager; +import org.compiere.model.MAccount; import org.compiere.model.MAcctSchema; import org.compiere.model.MAttributeSet; import org.compiere.model.MAttributeSetExclude; @@ -44,6 +47,7 @@ import org.compiere.model.MClient; import org.compiere.model.MCost; import org.compiere.model.MCostDetail; import org.compiere.model.MCostElement; +import org.compiere.model.MFactAcct; import org.compiere.model.MInOut; import org.compiere.model.MInOutLine; import org.compiere.model.MInOutLineMA; @@ -64,6 +68,8 @@ import org.compiere.model.MProductionLine; import org.compiere.model.MProject; import org.compiere.model.MProjectIssue; import org.compiere.model.MStorageOnHand; +import org.compiere.model.ProductCost; +import org.compiere.model.Query; import org.compiere.process.DocAction; import org.compiere.process.DocumentEngine; import org.compiere.process.ProcessInfo; @@ -574,6 +580,35 @@ public class AveragePOCostingTest extends AbstractTestCase { assertNotNull(cost2, "MCost record not found"); assertEquals(new BigDecimal("3.00"), cost2.getCurrentCostPrice().setScale(2, RoundingMode.HALF_UP)); + //check posting + ProductCost pc = new ProductCost(Env.getCtx(), line1.getM_Product_ID(), line1.getM_AttributeSetInstance_ID(), getTrxName()); + MAccount asset = pc.getAccount(ProductCost.ACCTTYPE_P_Asset, as); + Doc doc = DocManager.getDocument(as, MInOut.Table_ID, line1.getM_InOut_ID(), getTrxName()); + doc.setC_BPartner_ID(line1.getParent().getC_BPartner_ID()); + MAccount acctNIR = doc.getAccount(Doc.ACCTTYPE_NotInvoicedReceipts, as); + Query query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, line1.getM_InOut_ID(), as.getC_AcctSchema_ID(), getTrxName()); + List fas = query.list(); + assertTrue(fas.size() > 0, "Failed to retrieve fact posting entries for shipment document"); + boolean nirFound = false; + boolean assetFound = false; + for (MFactAcct fa : fas) { + if (asset.getAccount_ID() == fa.getAccount_ID()) { + if (line1.get_ID() == fa.getLine_ID()) { + assertEquals(fa.getAmtSourceDr().abs().toPlainString(), fa.getAmtSourceDr().toPlainString(), "Not DR Asset"); + assertTrue(fa.getAmtSourceDr().signum() > 0, "Not DR Asset"); + } + assetFound = true; + } else if (acctNIR.getAccount_ID() == fa.getAccount_ID()) { + if (line1.get_ID() == fa.getLine_ID()) { + assertEquals(fa.getAmtSourceCr().abs().toPlainString(), fa.getAmtSourceCr().toPlainString(), "Not CR Not Invoiced Receipt"); + assertTrue(fa.getAmtSourceCr().signum() > 0, "Not CR Not Invoiced Receipt"); + } + nirFound = true; + } + } + assertTrue(nirFound, "No Not Invoiced Receipt posting found"); + assertTrue(assetFound, "No Product Asset posting found"); + //shipment MOrder order = new MOrder(Env.getCtx(), 0, getTrxName()); order.setBPartner(MBPartner.get(Env.getCtx(), DictionaryIDs.C_BPartner.JOE_BLOCK.id)); @@ -617,6 +652,39 @@ public class AveragePOCostingTest extends AbstractTestCase { assertEquals(2, linema.length, "Unexpected number of MInOutLineMA records"); assertEquals(linema[0].getM_AttributeSetInstance_ID(), asi1.get_ID(), "Unexpected M_AttributeSetInstance_ID for MInOutLineMA 1"); assertEquals(linema[1].getM_AttributeSetInstance_ID(), asi2.get_ID(), "Unexpected M_AttributeSetInstance_ID for MInOutLineMA 2"); + assertEquals(1, linema[0].getMovementQty().intValue(), "Unexpected MovementQty for MInOutLineMA 1"); + assertEquals(1, linema[1].getMovementQty().intValue(), "Unexpected MovementQty for MInOutLineMA 2"); + + // check posting + if (!shipment.isPosted()) { + String error = DocumentEngine.postImmediate(Env.getCtx(), shipment.getAD_Client_ID(), shipment.get_Table_ID(), shipment.get_ID(), false, getTrxName()); + assertTrue(error == null, error); + } + pc = new ProductCost(Env.getCtx(), shipmentLine.getM_Product_ID(), shipmentLine.getM_AttributeSetInstance_ID(), getTrxName()); + MAccount cogs = pc.getAccount(ProductCost.ACCTTYPE_P_Cogs, as); + asset = pc.getAccount(ProductCost.ACCTTYPE_P_Asset, as); + query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, shipment.getM_InOut_ID(), as.getC_AcctSchema_ID(), getTrxName()); + fas = query.list(); + assertTrue(fas.size() > 0, "Failed to retrieve fact posting entries for shipment document"); + boolean cogsFound = false; + assetFound = false; + for (MFactAcct fa : fas) { + if (cogs.getAccount_ID() == fa.getAccount_ID()) { + if (shipmentLine.get_ID() == fa.getLine_ID()) { + assertEquals(fa.getAmtSourceDr().abs().toPlainString(), fa.getAmtSourceDr().toPlainString(), "Not DR COGS"); + assertTrue(fa.getAmtSourceDr().signum() > 0, "Not DR COGS"); + } + cogsFound = true; + } else if (asset.getAccount_ID() == fa.getAccount_ID()) { + if (shipmentLine.get_ID() == fa.getLine_ID()) { + assertEquals(fa.getAmtSourceCr().abs().toPlainString(), fa.getAmtSourceCr().toPlainString(), "Not CR Product Asset"); + assertTrue(fa.getAmtSourceCr().signum() > 0, "Not CR Product Asset"); + } + assetFound = true; + } + } + assertTrue(cogsFound, "No COGS posting found"); + assertTrue(assetFound, "No Product Asset posting found"); cd = MCostDetail.get(Env.getCtx(), "M_InOutLine_ID=?", shipmentLine.getM_InOutLine_ID(), linema[0].getM_AttributeSetInstance_ID(), as.get_ID(), getTrxName()); assertNotNull(cd, "MCostDetail not found for order line1");