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

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());

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

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 
+				Query query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, receipt.get_ID(), as.getC_AcctSchema_ID(), getTrxName());
-						+ " AND " + MFactAcct.COLUMNNAME_Record_ID + "=" + receipt.get_ID()
+				List<MFactAcct> fas = query.list();
-						+ " AND " + MFactAcct.COLUMNNAME_C_AcctSchema_ID + "=" + as.getC_AcctSchema_ID();
+				for (MFactAcct fa : fas) {
-				int[] ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName());
-				for (int id : ids) {
-					MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName());
 					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 
+				Query query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, delivery.get_ID(), as.getC_AcctSchema_ID(), getTrxName());
-						+ " AND " + MFactAcct.COLUMNNAME_Record_ID + "=" + delivery.get_ID()
+				List<MFactAcct> fas = query.list();
-						+ " AND " + MFactAcct.COLUMNNAME_C_AcctSchema_ID + "=" + as.getC_AcctSchema_ID();
+				for (MFactAcct fa : fas) {
-				int[] ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName());
-				for (int id : ids) {
-					MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName());
 					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);
+		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 
+		Query query = MFactAcct.createRecordIdQuery(MInOut.Table_ID, delivery.get_ID(), as.getC_AcctSchema_ID(), getTrxName());
-				+ " AND " + MFactAcct.COLUMNNAME_Record_ID + "=" + delivery.get_ID()
+		List<MFactAcct> fas = query.list();
-				+ " AND " + MFactAcct.COLUMNNAME_C_AcctSchema_ID + "=" + as.getC_AcctSchema_ID();
+		assertTrue(fas.size() > 0, "Failed to retrieve fact posting entries for shipment document");
-		int[] ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName());
-		assertTrue(ids.length > 0, "Failed to retrieve fact posting entries for shipment document");
 		boolean cogsFound = false;
 		boolean assetFound = false;
-		for (int id : ids) {
+		for (MFactAcct fa : fas) {
-			MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName());
 			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) {
+		for (MFactAcct fa : fas) {
-			MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName());
 			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");

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<MFactAcct> 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");