IDEMPIERE-5339 GL AP/AR and RLL/RLG facts posted on wrong side in allocation reversal document using Payment Reverse-Accrual (#1420)

* IDEMPIERE-5339 GL AP/AR and RLL/RLG facts posted on wrong side in allocation reversal document using Payment Reverse-Accrual

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

@@ -842,10 +842,7 @@ public class Doc_AllocationHdr extends Doc
 		if (valuesInv != null && valuesInv.size() >= 4) {
 			if (invoice.getReversal_ID() == 0 || invoice.get_ID() < invoice.getReversal_ID())
 			{
-				if ((invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) 
+				if (hasDebitTradeAmt(invoice)) {
-						|| (invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && invoice.isCreditMemo())
-						|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && invoice.isCreditMemo())
-						|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && !invoice.isCreditMemo())) {
 					invoiceSource = (BigDecimal) valuesInv.get(0); // AmtSourceDr
 					invoiceAccounted = (BigDecimal) valuesInv.get(1); // AmtAcctDr
 				} else {
@@ -855,10 +852,7 @@ public class Doc_AllocationHdr extends Doc
 			}
 			else
 			{
-				if ((invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) 
+				if (hasDebitTradeAmt(invoice)) {
-						|| (invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && invoice.isCreditMemo())
-						|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && invoice.isCreditMemo())
-						|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && !invoice.isCreditMemo())) {
 					invoiceSource = (BigDecimal) valuesInv.get(2); // AmtSourceCr
 					invoiceAccounted = (BigDecimal) valuesInv.get(3); // AmtAcctCr
 				} else {
@@ -915,10 +909,10 @@ public class Doc_AllocationHdr extends Doc
 		MAccount loss = MAccount.get (as.getCtx(), as.getAcctSchemaDefault().getRealizedLoss_Acct());
 		//
 
-		if ((invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) 
+		MAllocationHdr alloc = (MAllocationHdr) getPO();
-				|| (invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && invoice.isCreditMemo())
+		if (alloc.getReversal_ID() == 0 || alloc.get_ID() < alloc.getReversal_ID())
-				|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && invoice.isCreditMemo())
+		{
-				|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && !invoice.isCreditMemo()))
+			if (hasDebitTradeAmt(invoice))
 			{
 				FactLine fl = fact.createLine (line, loss, gain, as.getC_Currency_ID(), acctDifference);
 				fl.setDescription(description.toString());
@@ -934,6 +928,26 @@ public class Doc_AllocationHdr extends Doc
 				fl.setDescription(description.toString());
 				invGainLossFactLines.add(fl);
 			}
+		}
+		else
+		{
+			if (hasDebitTradeAmt(invoice))
+			{
+				FactLine fl = fact.createLine (line, acct, as.getC_Currency_ID(), acctDifference);
+				fl.setDescription(description.toString());
+				fl = fact.createLine (line, gain, loss, as.getC_Currency_ID(), acctDifference.negate());
+				fl.setDescription(description.toString());
+				invGainLossFactLines.add(fl);
+			}
+			else
+			{
+				FactLine fl = fact.createLine (line, gain, loss, as.getC_Currency_ID(), acctDifference);
+				fl.setDescription(description.toString());
+				invGainLossFactLines.add(fl);
+				fl = fact.createLine (line, acct, as.getC_Currency_ID(), acctDifference.negate());
+				fl.setDescription(description.toString());
+			}
+		}
 		return null;
 	}
 	
