IDEMPIERE-5089 Add MSysConfig.ALLOW_REVERSAL_OF_RECONCILED_PAYMENT (#1254)

migration/i9/oracle/202203170900_IDEMPIERE-5089.sql

@@ -0,0 +1,18 @@
+SELECT register_migration_script('202203170900_IDEMPIERE-5089.sql') FROM dual
+;
+
+SET SQLBLANKLINES ON
+SET DEFINE OFF
+
+-- IDEMPIERE-5089 Add MSysConfig.ALLOW_REVERSAL_OF_RECONCILED_PAYMENT
+INSERT INTO AD_SysConfig (AD_SysConfig_ID,EntityType,ConfigurationLevel,Updated,Value,AD_SysConfig_UU,IsActive,Name,Created,AD_Org_ID,CreatedBy,UpdatedBy,AD_Client_ID) VALUES (200087,'D','C',TO_DATE('2022-03-17 17:52:01','YYYY-MM-DD HH24:MI:SS'),'Y','3c4b2107-ee12-44f4-9adb-1a4acbdac692','Y','ALLOW_REVERSAL_OF_RECONCILED_PAYMENT',TO_DATE('2022-03-17 17:52:01','YYYY-MM-DD HH24:MI:SS'),0,100,100,0)
+;
+
+-- Mar 17, 2022 6:03:46 PM GMT+08:00
+INSERT INTO AD_Message (MsgType,MsgText,AD_Message_ID,AD_Message_UU,Updated,Value,IsActive,CreatedBy,AD_Org_ID,AD_Client_ID,Created,UpdatedBy,EntityType) VALUES ('E','You can''t reverse a payment that have been reconciled',200415,'667a6f6d-9a96-4519-a576-03e606101d08',TO_DATE('2022-03-17 18:03:45','YYYY-MM-DD HH24:MI:SS'),'NotAllowReversalOfReconciledPayment','Y',100,0,0,TO_DATE('2022-03-17 18:03:45','YYYY-MM-DD HH24:MI:SS'),100,'D')
+;
+
+-- Mar 17, 2022, 6:19:51 PM MYT
+UPDATE AD_SysConfig SET Description='Y/N - Define if user is allow to reverse a payment that have been reconciled',Updated=TO_TIMESTAMP('2022-03-17 18:19:51','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_SysConfig_ID=200087
+;
+

migration/i9/postgresql/202203170900_IDEMPIERE-5089.sql

@@ -0,0 +1,15 @@
+SELECT register_migration_script('202203170900_IDEMPIERE-5089.sql') FROM dual
+;
+
+-- IDEMPIERE-5089 Add MSysConfig.ALLOW_REVERSAL_OF_RECONCILED_PAYMENT
+INSERT INTO AD_SysConfig (AD_SysConfig_ID,EntityType,ConfigurationLevel,Updated,Value,AD_SysConfig_UU,IsActive,Name,Created,AD_Org_ID,CreatedBy,UpdatedBy,AD_Client_ID) VALUES (200087,'D','C',TO_TIMESTAMP('2022-03-17 17:52:01','YYYY-MM-DD HH24:MI:SS'),'Y','3c4b2107-ee12-44f4-9adb-1a4acbdac692','Y','ALLOW_REVERSAL_OF_RECONCILED_PAYMENT',TO_TIMESTAMP('2022-03-17 17:52:01','YYYY-MM-DD HH24:MI:SS'),0,100,100,0)
+;
+
+-- Mar 17, 2022 6:03:46 PM GMT+08:00
+INSERT INTO AD_Message (MsgType,MsgText,AD_Message_ID,AD_Message_UU,Updated,Value,IsActive,CreatedBy,AD_Org_ID,AD_Client_ID,Created,UpdatedBy,EntityType) VALUES ('E','You can''t reverse a payment that have been reconciled',200415,'667a6f6d-9a96-4519-a576-03e606101d08',TO_TIMESTAMP('2022-03-17 18:03:45','YYYY-MM-DD HH24:MI:SS'),'NotAllowReversalOfReconciledPayment','Y',100,0,0,TO_TIMESTAMP('2022-03-17 18:03:45','YYYY-MM-DD HH24:MI:SS'),100,'D')
+;
+
+-- Mar 17, 2022, 6:19:51 PM MYT
+UPDATE AD_SysConfig SET Description='Y/N - Define if user is allow to reverse a payment that have been reconciled',Updated=TO_TIMESTAMP('2022-03-17 18:19:51','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_SysConfig_ID=200087
+;
+

org.adempiere.base/src/org/compiere/model/MPayment.java

@@ -2693,6 +2693,14 @@ public class MPayment extends X_C_Payment
 		}
 		MPeriod.testPeriodOpen(getCtx(), dateAcct, getC_DocType_ID(), getAD_Org_ID());
 		
+		if (getC_BankStatementLine_ID() > 0 && isReconciled()) {
+			boolean allow = MSysConfig.getBooleanValue(MSysConfig.ALLOW_REVERSAL_OF_RECONCILED_PAYMENT, true, Env.getAD_Client_ID(getCtx()));
+			if (!allow) {
+				m_processMsg = Msg.getMsg(getCtx(), "NotAllowReversalOfReconciledPayment");
+				return null;
+			}
+		}
+		
 		//	Create Reversal
 		MPayment reversal = new MPayment (getCtx(), 0, get_TrxName());
 		copyValues(this, reversal);

org.adempiere.base/src/org/compiere/model/MSysConfig.java

@@ -51,6 +51,7 @@ public class MSysConfig extends X_AD_SysConfig
     public static final String ALLOCATION_DESCRIPTION = "ALLOCATION_DESCRIPTION";
     public static final String ALLOW_APPLY_PAYMENT_TO_CREDITMEMO = "ALLOW_APPLY_PAYMENT_TO_CREDITMEMO";
     public static final String ALLOW_OVER_APPLIED_PAYMENT = "ALLOW_OVER_APPLIED_PAYMENT";
+    public static final String ALLOW_REVERSAL_OF_RECONCILED_PAYMENT = "ALLOW_REVERSAL_OF_RECONCILED_PAYMENT";
     public static final String ALogin_ShowDate = "ALogin_ShowDate";
     public static final String ALogin_ShowOneRole = "ALogin_ShowOneRole"; // deprecated
     public static final String APPLICATION_DATABASE_VERSION = "APPLICATION_DATABASE_VERSION";

org.adempiere.base/src/org/compiere/wf/MWFActivity.java

@@ -990,7 +990,11 @@ public class MWFActivity extends X_AD_WF_Activity implements Runnable
 				if (m_process != null)
 				{
 					m_process.setProcessMsg(this.getTextMsg());
+					try {
 						m_process.saveEx();
+					} catch (Exception ex) {
+						log.log(Level.SEVERE, ex.getMessage(), ex);
+					}
 				}
 			} finally {
 				if (contextLost)

org.adempiere.base/src/org/compiere/wf/MWFProcess.java

@@ -102,7 +102,7 @@ public class MWFProcess extends X_AD_WF_Process
 		if (!TimeUtil.isValid(wf.getValidFrom(), wf.getValidTo()))
 			throw new IllegalStateException("Workflow not valid");
 		m_wf = wf;
-//TODO  m_pi = pi; red1 - never used  -check later	
+		m_pi = pi;	
 		setAD_Workflow_ID (wf.getAD_Workflow_ID());
 		setPriority(wf.getPriority());
 		super.setWFState (WFSTATE_NotStarted);
@@ -132,9 +132,6 @@ public class MWFProcess extends X_AD_WF_Process
 		//	Lock Entity
 		getPO();
 		setAD_Org_ID(m_po.getAD_Org_ID());//Add by Hideaki Hagiwara
-		//hengsin: remove lock/unlock which is causing deadlock
-		//if (m_po != null)
-			//m_po.lock();
 	}	//	MWFProcess
 
 	/**	State Machine				*/
@@ -144,10 +141,7 @@ public class MWFProcess extends X_AD_WF_Process
 	/**	Workflow					*/
 	private MWorkflow			m_wf = null;
 	/**	Process Info				*/
-/*TODO red1 - never used
- * 
 	private ProcessInfo			m_pi = null;
- */
 	/**	Persistent Object			*/
 	private PO					m_po = null;
 	/** Message from Activity		*/
@@ -625,6 +619,15 @@ public class MWFProcess extends X_AD_WF_Process
 		return m_po;
 	}	//	getPO
 
