IDEMPIERE-5878 Wrong GL posting in allocation for payment REVERSE/ACCRUAL, if payment WITH charge (#2074)

org.adempiere.base/src/org/compiere/acct/Doc_AllocationHdr.java

@@ -32,6 +32,7 @@ import org.compiere.model.MAcctSchemaElement;
 import org.compiere.model.MAllocationHdr;
 import org.compiere.model.MAllocationLine;
 import org.compiere.model.MCashLine;
+import org.compiere.model.MCharge;
 import org.compiere.model.MConversionRate;
 import org.compiere.model.MDocType;
 import org.compiere.model.MFactAcct;
@@ -687,7 +688,9 @@ public class Doc_AllocationHdr extends Doc
 		//	or Doc.ACCTTYPE_PaymentSelect (AP) or V_Prepayment
 		int accountType = Doc.ACCTTYPE_UnallocatedCash;
 		//
-		String sql = "SELECT p.C_BankAccount_ID, d.DocBaseType, p.IsReceipt, p.IsPrepayment "
+		int C_Charge_ID = 0;
+		
+		String sql = "SELECT p.C_BankAccount_ID, d.DocBaseType, p.IsReceipt, p.IsPrepayment, p.C_Charge_ID "
 				+ "FROM C_Payment p INNER JOIN C_DocType d ON (p.C_DocType_ID=d.C_DocType_ID) "
 				+ "WHERE C_Payment_ID=?";
 		PreparedStatement pstmt = null;
@@ -700,6 +703,7 @@ public class Doc_AllocationHdr extends Doc
 			if (rs.next ())
 			{
 				setC_BankAccount_ID(rs.getInt(1));
+				C_Charge_ID = rs.getInt(5);				// Charge
 				if (DOCTYPE_APPayment.equals(rs.getString(2)))
 					accountType = Doc.ACCTTYPE_PaymentSelect;
 				//	Prepayment
@@ -728,6 +732,9 @@ public class Doc_AllocationHdr extends Doc
 			log.log(Level.SEVERE, "NONE for C_Payment_ID=" + C_Payment_ID);
 			return null;
 		}
+		
+		if (C_Charge_ID != 0)
+			return MCharge.getAccount(C_Charge_ID, as);
 		return getAccount (accountType, as);
 	}	//	getPaymentAcct
 

org.idempiere.test/src/org/idempiere/test/model/Allocation2ndAcctSchemaTest.java

@@ -1881,6 +1881,68 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			deleteConversionRate(cr);
 		}
 	}