@@ -1088,10 +1102,7 @@ public class Doc_AllocationHdr extends Doc
 				BigDecimal invoiceAccounted = null;
 				if (invoice.getReversal_ID() == 0 || invoice.get_ID() < invoice.getReversal_ID())
 				{
-					if ((invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) 
+					if (hasDebitTradeAmt(invoice)) {
-							|| (invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && invoice.isCreditMemo())
-							|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && invoice.isCreditMemo())
-							|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && !invoice.isCreditMemo())) {
 						invoiceSource = (BigDecimal) valuesInv.get(0); // AmtSourceDr
 						invoiceAccounted = (BigDecimal) valuesInv.get(1); // AmtAcctDr
 					} else {
@@ -1101,10 +1112,7 @@ public class Doc_AllocationHdr extends Doc
 				}
 				else
 				{
-					if ((invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) 
+					if (hasDebitTradeAmt(invoice)) {
-							|| (invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && invoice.isCreditMemo())
-							|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && invoice.isCreditMemo())
-							|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && !invoice.isCreditMemo())) {
 						invoiceSource = (BigDecimal) valuesInv.get(2); // AmtSourceCr
 						invoiceAccounted = (BigDecimal) valuesInv.get(3); // AmtAcctCr
 					} else {
@@ -1355,10 +1363,10 @@ public class Doc_AllocationHdr extends Doc
 			
 			//
 			Integer C_AllocationLine_ID = htInvAllocLine.get(invoice.getC_Invoice_ID());
-			if ((invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) 
+			MAllocationHdr alloc = (MAllocationHdr) getPO();
-					|| (invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && invoice.isCreditMemo())
+			if (alloc.getReversal_ID() == 0 || alloc.get_ID() < alloc.getReversal_ID())
-					|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && invoice.isCreditMemo())
+			{
-					|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && !invoice.isCreditMemo()))
+				if (hasDebitTradeAmt(invoice))
 				{
 					FactLine fl = fact.createLine (null, acct, as.getC_Currency_ID(), acctDifference);
 					fl.setDescription(description.toString());
@@ -1389,6 +1397,40 @@ public class Doc_AllocationHdr extends Doc
 					}
 				}
 			}
+			else
+			{
+				if (hasDebitTradeAmt(invoice))
+				{
+					FactLine fl = fact.createLine (null, acct, as.getC_Currency_ID(), acctDifference.negate());
+					fl.setDescription(description.toString());
+					fl.setLine_ID(C_AllocationLine_ID == null ? 0 : C_AllocationLine_ID);
+					if (!fact.isAcctBalanced())
+					{
+						if (as.isCurrencyBalancing() && as.getC_Currency_ID() != invoice.getC_Currency_ID())
+							fl = fact.createLine (null, as.getCurrencyBalancing_Acct(), as.getC_Currency_ID(), acctDifference);
+						else
+							fl = fact.createLine (null, gain, loss, as.getC_Currency_ID(), acctDifference);
+						fl.setDescription(description.toString());
+						fl.setLine_ID(C_AllocationLine_ID == null ? 0 : C_AllocationLine_ID);
+					}
+				}
+				else
+				{
+					FactLine fl = fact.createLine (null, acct, as.getC_Currency_ID(), acctDifference);
+					fl.setDescription(description.toString());
+					fl.setLine_ID(C_AllocationLine_ID == null ? 0 : C_AllocationLine_ID);
+					if (!fact.isAcctBalanced())
+					{
+						if (as.isCurrencyBalancing() && as.getC_Currency_ID() != invoice.getC_Currency_ID())
+							fl = fact.createLine (null, as.getCurrencyBalancing_Acct(), as.getC_Currency_ID(), acctDifference.negate());
+						else 
+							fl = fact.createLine (null, gain, loss, as.getC_Currency_ID(), acctDifference.negate());	
+						fl.setDescription(description.toString());
+						fl.setLine_ID(C_AllocationLine_ID == null ? 0 : C_AllocationLine_ID);
+					}
+				}
+			}
+		}
 		return null;				
 	}	//	createInvoiceRounding
 
@@ -1755,6 +1797,19 @@ public class Doc_AllocationHdr extends Doc
 		}
 		return line;
 	}