+	/**
+	 * 
+	 * @return {@link ProcessInfo}
+	 */
+	public ProcessInfo getProcessInfo()
+	{
+		return m_pi;
+	}
+	
 	/**
 	 * 	Set Text Msg (add to existing)
 	 *	@param po base object

org.adempiere.base/src/org/compiere/wf/MWorkflow.java

@@ -48,6 +48,7 @@ import org.compiere.util.DB;
 import org.compiere.util.Env;
 import org.compiere.util.Msg;
 import org.compiere.util.Trx;
+import org.compiere.util.Util;
 import org.idempiere.cache.ImmutablePOSupport;
 import org.idempiere.cache.ImmutablePOCache;
 
@@ -779,7 +780,18 @@ public class MWorkflow extends X_AD_Workflow implements ImmutablePOSupport
 			if (localTrx != null)
 				localTrx.rollback();
 			log.log(Level.SEVERE, e.getLocalizedMessage(), e);
-			pi.setSummary(e.getMessage(), true);
+			StringBuilder msg = new StringBuilder();
+			if (retValue != null)
+			{
+				StateEngine state = retValue.getState();
+				if (!Util.isEmpty(retValue.getProcessMsg()) && (state.isTerminated() || state.isAborted()))
+				{
+					msg.append(retValue.getProcessMsg());
+					msg.append("\n");
+				}				
+			}
+			msg.append(e.getMessage());
+			pi.setSummary(msg.toString(), true);
 			retValue = null;
 		}
 		finally 

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

@@ -26,15 +26,22 @@ package org.idempiere.test.model;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.math.BigDecimal;
 import java.sql.Timestamp;
 
 import org.compiere.model.MBankStatement;
 import org.compiere.model.MBankStatementLine;
+import org.compiere.model.MPayment;
+import org.compiere.model.MSysConfig;
+import org.compiere.model.PO;
+import org.compiere.model.Query;
 import org.compiere.process.DocAction;
 import org.compiere.process.ProcessInfo;
+import org.compiere.util.CacheMgt;
 import org.compiere.util.Env;
+import org.compiere.util.Msg;
 import org.compiere.util.TimeUtil;
 import org.compiere.wf.MWorkflow;
 import org.idempiere.test.AbstractTestCase;
@@ -78,4 +85,165 @@ public class BankStatementTest extends AbstractTestCase {
 		stmt.load(getTrxName());
 		assertEquals(DocAction.STATUS_Completed, stmt.getDocStatus());
 	}
+	
+	@Test
+	public void testReversalOfReconciledPayment1() {
+		Timestamp today = TimeUtil.getDay(System.currentTimeMillis());
+		
+		Query query = new Query(Env.getCtx(), MSysConfig.Table_Name, "Name=? AND AD_Client_ID IN (0, ?)", null);
+		MSysConfig sysConfig = query.setOrderBy("AD_Client_ID Desc").setParameters(MSysConfig.ALLOW_REVERSAL_OF_RECONCILED_PAYMENT, getAD_Client_ID()).first();
+		if (!sysConfig.getValue().equals("Y")) {
+			sysConfig.setValue("Y");
+			try {
+				PO.setCrossTenantSafe();
+				sysConfig.saveEx();
+			} finally {
+				PO.clearCrossTenantSafe();
+			}
+			CacheMgt.get().reset();
+		} else {
+			sysConfig = null;
+		}
+		
+		try {
+			MPayment payment1 = new MPayment(Env.getCtx(), 0, getTrxName());
+			payment1.setC_BPartner_ID(117); //C&W
+			payment1.setC_DocType_ID(true); // Receipt
+			payment1.setDocStatus(DocAction.STATUS_Drafted);
+			payment1.setDocAction(DocAction.ACTION_Complete);
+			payment1.setPayAmt(Env.ONEHUNDRED);
+			payment1.setTenderType(MPayment.TENDERTYPE_Check);
+			payment1.setC_BankAccount_ID(100); // 1234_MoneyBank_123456789
+			payment1.setC_Currency_ID(100);  // USD
+			payment1.setDateTrx(today);
+			payment1.setDateAcct(today);
+			payment1.saveEx();
+			
+			ProcessInfo pi = MWorkflow.runDocumentActionWorkflow(payment1, DocAction.ACTION_Complete);
+			payment1.load(getTrxName());
+			assertFalse(pi.isError(), "Error processing payment: " + pi.getSummary());
+			assertEquals(DocAction.STATUS_Completed, payment1.getDocStatus(), "Payment document status is not completed: " + payment1.getDocStatus());
+			
+			MBankStatement stmt = new MBankStatement(Env.getCtx(), 0, getTrxName());
+			stmt.setC_BankAccount_ID(100);		
+			stmt.setStatementDate(today);
+			stmt.setDateAcct(today);
+			stmt.setName(System.currentTimeMillis()+"");
+			stmt.setDocAction(DocAction.ACTION_Complete);
+			stmt.setDocStatus(DocAction.STATUS_Drafted);
+			stmt.saveEx();
+			
+			MBankStatementLine line = new MBankStatementLine(stmt);
+			line.setValutaDate(today);
+			line.setStatementLineDate(today);
+			line.setStmtAmt(payment1.getPayAmt());
+			line.setTrxAmt(payment1.getPayAmt());
+			line.setC_Payment_ID(payment1.getC_Payment_ID());
+			line.setC_Currency_ID(100);
+			line.saveEx();
+			
+			pi = MWorkflow.runDocumentActionWorkflow(stmt, DocAction.ACTION_Complete);
+			assertFalse(pi.isError());
+			
+			stmt.load(getTrxName());
+			assertEquals(DocAction.STATUS_Completed, stmt.getDocStatus());
+			
+			payment1.load(getTrxName());
+			payment1.setDocAction(DocAction.ACTION_Reverse_Correct);
+			payment1.saveEx();
+			pi = MWorkflow.runDocumentActionWorkflow(payment1, DocAction.ACTION_Reverse_Correct);		
+			assertFalse(pi.isError(), "Error reversing payment: " + pi.getSummary());
+			assertEquals(DocAction.STATUS_Reversed, payment1.getDocStatus(), "Unexpected Payment Document Status");
+		} finally {
+			if (sysConfig != null) {
+				sysConfig.setValue("N");
+				try {
+					PO.setCrossTenantSafe();
+					sysConfig.saveEx();
+				} finally {
+					PO.clearCrossTenantSafe();
+				}
+			}
+		}
+	}
+	
+	@Test
+	public void testReversalOfReconciledPayment2() {
+		Timestamp today = TimeUtil.getDay(System.currentTimeMillis());
+		
+		Query query = new Query(Env.getCtx(), MSysConfig.Table_Name, "Name=? AND AD_Client_ID IN (0, ?)", null);
+		MSysConfig sysConfig = query.setOrderBy("AD_Client_ID Desc").setParameters(MSysConfig.ALLOW_REVERSAL_OF_RECONCILED_PAYMENT, getAD_Client_ID()).first();
+		if (!sysConfig.getValue().equals("N")) {
+			sysConfig.setValue("N");
+			try {
+				PO.setCrossTenantSafe();
+				sysConfig.saveEx();
+			} finally {
+				PO.clearCrossTenantSafe();
+			}
+			CacheMgt.get().reset();
+		} else {
+			sysConfig = null;
+		}
+		try {
+			MPayment payment1 = new MPayment(Env.getCtx(), 0, getTrxName());
+			payment1.setC_BPartner_ID(117); //C&W
+			payment1.setC_DocType_ID(true); // Receipt
+			payment1.setDocStatus(DocAction.STATUS_Drafted);
+			payment1.setDocAction(DocAction.ACTION_Complete);
+			payment1.setPayAmt(Env.ONEHUNDRED);
+			payment1.setTenderType(MPayment.TENDERTYPE_Check);
+			payment1.setC_BankAccount_ID(100); // 1234_MoneyBank_123456789
+			payment1.setC_Currency_ID(100);  // USD
+			payment1.setDateTrx(today);
+			payment1.setDateAcct(today);
+			payment1.saveEx();
+			
+			ProcessInfo pi = MWorkflow.runDocumentActionWorkflow(payment1, DocAction.ACTION_Complete);
+			payment1.load(getTrxName());
+			assertFalse(pi.isError(), "Error processing payment: " + pi.getSummary());
+			assertEquals(DocAction.STATUS_Completed, payment1.getDocStatus(), "Payment document status is not completed: " + payment1.getDocStatus());
+			
+			MBankStatement stmt = new MBankStatement(Env.getCtx(), 0, getTrxName());
+			stmt.setC_BankAccount_ID(100);		
+			stmt.setStatementDate(today);
+			stmt.setDateAcct(today);
+			stmt.setName(System.currentTimeMillis()+"");
+			stmt.setDocAction(DocAction.ACTION_Complete);
+			stmt.setDocStatus(DocAction.STATUS_Drafted);
+			stmt.saveEx();
+			
+			MBankStatementLine line = new MBankStatementLine(stmt);
+			line.setValutaDate(today);
+			line.setStatementLineDate(today);
+			line.setStmtAmt(payment1.getPayAmt());
+			line.setTrxAmt(payment1.getPayAmt());
+			line.setC_Payment_ID(payment1.getC_Payment_ID());
+			line.setC_Currency_ID(100);
+			line.saveEx();
+			
+			pi = MWorkflow.runDocumentActionWorkflow(stmt, DocAction.ACTION_Complete);
+			assertFalse(pi.isError());
+			
+			stmt.load(getTrxName());
+			assertEquals(DocAction.STATUS_Completed, stmt.getDocStatus());
+			
+			payment1.load(getTrxName());
+			payment1.setDocAction(DocAction.ACTION_Reverse_Correct);
+			payment1.saveEx();
+			pi = MWorkflow.runDocumentActionWorkflow(payment1, DocAction.ACTION_Reverse_Correct);		
+			assertTrue(pi.isError(), "Reversal of reconciled payment should fail here.");
+			assertTrue(pi.getSummary() != null && pi.getSummary().contains(Msg.getMsg(Env.getCtx(), "NotAllowReversalOfReconciledPayment")), "Unexpected error message: " + pi.getSummary());
+		} finally {
+			if (sysConfig != null) {
+				sysConfig.setValue("Y");
+				try {
+					PO.setCrossTenantSafe();
+					sysConfig.saveEx();
+				} finally {
+					PO.clearCrossTenantSafe();
+				}
+			}
+		}
+	}
 }

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

@@ -231,8 +231,9 @@ public class MProductTest extends AbstractTestCase {
 		product.setIsActive(false);
 		assertThrows(AdempiereException.class, () -> product.saveEx());
 		
-		//clear on hand so that we can deactivate product
+		//clear on hand and reservation so that we can deactivate product
 		DB.executeUpdateEx("UPDATE M_StorageOnHand SET QtyOnHand=0 WHERE M_Product_ID=?", new Object[] {product.get_ID()}, getTrxName());
+		DB.executeUpdateEx("UPDATE M_StorageReservation SET Qty=0 WHERE M_Product_ID=?", new Object[] {product.get_ID()}, getTrxName());
 		product.setIsActive(false);
 		product.saveEx();