From 062f2a8ca6f968419e5f0372c0b92d6d76da66fa Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Tue, 12 Aug 2014 09:51:20 +0200 Subject: [PATCH] IDEMPIERE-2134 Issues found on Payment Selection process - implement trx management * Payment Selection Manual: ** PaySelect.generatePaySelect: out of transaction, dangerous, if a line fails the header is still there * Payment Print/Export ** PayPrint.cmd_export and cmd_print: both are creating and completing payments out of transaction, if something fails in the middle the process can leave partial things --- .../oracle/201408120942_IDEMPIERE-2134.sql | 15 ++ .../201408120942_IDEMPIERE-2134.sql | 12 + .../compiere/model/MPaySelectionCheck.java | 238 +++++++++++------- .../adempiere/webui/apps/form/WPayPrint.java | 21 +- .../src/org/compiere/apps/form/PaySelect.java | 93 +++---- 5 files changed, 237 insertions(+), 142 deletions(-) create mode 100644 migration/i2.0z/oracle/201408120942_IDEMPIERE-2134.sql create mode 100644 migration/i2.0z/postgresql/201408120942_IDEMPIERE-2134.sql diff --git a/migration/i2.0z/oracle/201408120942_IDEMPIERE-2134.sql b/migration/i2.0z/oracle/201408120942_IDEMPIERE-2134.sql new file mode 100644 index 0000000000..25fe87dae9 --- /dev/null +++ b/migration/i2.0z/oracle/201408120942_IDEMPIERE-2134.sql @@ -0,0 +1,15 @@ +SET SQLBLANKLINES ON +SET DEFINE OFF + +-- Aug 12, 2014 9:41:46 AM CEST +-- IDEMPIERE-2134 Issues found on Payment Selection process +INSERT INTO AD_Message (MsgType,MsgText,AD_Message_ID,EntityType,AD_Message_UU,Value,IsActive,CreatedBy,UpdatedBy,Created,AD_Client_ID,AD_Org_ID,Updated) VALUES ('I','This process creates and complete payments automatically, do you want to print and at the same time create/complete payments?',200306,'D','e623e783-7b0d-4882-9ea6-0f04c30880ca','CreatePayments?','Y',100,100,TO_DATE('2014-08-12 09:41:45','YYYY-MM-DD HH24:MI:SS'),0,0,TO_DATE('2014-08-12 09:41:45','YYYY-MM-DD HH24:MI:SS')) +; + +-- Aug 12, 2014 9:47:55 AM CEST +UPDATE AD_Message SET MsgText='Is the payment print correct ? If answered yes, then it will create and complete payments.',Updated=TO_DATE('2014-08-12 09:47:55','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Message_ID=585 +; + +SELECT register_migration_script('201408120942_IDEMPIERE-2134.sql') FROM dual +; + diff --git a/migration/i2.0z/postgresql/201408120942_IDEMPIERE-2134.sql b/migration/i2.0z/postgresql/201408120942_IDEMPIERE-2134.sql new file mode 100644 index 0000000000..41f1d18e8c --- /dev/null +++ b/migration/i2.0z/postgresql/201408120942_IDEMPIERE-2134.sql @@ -0,0 +1,12 @@ +-- Aug 12, 2014 9:41:46 AM CEST +-- IDEMPIERE-2134 Issues found on Payment Selection process +INSERT INTO AD_Message (MsgType,MsgText,AD_Message_ID,EntityType,AD_Message_UU,Value,IsActive,CreatedBy,UpdatedBy,Created,AD_Client_ID,AD_Org_ID,Updated) VALUES ('I','This process creates and complete payments automatically, do you want to print and at the same time create/complete payments?',200306,'D','e623e783-7b0d-4882-9ea6-0f04c30880ca','CreatePayments?','Y',100,100,TO_TIMESTAMP('2014-08-12 09:41:45','YYYY-MM-DD HH24:MI:SS'),0,0,TO_TIMESTAMP('2014-08-12 09:41:45','YYYY-MM-DD HH24:MI:SS')) +; + +-- Aug 12, 2014 9:47:55 AM CEST +UPDATE AD_Message SET MsgText='Is the payment print correct ? If answered yes, then it will create and complete payments.',Updated=TO_TIMESTAMP('2014-08-12 09:47:55','YYYY-MM-DD HH24:MI:SS'),UpdatedBy=100 WHERE AD_Message_ID=585 +; + +SELECT register_migration_script('201408120942_IDEMPIERE-2134.sql') FROM dual +; + diff --git a/org.adempiere.base/src/org/compiere/model/MPaySelectionCheck.java b/org.adempiere.base/src/org/compiere/model/MPaySelectionCheck.java index 28db5763c7..0de3ad58a5 100644 --- a/org.adempiere.base/src/org/compiere/model/MPaySelectionCheck.java +++ b/org.adempiere.base/src/org/compiere/model/MPaySelectionCheck.java @@ -30,6 +30,7 @@ import org.compiere.util.CLogger; import org.compiere.util.DB; import org.compiere.util.Env; import org.compiere.util.Msg; +import org.compiere.util.Trx; /** * Payment Print/Export model. @@ -42,7 +43,7 @@ public class MPaySelectionCheck extends X_C_PaySelectionCheck /** * */ - private static final long serialVersionUID = -5890511999934551763L; + private static final long serialVersionUID = -5752888482207479355L; /** * Get Check for Payment @@ -268,89 +269,108 @@ public class MPaySelectionCheck extends X_C_PaySelectionCheck */ public static void confirmPrint (MPaySelectionCheck check, MPaymentBatch batch) { - MPayment payment = new MPayment(check.getCtx(), check.getC_Payment_ID(), check.get_TrxName()); - // Existing Payment - if (check.getC_Payment_ID() != 0) - { - // Update check number - if (check.getPaymentRule().equals(PAYMENTRULE_Check)) + boolean localTrx = false; + String trxName = check.get_TrxName(); + Trx trx = null; + if (trxName == null) { + localTrx = true; + trxName = Trx.createTrxName("ConfirmPrintSingle"); + trx = Trx.get(trxName, true); + check.set_TrxName(trxName); + } + try { + MPayment payment = new MPayment(check.getCtx(), check.getC_Payment_ID(), trxName); + // Existing Payment + if (check.getC_Payment_ID() != 0) { - payment.setCheckNo(check.getDocumentNo()); - if (!payment.save()) - s_log.log(Level.SEVERE, "Payment not saved: " + payment); + // Update check number + if (check.getPaymentRule().equals(PAYMENTRULE_Check)) + { + payment.setCheckNo(check.getDocumentNo()); + payment.saveEx(); + } + } + else // New Payment + { + payment = new MPayment(check.getCtx(), 0, trxName); + payment.setAD_Org_ID(check.getAD_Org_ID()); + // + if (check.getPaymentRule().equals(PAYMENTRULE_Check)) + payment.setBankCheck (check.getParent().getC_BankAccount_ID(), false, check.getDocumentNo()); + else if (check.getPaymentRule().equals(PAYMENTRULE_CreditCard)) + payment.setTenderType(X_C_Payment.TENDERTYPE_CreditCard); + else if (check.getPaymentRule().equals(PAYMENTRULE_DirectDeposit) + || check.getPaymentRule().equals(PAYMENTRULE_DirectDebit)) + payment.setBankACH(check); + else + { + s_log.log(Level.SEVERE, "Unsupported Payment Rule=" + check.getPaymentRule()); + return; + } + payment.setTrxType(X_C_Payment.TRXTYPE_CreditPayment); + payment.setAmount(check.getParent().getC_Currency_ID(), check.getPayAmt()); + payment.setDiscountAmt(check.getDiscountAmt()); + payment.setDateTrx(check.getParent().getPayDate()); + payment.setDateAcct(payment.getDateTrx()); // globalqss [ 2030685 ] + payment.setC_BPartner_ID(check.getC_BPartner_ID()); + // Link to Batch + if (batch != null) + { + if (batch.getC_PaymentBatch_ID() == 0) + batch.saveEx(trxName); // new + payment.setC_PaymentBatch_ID(batch.getC_PaymentBatch_ID()); + } + // Link to Invoice + MPaySelectionLine[] psls = check.getPaySelectionLines(true); + if (s_log.isLoggable(Level.FINE)) s_log.fine("confirmPrint - " + check + " (#SelectionLines=" + psls.length + ")"); + if (check.getQty() == 1 && psls != null && psls.length == 1) + { + MPaySelectionLine psl = psls[0]; + if (s_log.isLoggable(Level.FINE)) s_log.fine("Map to Invoice " + psl); + // + payment.setC_Invoice_ID (psl.getC_Invoice_ID()); + payment.setDiscountAmt (psl.getDiscountAmt()); + payment.setWriteOffAmt(psl.getDifferenceAmt()); + BigDecimal overUnder = psl.getOpenAmt().subtract(psl.getPayAmt()) + .subtract(psl.getDiscountAmt()).subtract(psl.getDifferenceAmt()); + payment.setOverUnderAmt(overUnder); + } + else + payment.setDiscountAmt(Env.ZERO); + payment.setWriteOffAmt(Env.ZERO); + payment.saveEx(); + // + int C_Payment_ID = payment.get_ID(); + if (C_Payment_ID < 1) + s_log.log(Level.SEVERE, "Payment not created=" + check); + else + { + check.setC_Payment_ID (C_Payment_ID); + check.saveEx(); // Payment process needs it + // added AdempiereException by zuhri + if (!payment.processIt(DocAction.ACTION_Complete)) + throw new AdempiereException("Failed when processing document - " + payment.getProcessMsg()); + // end added + payment.saveEx(); + } + } // new Payment + + check.setIsPrinted(true); + check.setProcessed(true); + check.saveEx(); + } catch (Exception e) { + if (localTrx && trx != null) { + trx.rollback(); + trx.close(); + trx = null; + } + throw new AdempiereException(e); + } finally { + if (localTrx && trx != null) { + trx.commit(); + trx.close(); } } - else // New Payment - { - payment = new MPayment(check.getCtx(), 0, check.get_TrxName()); - payment.setAD_Org_ID(check.getAD_Org_ID()); - // - if (check.getPaymentRule().equals(PAYMENTRULE_Check)) - payment.setBankCheck (check.getParent().getC_BankAccount_ID(), false, check.getDocumentNo()); - else if (check.getPaymentRule().equals(PAYMENTRULE_CreditCard)) - payment.setTenderType(X_C_Payment.TENDERTYPE_CreditCard); - else if (check.getPaymentRule().equals(PAYMENTRULE_DirectDeposit) - || check.getPaymentRule().equals(PAYMENTRULE_DirectDebit)) - payment.setBankACH(check); - else - { - s_log.log(Level.SEVERE, "Unsupported Payment Rule=" + check.getPaymentRule()); - return; - } - payment.setTrxType(X_C_Payment.TRXTYPE_CreditPayment); - payment.setAmount(check.getParent().getC_Currency_ID(), check.getPayAmt()); - payment.setDiscountAmt(check.getDiscountAmt()); - payment.setDateTrx(check.getParent().getPayDate()); - payment.setDateAcct(payment.getDateTrx()); // globalqss [ 2030685 ] - payment.setC_BPartner_ID(check.getC_BPartner_ID()); - // Link to Batch - if (batch != null) - { - if (batch.getC_PaymentBatch_ID() == 0) - batch.saveEx(); // new - payment.setC_PaymentBatch_ID(batch.getC_PaymentBatch_ID()); - } - // Link to Invoice - MPaySelectionLine[] psls = check.getPaySelectionLines(false); - if (s_log.isLoggable(Level.FINE)) s_log.fine("confirmPrint - " + check + " (#SelectionLines=" + psls.length + ")"); - if (check.getQty() == 1 && psls != null && psls.length == 1) - { - MPaySelectionLine psl = psls[0]; - if (s_log.isLoggable(Level.FINE)) s_log.fine("Map to Invoice " + psl); - // - payment.setC_Invoice_ID (psl.getC_Invoice_ID()); - payment.setDiscountAmt (psl.getDiscountAmt()); - payment.setWriteOffAmt(psl.getDifferenceAmt()); - BigDecimal overUnder = psl.getOpenAmt().subtract(psl.getPayAmt()) - .subtract(psl.getDiscountAmt()).subtract(psl.getDifferenceAmt()); - payment.setOverUnderAmt(overUnder); - } - else - payment.setDiscountAmt(Env.ZERO); - payment.setWriteOffAmt(Env.ZERO); - if (!payment.save()) - s_log.log(Level.SEVERE, "Payment not saved: " + payment); - // - int C_Payment_ID = payment.get_ID(); - if (C_Payment_ID < 1) - s_log.log(Level.SEVERE, "Payment not created=" + check); - else - { - check.setC_Payment_ID (C_Payment_ID); - check.saveEx(); // Payment process needs it - // added AdempiereException by zuhri - if (!payment.processIt(DocAction.ACTION_Complete)) - throw new AdempiereException("Failed when processing document - " + payment.getProcessMsg()); - // end added - if (!payment.save()) - s_log.log(Level.SEVERE, "Payment not saved: " + payment); - } - } // new Payment - - check.setIsPrinted(true); - check.setProcessed(true); - if (!check.save ()) - s_log.log(Level.SEVERE, "Check not saved: " + check); } // confirmPrint /************************************************************************** @@ -362,24 +382,50 @@ public class MPaySelectionCheck extends X_C_PaySelectionCheck */ public static int confirmPrint (MPaySelectionCheck[] checks, MPaymentBatch batch) { + boolean localTrx = false; + String trxName = null; + if (checks.length > 0) + trxName = checks[0].get_TrxName(); + Trx trx = null; + if (trxName == null) { + localTrx = true; + trxName = Trx.createTrxName("ConfirmPrintMulti"); + trx = Trx.get(trxName, true); + } int lastDocumentNo = 0; - for (int i = 0; i < checks.length; i++) - { - MPaySelectionCheck check = checks[i]; - confirmPrint(check, batch); - - // Get Check Document No - try + try { + for (int i = 0; i < checks.length; i++) { - int no = Integer.parseInt(check.getDocumentNo()); - if (lastDocumentNo < no) - lastDocumentNo = no; + MPaySelectionCheck check = checks[i]; + if (localTrx) + check.set_TrxName(trxName); + confirmPrint(check, batch); + + // Get Check Document No + try + { + int no = Integer.parseInt(check.getDocumentNo()); + if (lastDocumentNo < no) + lastDocumentNo = no; + } + catch (NumberFormatException ex) + { + s_log.log(Level.SEVERE, "DocumentNo=" + check.getDocumentNo(), ex); + } + } // all checks + } catch (Exception e) { + if (localTrx && trx != null) { + trx.rollback(); + trx.close(); + trx = null; } - catch (NumberFormatException ex) - { - s_log.log(Level.SEVERE, "DocumentNo=" + check.getDocumentNo(), ex); + throw new AdempiereException(e); + } finally { + if (localTrx && trx != null) { + trx.commit(); + trx.close(); } - } // all checks + } if (s_log.isLoggable(Level.FINE)) s_log.fine("Last Document No = " + lastDocumentNo); return lastDocumentNo; @@ -560,7 +606,7 @@ public class MPaySelectionCheck extends X_C_PaySelectionCheck /** * Get Payment Selection Lines of this check * @param requery requery - * @return array of peyment selection lines + * @return array of payment selection lines */ public MPaySelectionLine[] getPaySelectionLines (boolean requery) { diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/form/WPayPrint.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/form/WPayPrint.java index eddb48bc75..ad9163de21 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/form/WPayPrint.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/form/WPayPrint.java @@ -253,7 +253,7 @@ public class WPayPrint extends PayPrint implements IFormController, EventListene else if (e.getTarget() == bProcess) cmd_EFT(); else if (e.getTarget() == bPrint) - cmd_print(); + confirm_cmd_print(); } // actionPerformed /** @@ -410,6 +410,25 @@ public class WPayPrint extends PayPrint implements IFormController, EventListene dispose(); } // cmd_EFT + /** + * Confirm before printing + */ + protected void confirm_cmd_print() + { + FDialog.ask(m_WindowNo, form, "CreatePayments?", new Callback() { + + @Override + public void onCallback(Boolean result) + { + if (result) + { + cmd_print(); + } + + } + }); + } + /** * Print Checks and/or Remittance */ diff --git a/org.adempiere.ui/src/org/compiere/apps/form/PaySelect.java b/org.adempiere.ui/src/org/compiere/apps/form/PaySelect.java index 44fa79b307..20a44a46f7 100644 --- a/org.adempiere.ui/src/org/compiere/apps/form/PaySelect.java +++ b/org.adempiere.ui/src/org/compiere/apps/form/PaySelect.java @@ -51,12 +51,6 @@ public class PaySelect { /** @todo withholding */ - /** - * - */ - @SuppressWarnings("unused") - private static final long serialVersionUID = 2872767371244295934L; - /** Window No */ public int m_WindowNo = 0; @@ -417,52 +411,61 @@ public class PaySelect public String generatePaySelect(IMiniTable miniTable, ValueNamePair paymentRule, Timestamp payDate, BankInfo bi) { log.info(""); - // String trxName Trx.createTrxName("PaySelect"); - // Trx trx = Trx.get(trxName, true); trx needs to be committed too + String trxName = null; - trx = null; + Trx trx = null; + try { + trxName = Trx.createTrxName("PaySelect"); + trx = Trx.get(trxName, true); - String PaymentRule = paymentRule.getValue(); + String PaymentRule = paymentRule.getValue(); - // Create Header - m_ps = new MPaySelection(Env.getCtx(), 0, trxName); - m_ps.setName (Msg.getMsg(Env.getCtx(), "VPaySelect") - + " - " + paymentRule.getName() - + " - " + payDate); - m_ps.setPayDate (payDate); - m_ps.setC_BankAccount_ID(bi.C_BankAccount_ID); - m_ps.setIsApproved(true); - if (!m_ps.save()) - { - m_ps = null; - return Msg.translate(Env.getCtx(), "C_PaySelection_ID"); - } - if (log.isLoggable(Level.CONFIG)) log.config(m_ps.toString()); + // Create Header + m_ps = new MPaySelection(Env.getCtx(), 0, trxName); + m_ps.setName (Msg.getMsg(Env.getCtx(), "VPaySelect") + + " - " + paymentRule.getName() + + " - " + payDate); + m_ps.setPayDate (payDate); + m_ps.setC_BankAccount_ID(bi.C_BankAccount_ID); + m_ps.setIsApproved(true); + m_ps.saveEx(); + if (log.isLoggable(Level.CONFIG)) log.config(m_ps.toString()); - // Create Lines - int rows = miniTable.getRowCount(); - int line = 0; - for (int i = 0; i < rows; i++) - { - IDColumn id = (IDColumn)miniTable.getValueAt(i, 0); - if (id.isSelected()) + // Create Lines + int rows = miniTable.getRowCount(); + int line = 0; + for (int i = 0; i < rows; i++) { - line += 10; - MPaySelectionLine psl = new MPaySelectionLine (m_ps, line, PaymentRule); - int C_Invoice_ID = id.getRecord_ID().intValue(); - BigDecimal OpenAmt = (BigDecimal)miniTable.getValueAt(i, 8); - BigDecimal PayAmt = (BigDecimal)miniTable.getValueAt(i, 9); - boolean isSOTrx = false; - // - psl.setInvoice(C_Invoice_ID, isSOTrx, - OpenAmt, PayAmt, OpenAmt.subtract(PayAmt)); - if (!psl.save(trxName)) + IDColumn id = (IDColumn)miniTable.getValueAt(i, 0); + if (id.isSelected()) { - return Msg.translate(Env.getCtx(), "C_PaySelectionLine_ID"); + line += 10; + MPaySelectionLine psl = new MPaySelectionLine (m_ps, line, PaymentRule); + int C_Invoice_ID = id.getRecord_ID().intValue(); + BigDecimal OpenAmt = (BigDecimal)miniTable.getValueAt(i, 8); + BigDecimal PayAmt = (BigDecimal)miniTable.getValueAt(i, 9); + boolean isSOTrx = false; + // + psl.setInvoice(C_Invoice_ID, isSOTrx, + OpenAmt, PayAmt, OpenAmt.subtract(PayAmt)); + psl.saveEx(trxName); + if (log.isLoggable(Level.FINE)) log.fine("C_Invoice_ID=" + C_Invoice_ID + ", PayAmt=" + PayAmt); } - if (log.isLoggable(Level.FINE)) log.fine("C_Invoice_ID=" + C_Invoice_ID + ", PayAmt=" + PayAmt); + } // for all rows in table + } catch (Exception e) { + if (trx != null) { + trx.rollback(); + trx.close(); + trx = null; } - } // for all rows in table + m_ps = null; + throw new AdempiereException(e); + } finally { + if (trx != null) { + trx.commit(); + trx.close(); + } + } return null; } // generatePaySelect @@ -507,4 +510,4 @@ public class PaySelect } } // BankInfo -} // VPaySelect +} // PaySelect