+	
+	/**
+	 * Has Debit Receivables/Payables Trade Amount
+	 * @param invoice
+	 * @return
+	 */
+	private boolean hasDebitTradeAmt(MInvoice invoice)
+	{
+		return (invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && !invoice.isCreditMemo()) 
+				|| (invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && invoice.isCreditMemo())
+				|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() >= 0 && invoice.isCreditMemo())
+				|| (!invoice.isSOTrx() && invoice.getGrandTotal().signum() < 0 && !invoice.isCreditMemo());
+	}
 }   //  Doc_Allocation
 
 /**

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

@@ -51,6 +51,8 @@ import org.compiere.model.MFactAcct;
 import org.compiere.model.MInvoice;
 import org.compiere.model.MInvoiceLine;
 import org.compiere.model.MPayment;
+import org.compiere.model.MPriceList;
+import org.compiere.model.MPriceListVersion;
 import org.compiere.model.MProduct;
 import org.compiere.model.PO;
 import org.compiere.model.Query;
@@ -169,10 +171,10 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, currBalList);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
 			deleteConversionRate(cr3);
-			rollback();
 		}
 	}
 	
@@ -257,8 +259,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, null);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 	
@@ -327,9 +329,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -388,8 +390,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, null);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 	
@@ -536,9 +538,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, currBalLineList);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -674,8 +676,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, null);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 	
@@ -774,9 +776,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, currBalLineList);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -865,8 +867,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, currBalLineList);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 	
@@ -963,9 +965,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, currBalLineList);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -1052,8 +1054,334 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, null, currBalLineList);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
+		}
+	}
+	
+	@Test
+	/**
+	 * Test the allocation posting (different period + reversal)
+	 * Invoice Total=1000, Period 1
+	 * Payment1 Total=1000, Period 2
+	 * Payment2 Total=1000, Period 3 (Reversal)
+	 * https://idempiere.atlassian.net/browse/IDEMPIERE-5339
+	 */
+	public void testAllocateInvoicePaymentPosting_11() {
+		MBPartner bpartner = MBPartner.get(Env.getCtx(), DictionaryIDs.C_BPartner.C_AND_W.id); // C&W Construction
+		MCharge charge = MCharge.get(Env.getCtx(), DictionaryIDs.C_Charge.FREIGHT.id); // Freight Charges
+		Timestamp currentDate = Env.getContextAsDate(Env.getCtx(), "#Date");
+		
+		Calendar cal = Calendar.getInstance();
+		cal.setTimeInMillis(currentDate.getTime());
+		cal.add(Calendar.DAY_OF_MONTH, -2);
+		Timestamp date1 = new Timestamp(cal.getTimeInMillis());
+		
+		cal.setTimeInMillis(currentDate.getTime());
+		cal.add(Calendar.DAY_OF_MONTH, -1);
+		Timestamp date2 = new Timestamp(cal.getTimeInMillis());
+		
+		Timestamp date3 = currentDate;
+		
+		int C_ConversionType_ID = DictionaryIDs.C_ConversionType.SPOT.id; // Spot
+		
+		MCurrency aud = MCurrency.get(DictionaryIDs.C_Currency.AUD.id); // AUD
+		MCurrency eur = MCurrency.get(DictionaryIDs.C_Currency.EUR.id); // EUR
+		MCurrency usd = MCurrency.get(DictionaryIDs.C_Currency.USD.id); // USD
+		BigDecimal audToEur = new BigDecimal(0.7);
+		MConversionRate cr1a = createConversionRate(aud.getC_Currency_ID(), eur.getC_Currency_ID(), C_ConversionType_ID, date1, audToEur, true);
+		BigDecimal audToUsd = new BigDecimal(0.8);
+		MConversionRate cr1b = createConversionRate(aud.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date1, audToUsd, true);
+		
+		audToEur = new BigDecimal(0.8);
+		MConversionRate cr2a = createConversionRate(aud.getC_Currency_ID(), eur.getC_Currency_ID(), C_ConversionType_ID, date2, audToEur, true);
+		audToUsd = new BigDecimal(0.9);
+		MConversionRate cr2b = createConversionRate(aud.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date2, audToUsd, true);
+		
+		audToEur = new BigDecimal(0.8);
+		MConversionRate cr3a = createConversionRate(aud.getC_Currency_ID(), eur.getC_Currency_ID(), C_ConversionType_ID, date3, audToEur, true);
+		audToUsd = new BigDecimal(0.9);
+		MConversionRate cr3b = createConversionRate(aud.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date3, audToUsd, true);
+		
+		MPriceList priceList = new MPriceList(Env.getCtx(), 0, null);
+		priceList.setName("Export AUD " + System.currentTimeMillis());
+		MCurrency australianDollar = MCurrency.get(DictionaryIDs.C_Currency.AUD.id); // Australian Dollar (AUD)
+		priceList.setC_Currency_ID(australianDollar.getC_Currency_ID());
+		priceList.setPricePrecision(australianDollar.getStdPrecision());
+		priceList.saveEx();
+		
+		MPriceListVersion plv = new MPriceListVersion(priceList);
+		plv.setM_DiscountSchema_ID(DictionaryIDs.M_DiscountSchema.SALES_2001.id); // Sales 2001
+		plv.setValidFrom(currentDate);
+		plv.saveEx();
+		
+		try {
+			MBankAccount ba = getBankAccount(usd.getC_Currency_ID());
+			MInvoice invoice = createInvoice(true, bpartner, date1, priceList.getM_PriceList_ID(), C_ConversionType_ID);
+			BigDecimal qty = BigDecimal.ONE;
+			BigDecimal price = new BigDecimal(1000);
+			createInvoiceLine(invoice, 10, null, charge, qty, price);
+			completeDocument(invoice);
+			postDocument(invoice);
+			
+			BigDecimal payAmt = new BigDecimal(1000);
+			MPayment payment = createPayment(true, bpartner, ba.getC_BankAccount_ID(), date2, payAmt, aud.getC_Currency_ID(), C_ConversionType_ID);
+			completeDocument(payment);
+			postDocument(payment);
+			
+			MAllocationHdr alloc = createAllocationHdr(date2, aud.getC_Currency_ID());
+			BigDecimal allocAmount = new BigDecimal(1000);
+			createAllocationLine(alloc, allocAmount, invoice, payment);
+			completeDocument(alloc);
+			postDocument(alloc);
+			
+			MAcctSchema[] ass = MAcctSchema.getClientAcctSchema(Env.getCtx(), Env.getAD_Client_ID(Env.getCtx()));
+			MAllocationHdr[] allocList = MAllocationHdr.getOfPayment(Env.getCtx(), payment.getC_Payment_ID(), getTrxName());
+			
+			ArrayList<PostingLine> paymentLineList = new ArrayList<PostingLine>();
+			ArrayList<PostingLine> tradeLineList = new ArrayList<PostingLine>();
+			ArrayList<PostingLine> gainLossLineList = new ArrayList<PostingLine>();
+			BigDecimal accountedDrAmt = getAccountedAmount(usd, allocAmount, cr2b.getMultiplyRate());
+			BigDecimal accountedCrAmt = getAccountedAmount(usd, allocAmount, cr2b.getMultiplyRate());
+			paymentLineList.add(new PostingLine(usd, accountedDrAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(usd, Env.ZERO, accountedCrAmt));
+			BigDecimal gainLossAmt = new BigDecimal(100).setScale(usd.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(usd, Env.ZERO, gainLossAmt));
+			tradeLineList.add(new PostingLine(usd, gainLossAmt, Env.ZERO));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+			
+			paymentLineList = new ArrayList<PostingLine>();
+			tradeLineList = new ArrayList<PostingLine>();
+			gainLossLineList = new ArrayList<PostingLine>();
+			accountedDrAmt = getAccountedAmount(eur, allocAmount, cr2a.getMultiplyRate());
+			accountedCrAmt = getAccountedAmount(eur, allocAmount, cr2a.getMultiplyRate());
+			paymentLineList.add(new PostingLine(eur, accountedDrAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(eur, Env.ZERO, accountedCrAmt));
+			gainLossAmt = new BigDecimal(100).setScale(eur.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(eur, Env.ZERO, gainLossAmt));
+			tradeLineList.add(new PostingLine(eur, gainLossAmt, Env.ZERO));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+			
+			reverseDocument(payment, false);
+			MPayment reversalPayment = new MPayment(Env.getCtx(), payment.getReversal_ID(), getTrxName());
+			postDocument(reversalPayment);
+			
+			MAllocationHdr reversalAlloc = null;
+			allocList = MAllocationHdr.getOfPayment(Env.getCtx(), payment.getC_Payment_ID(), getTrxName());
+			for (MAllocationHdr allocation : allocList) {
+				if (allocation.getReversal_ID() > 0 && allocation.getReversal_ID() < allocation.get_ID()) {
+					reversalAlloc = allocation;
+					break;
+				}
+			}
+			
+			if (reversalAlloc == null)
+				reversalAlloc = new MAllocationHdr(Env.getCtx(), alloc.getReversal_ID(), getTrxName());
+		
+			allocList = new MAllocationHdr[] { reversalAlloc };
+			
+			paymentLineList = new ArrayList<PostingLine>();
+			tradeLineList = new ArrayList<PostingLine>();
+			gainLossLineList = new ArrayList<PostingLine>();
+			accountedDrAmt = getAccountedAmount(usd, allocAmount, cr3b.getMultiplyRate());
+			accountedCrAmt = getAccountedAmount(usd, allocAmount, cr3b.getMultiplyRate());
+			tradeLineList.add(new PostingLine(usd, accountedDrAmt, Env.ZERO));
+			paymentLineList.add(new PostingLine(usd, Env.ZERO, accountedCrAmt));
+			gainLossAmt = new BigDecimal(100).setScale(usd.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(usd, gainLossAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(usd, Env.ZERO, gainLossAmt));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+			
+			paymentLineList = new ArrayList<PostingLine>();
+			tradeLineList = new ArrayList<PostingLine>();
+			gainLossLineList = new ArrayList<PostingLine>();
+			accountedDrAmt = getAccountedAmount(eur, allocAmount, cr3a.getMultiplyRate());
+			accountedCrAmt = getAccountedAmount(eur, allocAmount, cr3a.getMultiplyRate());
+			tradeLineList.add(new PostingLine(eur, accountedDrAmt, Env.ZERO));
+			paymentLineList.add(new PostingLine(eur, Env.ZERO, accountedCrAmt));
+			gainLossAmt = new BigDecimal(100).setScale(eur.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(eur, gainLossAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(eur, Env.ZERO, gainLossAmt));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+		} finally {
+			rollback();
+			
+			deleteConversionRate(cr1a);
+			deleteConversionRate(cr1b);
+			deleteConversionRate(cr2a);
+			deleteConversionRate(cr2b);
+			deleteConversionRate(cr3a);
+			deleteConversionRate(cr3b);
+			
+			plv.deleteEx(true);
+			priceList.deleteEx(true);
+		}
+	}
+	
+	@Test
+	/**
+	 * Test the allocation posting (different period + reversal)
+	 * Payment Total=1000, Period 1
+	 * Invoice Total=1000, Period 2
+	 * Invoice Total=1000, Period 3 (Reversal)
+	 * https://idempiere.atlassian.net/browse/IDEMPIERE-5339
+	 */
+	public void testAllocateInvoicePaymentPosting_12() {
+		MBPartner bpartner = MBPartner.get(Env.getCtx(), DictionaryIDs.C_BPartner.C_AND_W.id); // C&W Construction
+		MCharge charge = MCharge.get(Env.getCtx(), DictionaryIDs.C_Charge.FREIGHT.id); // Freight Charges
+		Timestamp currentDate = Env.getContextAsDate(Env.getCtx(), "#Date");
+		
+		Calendar cal = Calendar.getInstance();
+		cal.setTimeInMillis(currentDate.getTime());
+		cal.add(Calendar.DAY_OF_MONTH, -2);
+		Timestamp date1 = new Timestamp(cal.getTimeInMillis());
+		
+		cal.setTimeInMillis(currentDate.getTime());
+		cal.add(Calendar.DAY_OF_MONTH, -1);
+		Timestamp date2 = new Timestamp(cal.getTimeInMillis());
+		
+		Timestamp date3 = currentDate;
+		
+		int C_ConversionType_ID = DictionaryIDs.C_ConversionType.SPOT.id; // Spot
+		
+		MCurrency aud = MCurrency.get(DictionaryIDs.C_Currency.AUD.id); // AUD
+		MCurrency eur = MCurrency.get(DictionaryIDs.C_Currency.EUR.id); // EUR
+		MCurrency usd = MCurrency.get(DictionaryIDs.C_Currency.USD.id); // USD
+		BigDecimal audToEur = new BigDecimal(0.7);
+		MConversionRate cr1a = createConversionRate(aud.getC_Currency_ID(), eur.getC_Currency_ID(), C_ConversionType_ID, date1, audToEur, true);
+		BigDecimal audToUsd = new BigDecimal(0.8);
+		MConversionRate cr1b = createConversionRate(aud.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date1, audToUsd, true);
+		
+		audToEur = new BigDecimal(0.8);
+		MConversionRate cr2a = createConversionRate(aud.getC_Currency_ID(), eur.getC_Currency_ID(), C_ConversionType_ID, date2, audToEur, true);
+		audToUsd = new BigDecimal(0.9);
+		MConversionRate cr2b = createConversionRate(aud.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date2, audToUsd, true);
+		
+		audToEur = new BigDecimal(0.8);
+		MConversionRate cr3a = createConversionRate(aud.getC_Currency_ID(), eur.getC_Currency_ID(), C_ConversionType_ID, date3, audToEur, true);
+		audToUsd = new BigDecimal(0.9);
+		MConversionRate cr3b = createConversionRate(aud.getC_Currency_ID(), usd.getC_Currency_ID(), C_ConversionType_ID, date3, audToUsd, true);
+		
+		MPriceList priceList = new MPriceList(Env.getCtx(), 0, null);
+		priceList.setName("Export AUD " + System.currentTimeMillis());
+		MCurrency australianDollar = MCurrency.get(DictionaryIDs.C_Currency.AUD.id); // Australian Dollar (AUD)
+		priceList.setC_Currency_ID(australianDollar.getC_Currency_ID());
+		priceList.setPricePrecision(australianDollar.getStdPrecision());
+		priceList.saveEx();
+		
+		MPriceListVersion plv = new MPriceListVersion(priceList);
+		plv.setM_DiscountSchema_ID(DictionaryIDs.M_DiscountSchema.SALES_2001.id); // Sales 2001
+		plv.setValidFrom(currentDate);
+		plv.saveEx();
+		
+		try {
+			MBankAccount ba = getBankAccount(usd.getC_Currency_ID());
+			BigDecimal payAmt = new BigDecimal(1000);
+			MPayment payment = createPayment(true, bpartner, ba.getC_BankAccount_ID(), date1, payAmt, aud.getC_Currency_ID(), C_ConversionType_ID);
+			completeDocument(payment);
+			postDocument(payment);
+			
+			MInvoice invoice = createInvoice(true, bpartner, date2, priceList.getM_PriceList_ID(), C_ConversionType_ID);
+			BigDecimal qty = BigDecimal.ONE;
+			BigDecimal price = new BigDecimal(1000);
+			createInvoiceLine(invoice, 10, null, charge, qty, price);
+			completeDocument(invoice);
+			postDocument(invoice);
+			
+			MAllocationHdr alloc = createAllocationHdr(date2, aud.getC_Currency_ID());
+			BigDecimal allocAmount = new BigDecimal(1000);
+			createAllocationLine(alloc, allocAmount, invoice, payment);
+			completeDocument(alloc);
+			postDocument(alloc);
+			
+			MAcctSchema[] ass = MAcctSchema.getClientAcctSchema(Env.getCtx(), Env.getAD_Client_ID(Env.getCtx()));
+			MAllocationHdr[] allocList = MAllocationHdr.getOfPayment(Env.getCtx(), payment.getC_Payment_ID(), getTrxName());
+			
+			ArrayList<PostingLine> paymentLineList = new ArrayList<PostingLine>();
+			ArrayList<PostingLine> tradeLineList = new ArrayList<PostingLine>();
+			ArrayList<PostingLine> gainLossLineList = new ArrayList<PostingLine>();
+			BigDecimal accountedDrAmt = getAccountedAmount(usd, allocAmount, cr1b.getMultiplyRate());
+			BigDecimal accountedCrAmt = getAccountedAmount(usd, allocAmount, cr1b.getMultiplyRate());
+			paymentLineList.add(new PostingLine(usd, accountedDrAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(usd, Env.ZERO, accountedCrAmt));
+			BigDecimal gainLossAmt = new BigDecimal(100).setScale(usd.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(usd, gainLossAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(usd, Env.ZERO, gainLossAmt));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+			
+			paymentLineList = new ArrayList<PostingLine>();
+			tradeLineList = new ArrayList<PostingLine>();
+			gainLossLineList = new ArrayList<PostingLine>();
+			accountedDrAmt = getAccountedAmount(eur, allocAmount, cr1a.getMultiplyRate());
+			accountedCrAmt = getAccountedAmount(eur, allocAmount, cr1a.getMultiplyRate());
+			paymentLineList.add(new PostingLine(eur, accountedDrAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(eur, Env.ZERO, accountedCrAmt));
+			gainLossAmt = new BigDecimal(100).setScale(eur.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(eur, gainLossAmt, Env.ZERO));
+			tradeLineList.add(new PostingLine(eur, Env.ZERO, gainLossAmt));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+			
+			reverseDocument(invoice, false);
+			MInvoice reversalInvoice = new MInvoice(Env.getCtx(), invoice.getReversal_ID(), getTrxName());
+			postDocument(reversalInvoice);
+			
+			MAllocationHdr reversalAlloc = null;
+			allocList = MAllocationHdr.getOfInvoice(Env.getCtx(), invoice.getC_Invoice_ID(), getTrxName());
+			for (MAllocationHdr allocation : allocList) {
+				if (allocation.getReversal_ID() > 0 && allocation.getReversal_ID() < allocation.get_ID()) {
+					reversalAlloc = allocation;
+					break;
+				}
+			}
+			
+			if (reversalAlloc == null)
+				reversalAlloc = new MAllocationHdr(Env.getCtx(), alloc.getReversal_ID(), getTrxName());
+		
+			allocList = new MAllocationHdr[] { reversalAlloc };
+			
+			paymentLineList = new ArrayList<PostingLine>();
+			tradeLineList = new ArrayList<PostingLine>();
+			gainLossLineList = new ArrayList<PostingLine>();
+			accountedDrAmt = getAccountedAmount(usd, allocAmount, cr1b.getMultiplyRate());
+			accountedCrAmt = getAccountedAmount(usd, allocAmount, cr1b.getMultiplyRate());
+			tradeLineList.add(new PostingLine(usd, accountedDrAmt, Env.ZERO));
+			paymentLineList.add(new PostingLine(usd, Env.ZERO, accountedCrAmt));
+			gainLossAmt = new BigDecimal(100).setScale(usd.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(usd, Env.ZERO, gainLossAmt));
+			tradeLineList.add(new PostingLine(usd, gainLossAmt, Env.ZERO));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+			
+			paymentLineList = new ArrayList<PostingLine>();
+			tradeLineList = new ArrayList<PostingLine>();
+			gainLossLineList = new ArrayList<PostingLine>();
+			accountedDrAmt = getAccountedAmount(eur, allocAmount, cr1a.getMultiplyRate());
+			accountedCrAmt = getAccountedAmount(eur, allocAmount, cr1a.getMultiplyRate());
+			tradeLineList.add(new PostingLine(eur, accountedDrAmt, Env.ZERO));
+			paymentLineList.add(new PostingLine(eur, Env.ZERO, accountedCrAmt));
+			gainLossAmt = new BigDecimal(100).setScale(eur.getStdPrecision(), RoundingMode.HALF_UP);
+			gainLossLineList.add(new PostingLine(eur, Env.ZERO, gainLossAmt));
+			tradeLineList.add(new PostingLine(eur, gainLossAmt, Env.ZERO));
+			
+			testAllocationPosting(ass, allocList, paymentLineList, tradeLineList, gainLossLineList, null);
+		} finally {
+			rollback();
+
+			deleteConversionRate(cr1a);
+			deleteConversionRate(cr1b);
+			deleteConversionRate(cr2a);
+			deleteConversionRate(cr2b);
+			deleteConversionRate(cr3a);
+			deleteConversionRate(cr3b);
+			
+			plv.deleteEx(true);
+			priceList.deleteEx(true);			
 		}
 	}
 	
@@ -1114,9 +1442,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, null, tradeLineList, gainLossLineList, null);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -1166,8 +1494,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, null, tradeLineList, null, null);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 	
@@ -1237,9 +1565,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, null, tradeLineList, gainLossLineList, null);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -1298,8 +1626,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, null, tradeLineList, null, null);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 	
@@ -1356,9 +1684,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, null, gainLossLineList, null);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -1404,8 +1732,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, null, null, null);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 	
@@ -1468,9 +1796,9 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, null, gainLossLineList, null);
 		} finally {
+			rollback();
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
-			rollback();
 		}
 	}
 	
@@ -1522,8 +1850,8 @@ public class Allocation2ndAcctSchemaTest extends AbstractTestCase {
 			
 			testAllocationPosting(ass, allocList, paymentLineList, null, null, null);
 		} finally {
-			deleteConversionRate(cr);
 			rollback();
+			deleteConversionRate(cr);
 		}
 	}
 		

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

@@ -419,10 +419,10 @@ public class AllocationTest extends AbstractTestCase {
 			}
 			
 		} finally {
+			rollback();
+
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);			
-			
-			rollback();
 		}
 	}
 	
@@ -523,10 +523,10 @@ public class AllocationTest extends AbstractTestCase {
 			}
 			
 		} finally {
+			rollback();
+
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);			
-			
-			rollback();
 		}
 	}
 	
@@ -2136,11 +2136,11 @@ public class AllocationTest extends AbstractTestCase {
 				}
 			}
 		} finally {
+			rollback();
+			
 			deleteConversionRate(cr1);
 			deleteConversionRate(cr2);
 			deleteConversionRate(cr3);			
-			
-			rollback();
 		}
 	}