+	
+	@Test
+	@ResourceLock(value = MConversionRate.Table_Name)
+	/**
+	 * Test the allocation posting (different period + reversal)
+	 * Payment with Charge Total=1000, Period 1
+	 * Payment with Charge Total=1000, Period 2 (Reversed)
+	 * https://idempiere.atlassian.net/browse/IDEMPIERE-5878
+	 */
+	public void testAllocatePaymentPosting_5() {
+		MBPartner bpartner = MBPartner.get(Env.getCtx(), DictionaryIDs.C_BPartner.AGRI_TECH.id); 
+		Timestamp currentDate = Env.getContextAsDate(Env.getCtx(), "#Date");
+
+		Calendar cal = Calendar.getInstance();
+		cal.setTimeInMillis(currentDate.getTime());
+		cal.add(Calendar.DAY_OF_MONTH, -1);
+		Timestamp date1 = new Timestamp(cal.getTimeInMillis());
+		Timestamp date2 = currentDate;
+		
+		int C_ConversionType_ID = DictionaryIDs.C_ConversionType.COMPANY.id; // Company
+		
+		MCurrency usd = MCurrency.get(DictionaryIDs.C_Currency.USD.id); // USD
+		MCurrency euro = MCurrency.get(DictionaryIDs.C_Currency.EUR.id); // EUR
+		BigDecimal eurToUsd1 = new BigDecimal(4);
+		MConversionRate cr1 = createConversionRate(euro.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date1, eurToUsd1);
+		
+		BigDecimal eurToUsd2 = new BigDecimal(5);
+		MConversionRate cr2 = createConversionRate(euro.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date2, eurToUsd2);
+		
+		try {
+			MBankAccount ba = getBankAccount(usd.getC_Currency_ID());
+			BigDecimal payAmt = new BigDecimal(1000);
+			MPayment payment = createPayment(true, bpartner, ba.getC_BankAccount_ID(), date1, payAmt, euro.getC_Currency_ID(), C_ConversionType_ID);
+			payment.setC_Charge_ID(DictionaryIDs.C_Charge.BANK.id); // Bank Charge
+			payment.saveEx();
+			completeDocument(payment);
+			postDocument(payment);
+			
+			reverseDocument(payment, false);
+			MPayment reversalPayment = new MPayment(Env.getCtx(), payment.getReversal_ID(), getTrxName());
+			postDocument(reversalPayment);
+			
+			MAcctSchema[] ass = MAcctSchema.getClientAcctSchema(Env.getCtx(), Env.getAD_Client_ID(Env.getCtx()));
+			MAllocationHdr[] allocList = MAllocationHdr.getOfPayment(Env.getCtx(), payment.getC_Payment_ID(), getTrxName());
+			
+			BigDecimal allocAmount = payAmt;
+			ArrayList<PostingLine> paymentLineList = new ArrayList<PostingLine>();
+			ArrayList<PostingLine> gainLossLineList = new ArrayList<PostingLine>();
+			BigDecimal accountedDrAmt = getAccountedAmount(usd, allocAmount, cr1.getMultiplyRate());
+			BigDecimal accountedCrAmt = getAccountedAmount(usd, allocAmount, cr2.getMultiplyRate());
+			paymentLineList.add(new PostingLine(usd, accountedDrAmt, Env.ZERO));
+			paymentLineList.add(new PostingLine(usd, Env.ZERO, accountedCrAmt));
+			BigDecimal gainLossAmt = new BigDecimal(1000).setScale(usd.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(usd, gainLossAmt, Env.ZERO));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, null, gainLossLineList, null);
+		} finally {
+			rollback();
+			deleteConversionRate(cr1);
+			deleteConversionRate(cr2);
+		}
+	}
 		
 	private MConversionRate createConversionRate(int C_Currency_ID, int C_Currency_ID_To, int C_ConversionType_ID, 
 			Timestamp date, BigDecimal rate) {
@@ -2072,11 +2134,13 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 
 			int C_BPartner_ID = 0;
 			int C_BankAccount_ID = 0;
+			int C_Charge_ID = 0;
 			MAllocationLine[] lines = alloc.getLines(false);
 			for (MAllocationLine line : lines) {
 				if (line.getC_Payment_ID() > 0) {
 					C_BPartner_ID = line.getC_Payment().getC_BPartner_ID();
 					C_BankAccount_ID = line.getC_Payment().getC_BankAccount_ID();
+					C_Charge_ID = line.getC_Payment().getC_Charge_ID();
 					break;
 				}
 			}
@@ -2101,6 +2165,10 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 				if (C_BPartner_ID > 0)
 					doc.setC_BPartner_ID(C_BPartner_ID);
 				
+				MAccount acctCharge = null;
+				if (C_Charge_ID != 0)
+					acctCharge = MCharge.getAccount(C_Charge_ID, as);
+				
 				MAccount acctUnallocatedCash = doc.getAccount(Doc.ACCTTYPE_UnallocatedCash, as);
 				MAccount acctReceivable = doc.getAccount(Doc.ACCTTYPE_C_Receivable, as);
 				MAccount acctLiability = doc.getAccount(Doc.ACCTTYPE_V_Liability, as);
@@ -2119,7 +2187,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 				int[] ids = MFactAcct.getAllIDs(MFactAcct.Table_Name, whereClause, getTrxName());
 				for (int id : ids) {
 					MFactAcct fa = new MFactAcct(Env.getCtx(), id, getTrxName());
-					if (acctUnallocatedCash != null && acctUnallocatedCash.getAccount_ID() == fa.getAccount_ID())
+					if (C_Charge_ID != 0 && acctCharge != null && acctCharge.getAccount_ID() == fa.getAccount_ID())
+						totalPaymentAmtAcct = totalPaymentAmtAcct.add(fa.getAmtAcctDr()).subtract(fa.getAmtAcctCr());
+					else if (C_Charge_ID == 0 && acctUnallocatedCash != null && acctUnallocatedCash.getAccount_ID() == fa.getAccount_ID())
 						totalPaymentAmtAcct = totalPaymentAmtAcct.add(fa.getAmtAcctDr()).subtract(fa.getAmtAcctCr());
 					else if ((acctReceivable != null && acctReceivable.getAccount_ID() == fa.getAccount_ID()) || 
 							(acctLiability != null && acctLiability.getAccount_ID() == fa.getAccount_ID()))