IDEMPIERE-5337 Refactoring of Generate Charge from Account form (#1384)

org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/form/WCharge.java

@@ -21,6 +21,7 @@
 
 package org.adempiere.webui.apps.form;
 
+import java.io.Serializable;
 import java.util.logging.Level;
 
 import org.adempiere.webui.component.Button;
@@ -34,6 +35,7 @@ import org.adempiere.webui.component.Panel;
 import org.adempiere.webui.component.Row;
 import org.adempiere.webui.component.Rows;
 import org.adempiere.webui.component.Textbox;
+import org.adempiere.webui.component.WListItemRenderer;
 import org.adempiere.webui.component.WListbox;
 import org.adempiere.webui.panel.ADForm;
 import org.adempiere.webui.panel.CustomForm;
@@ -68,13 +70,12 @@ import org.zkoss.zul.Separator;
  *
  */
 @org.idempiere.ui.zk.annotation.Form(name = "org.compiere.apps.form.VCharge")
-public class WCharge extends Charge implements IFormController, EventListener<Event>
+public class WCharge extends Charge implements IFormController, EventListener<Event>, Serializable
 {
 	/**
-	 *
+	 * generated serial id
 	 */
-	@SuppressWarnings("unused")
+	private static final long serialVersionUID = 4571016052942218676L;
-	private static final long serialVersionUID = 4210542409436277344L;
 
 	private CustomForm form = new CustomForm();
 
@@ -106,73 +107,8 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
     private Button m_btnAccount = new Button();
     /** Table to hold data of accounts. */
     private WListbox m_tblData = new WListbox();
-
+    /** selected account count */
-    /** confirmation panel. */
+	private int m_selectedCount;
-    private ConfirmPanel m_pnlConfirm = new ConfirmPanel();
-    /** Confirmation Grid. */
-    private Grid m_grdConfirm = GridFactory.newGridLayout();
-
-    /** Enumeration of column names and indices. */
-    private enum EColumn
-    {
-        /** Select column to record whether the account is selected. */
-        SELECT(0, "Select"),
-        /** Value column to hold the account key. */
-        VALUE(1, "Value"),
-        /** Name column to hold the account name. */
-        NAME(2, "Name"),
-        /** Expense column to indicate whether or not the account is an expense account. */
-        EXPENSE(3, "Expense");
-
-        /** The column's index. */
-        @SuppressWarnings("unused")
-		private final int m_index;
-        /** The column's name. */
-        private final String m_title;
-
-        /**
-         * Constructor.
-         *
-         * @param index index of the column
-         * @param title name of the column
-         */
-        EColumn(int index, String title)
-        {
-            m_index = index;
-            m_title = title;
-        }
-
-        /**
-         * Gets the index of the column.
-         *
-         *  @return the column index.
-         */
-        /*public int index()
-        {
-            return m_index;
-        }*/
-
-        /**
-         * Gets the name of the column.
-         *
-         * @return the column's name
-         */
-        public String title()
-        {
-            return m_title;
-        }
-
-        /**
-         * Gets the number of columns.
-         *
-         * @return the number of columns.
-         */
-        /*public static int count()
-        {
-            return values().length;
-        }*/
-
-    }
 
     /**
      * Default constructor.
@@ -190,7 +126,7 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
      */
     protected void initForm()
     {
-        log.info("");
+        if (log.isLoggable(Level.INFO)) log.info("");
         try
         {
             staticInitialise();
@@ -213,7 +149,6 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
     {
         createNewChargePanel();
         createAccountPanel();
-        createConfirmPanel();
 
         return;
     }
@@ -232,13 +167,6 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
 		Center center = new Center();
         contentPane.appendChild(center);
 		center.appendChild(m_pnlAccount);
-
-		South south = new South();
-		contentPane.appendChild(south);
-		Panel southPanel = new Panel();
-		south.appendChild(southPanel);
-		southPanel.appendChild(new Separator());
-		southPanel.appendChild(m_grdConfirm);
 	}
 
     /**
@@ -281,6 +209,8 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
 		south.appendChild(southPanel);
 		m_btnAccount.setLabel(Msg.getMsg(Env.getCtx(), AD_MESSAGE_CREATE) + " " + Msg.getMsg(Env.getCtx(), "From") + " " + Msg.getElement(Env.getCtx(), "Account_ID"));
         m_btnAccount.addEventListener(Events.ON_CLICK, this);
+        m_btnAccount.setDisabled(true);
+        southPanel.appendChild(new Separator());
 		southPanel.appendChild(m_btnAccount);
 
         return;
@@ -348,11 +278,21 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
      */
     private void dynamicInitialise()
     {    	
-    	findChargeElementID();
         ListModelTable model = new ListModelTable(getData());
         m_tblData.setData(model, getColumnNames());
 		setColumnClass(m_tblData);		
-		findTaxCategoryID();
+		m_selectedCount = 0;
+		WListItemRenderer renderer = (WListItemRenderer) m_tblData.getItemRenderer();
+		renderer.addTableValueChangeListener(e -> {
+			if (e.getColumn() == EColumn.SELECT.index() && e.getNewValue() instanceof Boolean) {
+				Boolean b = (Boolean) e.getNewValue();
+				if (b)
+					m_selectedCount++;
+				else
+					m_selectedCount--;
+				m_btnAccount.setDisabled(m_selectedCount == 0);
+			}			
+		});
 		
         return;
     }   //  dynInit
@@ -364,7 +304,7 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
      */
     public void onEvent(Event event)
     {
-        log.info(event.getName());
+        if (log.isLoggable(Level.INFO)) log.info(event.getName());
         //
         if (event.getTarget().getId().equals(ConfirmPanel.A_OK) || m_C_Element_ID == 0)
         {
@@ -391,7 +331,7 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
         String value;
         String name;
 
-        log.config("");
+        if (log.isLoggable(Level.CONFIG)) log.config("");
         //  Get Input
         value = m_txbValueField.getValue();
         if (value.length() == 0)
@@ -439,25 +379,12 @@ public class WCharge extends Charge implements IFormController, EventListener<Ev
             FDialog.error(form.getWindowNo(), form, "ChargeNotCreated", listRejected.toString());
         }
         
+        m_selectedCount = 0;
+        m_btnAccount.setDisabled(true);
+
         return;
     }   //  createAccount
 
-    /**
-     *  Create Confirmation Panel with OK Button.
-     */
-    public void createConfirmPanel()
-    {
-        Rows rows = new Rows();
-        Row row = new Row();
-        m_pnlConfirm.addActionListener(this);
-        row.appendChild(m_pnlConfirm);
-        rows.appendChild(row);
-        m_grdConfirm.appendChild(rows);
-
-        return;
-    }   //  ConfirmPanel
-
-
     public void close()
     {
         SessionManager.getAppDesktop().closeActiveWindow();

org.adempiere.ui/src/org/compiere/apps/form/Charge.java

@@ -40,12 +40,9 @@ import org.compiere.util.Msg;
 public class Charge
 {	
 	/**	Window No			*/
-	public int         m_WindowNo = 0;
+	protected int         m_WindowNo = 0;
-//	/**	FormFrame			*/
-//	private FormFrame 	m_frame;
-
 	/** Account Element     */
-	public int         m_C_Element_ID = 0;
+	protected int         m_C_Element_ID = 0;
 	/** AccountSchema       */
 	private int         m_C_AcctSchema_ID = 0;
 	/** Default Charge Tax Category */
@@ -54,12 +51,31 @@ public class Charge
 	private int         m_AD_Org_ID = 0;
 	private MAcctSchema  m_acctSchema = null;
 	/**	Logger			*/
-	public static final CLogger log = CLogger.getCLogger(Charge.class);
+	protected static final CLogger log = CLogger.getCLogger(Charge.class);
+
+	/** optional trx name */
+	private String m_trxName;
 	
 	/**
-	 *  Dynamic Init
+	 * default constructor
-	 *  - Get defaults for primary AcctSchema
+	 */
-	 *  - Create Table with Accounts
+	public Charge() 
+	{
+		findChargeElementID();
+    	findTaxCategoryID();
+	}
+	
+	/**
+	 * set optional trx name
+	 * @param trxName
+	 */
+	public void setTrxName(String trxName)
+	{
+		m_trxName = trxName;
+	}
+	
+	/**
+	 *  @return list of account element(Boolean[Select],KeyNamePair[C_ElementValue_ID,Value],Name,Boolean[IsExpense]) records
 	 */
 	public Vector<Vector<Object>> getData()
 	{
@@ -75,7 +91,7 @@ public class Charge
 		ResultSet rs = null;
 		try
 		{
-			pstmt = DB.prepareStatement(sql, null);
+			pstmt = DB.prepareStatement(sql, m_trxName);
 			pstmt.setInt(1, m_C_Element_ID);
 			rs = pstmt.executeQuery();
 			while (rs.next())
@@ -105,10 +121,10 @@ public class Charge
 	}
 	
 	/**
-     * Finds the Element Identifier for the current charge.
+     * Finds C_Element_ID of primary accounting schema
      *
      */
-    public void findChargeElementID()
+    protected void findChargeElementID()
     {
     	m_C_AcctSchema_ID = Env.getContextAsInt(Env.getCtx(), Env.C_ACCTSCHEMA_ID);
         //  get Element
@@ -119,7 +135,7 @@ public class Charge
         ResultSet rs = null;
         try
         {
-            pstmt = DB.prepareStatement(sql, null);
+            pstmt = DB.prepareStatement(sql, m_trxName);
             pstmt.setInt(1, m_C_AcctSchema_ID);
             rs = pstmt.executeQuery();
             if (rs.next())
@@ -139,6 +155,10 @@ public class Charge
         }
     }
 	
+    /**
+     * 
+     * @return column names
+     */
 	public Vector<String> getColumnNames()
 	{
 		//  Header Info
@@ -151,6 +171,10 @@ public class Charge
 		return columnNames;
 	}
 	
+	/**
+	 * set class type of columns
+	 * @param dataTable
+	 */
 	public void setColumnClass(IMiniTable dataTable)
 	{
 		dataTable.setColumnClass(0, Boolean.class, false);      //  0-Selection
@@ -162,9 +186,9 @@ public class Charge
 	}
 	
 	/**
-     * Finds the identifier for the tax category for the client.
+     * Finds default tax category for the client.
      */
-	public void findTaxCategoryID()
+	protected void findTaxCategoryID()
 	{
 		//  Other Defaults
 		m_AD_Client_ID = Env.getAD_Client_ID(Env.getCtx());
@@ -178,7 +202,7 @@ public class Charge
 		ResultSet rs = null;
 		try
 		{
-			pstmt = DB.prepareStatement(sql, null);
+			pstmt = DB.prepareStatement(sql, m_trxName);
 			pstmt.setInt(1, m_AD_Client_ID);
 			rs = pstmt.executeQuery();
 			if (rs.next())
@@ -201,20 +225,24 @@ public class Charge
 	 *  @param value value
 	 *  @param name name
 	 *  @param isExpenseType is expense
-	 *  @return element value
+	 *  @return C_ElementValue_ID or 0 if create fail
 	 */
-	protected int createElementValue (String value, String name, boolean isExpenseType)
+	public int createElementValue (String value, String name, boolean isExpenseType)
 	{
-		log.config(name);
+		if (log.isLoggable(Level.CONFIG)) log.config(name);
 		//
 		MElementValue ev = new MElementValue(Env.getCtx(), value, name, null,
 			isExpenseType ? MElementValue.ACCOUNTTYPE_Expense : MElementValue.ACCOUNTTYPE_Revenue, 
 				MElementValue.ACCOUNTSIGN_Natural,
-				false, false, null);
+				false, false, m_trxName);
 		ev.setAD_Org_ID(m_AD_Org_ID);
 		ev.setC_Element_ID(m_C_Element_ID);
-		if (!ev.save())
+		try {
-			log.log(Level.WARNING, "C_ElementValue_ID not created");
+			ev.saveEx();
+		} catch (Exception e) {
+			log.log(Level.WARNING, "C_ElementValue_ID not created", e);
+			return 0;
+		}
 		return ev.getC_ElementValue_ID();
 	}   //  createElementValue
 
@@ -223,9 +251,9 @@ public class Charge
      *
      *  @param name             charge name
      *  @param elementValueId   element value identifier
-     *  @return charge identifier, or 0 if no charge created.
+     *  @return charge identifier, or 0 if create fail
      */
-    protected int createCharge(String name, int elementValueId)
+    public int createCharge(String name, int elementValueId)
     {
         MCharge charge;
         MAccount account;
@@ -245,13 +273,17 @@ public class Charge
         
         if (log.isLoggable(Level.CONFIG)) log.config(name + " - ");
         // Charge
-        charge = new MCharge(Env.getCtx(), 0, null);
+        charge = new MCharge(Env.getCtx(), 0, m_trxName);
         // IDEMPIERE-1099 - Key must be included in name to avoid name crashes in account schema.
         charge.setName(account.getAccount().getValue() + " " + name);
         charge.setC_TaxCategory_ID(m_C_TaxCategory_ID);
-        if (!charge.save())
+        try 
         {
-            log.log(Level.SEVERE, name + " not created");
+        	charge.saveEx();
+        } 
+        catch (Exception e)
+        {
+            log.log(Level.SEVERE, name + " not created", e);
             return 0;
         }
 
@@ -269,7 +301,7 @@ public class Charge
     {
         StringBuffer sql = createUpdateAccountSql(charge, account);
         //
-        int noAffectedRows = DB.executeUpdate(sql.toString(), null);
+        int noAffectedRows = DB.executeUpdate(sql.toString(), m_trxName);
         if (noAffectedRows != 1)
         {
             log.log(Level.SEVERE, "Update #" + noAffectedRows + "\n" + sql.toString());
@@ -326,7 +358,7 @@ public class Charge
         //  Get AcctSchama
         if (m_acctSchema == null)
         {
-            m_acctSchema = new MAcctSchema(Env.getCtx(), m_C_AcctSchema_ID, null);
+            m_acctSchema = new MAcctSchema(Env.getCtx(), m_C_AcctSchema_ID, m_trxName);
         }
 
         return;
@@ -362,14 +394,18 @@ public class Charge
             defaultAccount.getUser2_ID(),
             defaultAccount.getUserElement1_ID(),
             defaultAccount.getUserElement2_ID(),
-            null);
+            m_trxName);
 
         return account;
     }
 
-	public StringBuffer listCreated;
+	protected StringBuffer listCreated;
-	public StringBuffer listRejected;
+	protected StringBuffer listRejected;
 
+	/**
+	 * create charge from selected account elements
+	 * @param dataTable
+	 */
     public void createAccount(IMiniTable dataTable)
 	{
 		log.config("");
@@ -405,4 +441,68 @@ public class Charge
 		}
 	}   //  createAccount
     
+    /**
+     * 
+     * @return comma separated list of account element names (where charge create success)
+     */
+    public String getCreatedAccountNames() {
+    	return listCreated != null ? listCreated.toString() : "";
+    }
+    
+    /**
+     * 
+     * @return comma separated list of account element names (where charge create fail)
+     */
+    public String getRejectedAccountNames() {
+    	return listRejected != null ? listRejected.toString() : "";
+    }
+
+    /** Enumeration of column names and indices. */
+    public enum EColumn
+    {
+        /** Select column to record whether the account is selected. */
+        SELECT(0, "Select"),
+        /** Value column to hold the account key. */
+        VALUE(1, "Value"),
+        /** Name column to hold the account name. */
+        NAME(2, "Name"),
+        /** Expense column to indicate whether or not the account is an expense account. */
+        EXPENSE(3, "Expense");
+
+        /** The column's index. */
+		private final int m_index;
+        /** The column's name. */
+        private final String m_title;
+
+        /**
+         * Constructor.
+         *
+         * @param index index of the column
+         * @param title name of the column
+         */
+        EColumn(int index, String title)
+        {
+            m_index = index;
+            m_title = title;
+        }
+
+        /**
+         * Gets the name of the column.
+         *
+         * @return the column's name
+         */
+        public String title()
+        {
+            return m_title;
+        }
+
+        /**
+         * 
+         * @return column index (start from 0)
+         */
+        public int index() 
+        {
+        	return m_index;
+        }
+    }
 }   //  Charge

org.idempiere.test/src/org/idempiere/test/form/ChargeFormTest.java

@@ -0,0 +1,106 @@
+/***********************************************************************
+ * This file is part of iDempiere ERP Open Source                      *
+ * http://www.idempiere.org                                            *
+ *                                                                     *
+ * Copyright (C) Contributors                                          *
+ *                                                                     *
+ * This program is free software; you can redistribute it and/or       *
+ * modify it under the terms of the GNU General Public License         *
+ * as published by the Free Software Foundation; either version 2      *
+ * of the License, or (at your option) any later version.              *
+ *                                                                     *
+ * This program is distributed in the hope that it will be useful,     *
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of      *
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the        *
+ * GNU General Public License for more details.                        *
+ *                                                                     *
+ * You should have received a copy of the GNU General Public License   *
+ * along with this program; if not, write to the Free Software         *
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,          *
+ * MA 02110-1301, USA.                                                 *
+ *                                                                     *
+ * Contributors:                                                       *
+ * - hengsin                         								   *
+ **********************************************************************/
+package org.idempiere.test.form;
+
+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.util.Vector;
+
+import org.compiere.apps.form.Charge;
+import org.idempiere.test.AbstractTestCase;
+import org.idempiere.test.ui.MiniTableImpl;
+import org.junit.jupiter.api.Test;
+
+/**
+ * @author hengsin
+ *
+ */
+public class ChargeFormTest extends AbstractTestCase {
+
+	/**
+	 * 
+	 */
+	public ChargeFormTest() {
+	}
+
+	@Test
+	public void testCreateNewAccontAndCharge() {
+		Charge charge = new Charge();
+		charge.setTrxName(getTrxName());
+		
+		String value = "testCreateNewAccontAndCharge";
+		String name = "testCreateNewAccontAndCharge";
+		
+		int elementValueId = charge.createElementValue(value, name, true);
+		assertTrue(elementValueId > 0, "Failed to create new account element record");
+		int chargeId = charge.createCharge(name, elementValueId);
+		assertTrue(chargeId > 0, "Failed to creaet new Charge record");
+	}
+	
+	@Test
+	public void testCreateChargeFromAccount() {
+		Charge charge = new Charge();
+		charge.setTrxName(getTrxName());
+		
+		//load data
+		MiniTableImpl minitable = new MiniTableImpl();
+		Vector<String> columns = charge.getColumnNames();
+		for (String column : columns) {
+			minitable.addColumn(column);
+		}
+		charge.setColumnClass(minitable);
+		//Boolean[Select],KeyNamePair[C_ElementValue_ID,Value],Name,Boolean[IsExpense]
+		Vector<Vector<Object>> accounts = charge.getData();
+		assertTrue(accounts.size() > 0, "Failed to retrieve account element records");
+		for(int i = 0; i < accounts.size(); i++) {
+			minitable.setRowCount(i+1);
+			Vector<Object> account = accounts.get(i);
+			for(int j = 0; j < account.size(); j++)
+				minitable.setValueAt(account.get(j), i, j);
+		}		
+		assertEquals(accounts.size(), minitable.getRowCount(), "Error populating mini table");
+		
+		//test with no selection
+		charge.createAccount(minitable);
+		String created = charge.getCreatedAccountNames();
+		assertTrue(created == null || created.length() == 0, "Unexpected create of charge record");
+		
+		//test create with selection of first and second account element
+		minitable.setValueAt(Boolean.TRUE, 0, Charge.EColumn.SELECT.index());
+		minitable.setValueAt(Boolean.TRUE, 1, Charge.EColumn.SELECT.index());
+		//create charge from selected account element
+		charge.createAccount(minitable);
+		created = charge.getCreatedAccountNames();
+		assertTrue(created != null && created.length() > 0, "Failed to create Charge record from Account Element ");
+		String[] splited = created.split("[,]");
+		assertEquals(2, splited.length, "Unexpected number of charge records created");
+		
+		//should auto de-select
+		assertFalse((Boolean)minitable.getValueAt(0, Charge.EColumn.SELECT.index()), "Selection of first row not reset after create");
+		assertFalse((Boolean)minitable.getValueAt(1, Charge.EColumn.SELECT.index()), "Selection of second row not reset after create");
+	}
+}