IDEMPIERE-5738 - Info Window ID columns are sorted by ID instead of display value (#1863)

* IDEMPIERE-5738 - Info Window ID columns are sorted by ID instead of display value

* IDEMPIERE-5738 - improved parsing logic

- improved parsing logic for parsing display column aliases
- added getByKeyColumns method to MTable

* IDEMPIERE-5738 - requested changes

- replace JOIN with LEFT JOIN
- add AD_Reference_Value_ID to ColumnInfo.java
- check AD_Reference_ID and AD_Reference_Value_ID
- improve parsing

* IDEMPIERE-5738 - simplify 'getTable' logic

* IDEMPIERE-5738 - updating MTable from master

* IDEMPIERE-5738 - updating MTable from master

* IDEMPIERE-5738 - updating MTable from master

* IDEMPIERE-5738 - bug fix in getTable

* IDEMPIERE-5738 - fix bug if where clause is defined

* IDEMPIERE-5738 - small fixes

- change reference check from idID to isLookup
- fix wron tab spacing of getSQLJoin() method

* IDEMPIERE-5738 - fix unnecessary joins

* IDEMPIERE-5738 - small fixes

- rename joinTable to joinTableForUserOrder
- clear joinTableForUserOrder value in getUserOrderClause method

* IDEMPIERE-5738 - fix error with query criteria

* IDEMPIERE-5738 - fix trl tables, select distinct

- fix problem with translation tables
- fix problem with distinct info windows (error: for SELECT DISTINCT, ORDER BY expressions must appear in select list)

* IDEMPIERE-5738 - change approach from LEFT JOIN to subquery

- approach suggested by Heng Sin

* IDEMPIERE-5738 - support UUID key and composite key tables

* IDEMPIERE-5738 - fix WHERE clause in ORDER BY subquery

- improved ColumnInfo.java by selectClause
- in ORDER BY we need to use InfoColumn.SelectClause

* IDEMPIERE-5738 - small fixes

Requested changes by Heng Sin

org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/info/InfoWindow.java

@@ -986,6 +986,7 @@ public class InfoWindow extends InfoPanel implements ValueChangeListener, EventL
 				}
 				columnInfo.setColDescription(infoColumn.getNameTrl());
 				columnInfo.setAD_Reference_ID(infoColumn.getAD_Reference_ID());
+				columnInfo.setAD_Reference_Value_ID(infoColumn.getAD_Reference_Value_ID());
 				columnInfo.setGridField(gridFields.get(i));
 				columnInfo.setColumnName(infoColumn.getColumnName());
 				list.add(columnInfo);
@@ -1040,9 +1041,13 @@ public class InfoWindow extends InfoPanel implements ValueChangeListener, EventL
 					if (tableInfo.getTableName().equalsIgnoreCase(lookupInfo.TableName))
 					{
 						displayColumn = displayColumn.replace(lookupInfo.TableName+".", tableInfo.getSynonym()+".");
-						ColumnInfo columnInfo = new ColumnInfo(infoColumn.getNameTrl(), displayColumn, KeyNamePair.class, infoColumn.getSelectClause(), infoColumn.isReadOnly() || haveNotProcess);
+						ColumnInfo columnInfo = new ColumnInfo(infoColumn.getNameTrl(), displayColumn, KeyNamePair.class, infoColumn.getSelectClause(), infoColumn.isReadOnly() || haveNotProcess, displayColumn, infoColumn.getSelectClause());
 						return columnInfo;
 					}
+					else
+					{
+				        displayColumn = parseAliases(displayColumn, lookupInfo.TableName);
+					}
 					break;
 				}
 			}
@@ -1058,10 +1063,44 @@ public class InfoWindow extends InfoPanel implements ValueChangeListener, EventL
 			colSQL += " AS " + infoColumn.getColumnName();
         editorMap.put(colSQL, editor);
         Class<?> colClass = columnName.endsWith("_ID") || columnName.equals("CreatedBy") || columnName.equals("UpdatedBy") ? KeyNamePair.class : String.class;
-		ColumnInfo columnInfo = new ColumnInfo(infoColumn.getNameTrl(), colSQL, colClass, (String)null, infoColumn.isReadOnly() || haveNotProcess);
+        ColumnInfo columnInfo = new ColumnInfo(infoColumn.getNameTrl(), colSQL, colClass, (String)null, infoColumn.isReadOnly() || haveNotProcess, displayColumn, infoColumn.getSelectClause());
 		return columnInfo;
 	}
 
+	/**
+	 * Check and parse the correct aliases of the display SQL
+	 * @param displayColumn
+	 * @return parsed displayColumn
+	 */
+	private String parseAliases(String displayColumn, String tableName) {
+		if(Util.isEmpty(displayColumn))
+			return null;
+		String tabelNameTrl = tableName + "_Trl";
+		String alias = getAlias(tableName);
+
+		if(displayColumn.contains(tabelNameTrl+".")) {
+			if(displayColumn.contains(tableName+".") && !tableName.equalsIgnoreCase(alias)) {
+				return displayColumn.replace(tableName+".", alias+".");
+			}
+			return displayColumn;
+		}
+
+		if(displayColumn.contains(alias+".")){
+			return displayColumn;
+		}
+		else if(displayColumn.contains(tableName+".") && !tableName.equalsIgnoreCase(alias)) {
+			return displayColumn.replace(tableName+".", alias+".");
+		}
+		else if(!displayColumn.matches("\\w{1,}\\s{0,}\\((.*?)\\)")) {	// {function name}({*column name*})
+			displayColumn = (alias+"."+displayColumn);
+		}
+		else {
+			return null;
+		}
+
+		return displayColumn;
+	}
+
 	/* (non-Javadoc)
 	 * @see org.adempiere.webui.panel.InfoPanel#getSQLWhere()
 	 */
@@ -2032,25 +2071,37 @@ public class InfoWindow extends InfoPanel implements ValueChangeListener, EventL
     protected String buildDataSQL(int start, int end) {
 		String dataSql;
 		String dynWhere = getSQLWhere();
+		String orderClause = getUserOrderClause();
         StringBuilder sql = new StringBuilder (m_sqlMain);
+        
+        // add dynamic WHERE clause
         if (dynWhere.length() > 0)
             sql.append(dynWhere);   //  includes first AND
         
+        // trim trailing WHERE statement
         if (sql.toString().trim().endsWith("WHERE")) {
         	int index = sql.lastIndexOf(" WHERE");
         	sql.delete(index, sql.length());
         }
-        
         dataSql = Msg.parseTranslation(Env.getCtx(), sql.toString());    //  Variables
         dataSql = MRole.getDefault().addAccessSQL(dataSql, getTableName(),
             MRole.SQL_FULLYQUALIFIED, MRole.SQL_RO);
         
+        // add other SQL clause
         String otherClause = getOtherClauseParsed();
         if (otherClause.length() > 0) {
         	dataSql = dataSql + " " + otherClause;
         }
+        // add ORDER BY clause
+        dataSql = dataSql + orderClause;
 
-        dataSql = dataSql + getUserOrderClause();
+        // for SELECT DISTINCT, ORDER BY expressions must appear in select list - applies for lookup columns and multiselection columns
+        if(dataSql.startsWith("SELECT DISTINCT") && indexOrderColumn > 0) {
+        	ColumnInfo orderColumnInfo = p_layout[indexOrderColumn];
+        	if (DisplayType.isLookup(orderColumnInfo.getAD_Reference_ID()) || DisplayType.isChosenMultipleSelection(orderColumnInfo.getAD_Reference_ID())) {
+        		dataSql = appendOrderByToSelectList(dataSql, orderClause);
+        	}
+        }
 
         if (end > start && isUseDatabasePaging() && DB.getDatabase().isPagingSupported())
         {
@@ -2059,6 +2110,50 @@ public class InfoWindow extends InfoPanel implements ValueChangeListener, EventL
 		return dataSql;
 	}
 
+    /**
+     * Append ORDER BY expressions into the select list
+     * @param sql
+     * @param orderBy
+     * @return String parsed sql
+     */
+    private String appendOrderByToSelectList(String sql, String orderBy) {
+    	if(Util.isEmpty(orderBy))
+    		return sql;
+		int idxFrom = getIdxFrom(sql);
+		if(idxFrom < 0)
+			return sql;
+	
+		String select = sql.substring(0, idxFrom);
+		select += ", " + orderBy.replaceFirst("\\s+ORDER BY\\s+", "").replaceAll("\\s+ASC\\s+", "").replaceAll("\\s+DESC\\s+", "");	// \s+ stands for one or more whitespace character
+		return select + sql.substring(idxFrom);
+    }
+
+    /**
+     * Get the index of the FROM statement
+     * @param sql
+     * @return int idx
+     */
+    private int getIdxFrom(String sql) {
+		int parenthesisLevel = 0;
+		int idxSelect = sql.indexOf("SELECT DISTINCT");
+		sql = sql.substring(idxSelect);
+	
+		for(int i = 0; i < sql.length(); i++) {
+			// identify and ignore sub-query
+			char c = sql.charAt(i);
+				if (c == ')')
+					parenthesisLevel--;
+				else if (c == '(')
+					parenthesisLevel++;
+	
+				// RegEx ^(\s+FROM)(\s) checks for <whitespace>FROM<whitespace> pattern
+				if(sql.substring(i, i+6).toUpperCase().matches("^(\\s+FROM)(\\s)") && parenthesisLevel == 0)
+					return i;
+		}
+	
+		return -1;
+    }
+
     private String getOtherClauseParsed() {
     	String otherClause = "";
         if (infoWindow != null && infoWindow.getOtherClause() != null && infoWindow.getOtherClause().trim().length() > 0) {
@@ -2483,6 +2578,7 @@ public class InfoWindow extends InfoPanel implements ValueChangeListener, EventL
 						columnInfo.setColDescription(infoColumn.getDescriptionTrl());
 						columnInfo.setGridField(getGridField(infoColumn));
 						columnInfo.setAD_Reference_ID(infoColumn.getAD_Reference_ID());
+						columnInfo.setAD_Reference_Value_ID(infoColumn.getAD_Reference_Value_ID());
 						list.add(columnInfo);
 					}
 

org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/panel/InfoPanel.java

@@ -80,10 +80,12 @@ import org.compiere.model.MInfoColumn;
 import org.compiere.model.MInfoWindow;
 import org.compiere.model.MPInstance;
 import org.compiere.model.MProcess;
+import org.compiere.model.MRefTable;
 import org.compiere.model.MRole;
 import org.compiere.model.MSysConfig;
 import org.compiere.model.MTable;
 import org.compiere.model.X_AD_CtxHelp;
+import org.compiere.model.AccessSqlParser.TableInfo;
 import org.compiere.process.ProcessInfo;
 import org.compiere.process.ProcessInfoLog;
 import org.compiere.process.ProcessInfoUtil;
@@ -1318,14 +1320,15 @@ public abstract class InfoPanel extends Window implements EventListener<Event>,
 		if (indexOrderColumn > 0 && (indexOrderColumn + 1 > p_layout.length || !p_layout[indexOrderColumn].getColSQL().trim().equals(sqlOrderColumn))) {
 			// try to find out new index of ordered column, in case has other column is hide or display
 			for (int testIndex = 0; testIndex < p_layout.length; testIndex++) {
-				if (p_layout[testIndex].getColSQL().trim().equals(sqlOrderColumn)) {
+				if (p_layout[testIndex].getColSQL().trim().equals(sqlOrderColumn) || p_layout[testIndex].getDisplayColumn().equals(sqlOrderColumn)) {
 					indexOrderColumn = testIndex;
 					break;
 				}
 			}
 			
 			// index still incorrect and can't find out new index (ordered column become hide column)
-			if (indexOrderColumn > 0 && (indexOrderColumn + 1 > p_layout.length || !p_layout[indexOrderColumn].getColSQL().trim().equals(sqlOrderColumn))) {
+			if (indexOrderColumn > 0 && (indexOrderColumn + 1 > p_layout.length
+					|| (!p_layout[indexOrderColumn].getColSQL().trim().equals(sqlOrderColumn) && !p_layout[indexOrderColumn].getDisplayColumn().equals(sqlOrderColumn)))) {
 				indexOrderColumn = -1;
 				sqlOrderColumn = null;
 				m_sqlUserOrder = null;
@@ -1333,6 +1336,7 @@ public abstract class InfoPanel extends Window implements EventListener<Event>,
 		}
 			
 	}
+	
 	/**
 	 * build order clause of current sort order, and save it to m_sqlUserOrder
 	 * @return
@@ -1356,7 +1360,29 @@ public abstract class InfoPanel extends Window implements EventListener<Event>,
 	 * @return
 	 */
 	protected String getUserOrderClause(int col) {
-		String colsql = p_layout[col].getColSQL().trim();
+		ColumnInfo orderColumnInfo = p_layout[col];
+		String displayColumn = orderColumnInfo.getDisplayColumn();
+		String colsql = !Util.isEmpty(displayColumn) ? displayColumn : p_layout[col].getColSQL().trim();
+		
+		colsql = getSelectForOrderBy(colsql);
+		if(!Util.isEmpty(displayColumn) && (DisplayType.isLookup(orderColumnInfo.getAD_Reference_ID()) || DisplayType.isChosenMultipleSelection(orderColumnInfo.getAD_Reference_ID()))) {
+			String from = getFromForOrderBy(orderColumnInfo, displayColumn);
+			String where = getWhereForOrderBy(orderColumnInfo);
+			
+			return String.format(" ORDER BY (SELECT %s FROM %s WHERE %s) %s ", colsql, from, where, isColumnSortAscending? "" : "DESC");
+		}
+		else {
+			return String.format(" ORDER BY %s %s ", colsql, isColumnSortAscending? "" : "DESC");
+		}
+	}
+
+	/**
+	 * Get SQL SELECT clause for ORDER BY
+	 * @param colsql
+	 * @return String SELECT clause
+	 */
+	private String getSelectForOrderBy(String colsql) {
+		
 		int lastSpaceIdx = colsql.lastIndexOf(" ");
 		if (lastSpaceIdx > 0)
 		{
@@ -1385,8 +1411,62 @@ public abstract class InfoPanel extends Window implements EventListener<Event>,
 				}
 			}
 		}
+		return colsql;
+	}
 	
-		return String.format(" ORDER BY %s %s ", colsql, isColumnSortAscending? "" : "DESC");
+	/**
+	 * Get SQL FROM clause for ORDER BY
+	 * @param orderColumnInfo
+	 * @param displayColumn
+	 * @return String FROM clause
+	 */
+	private String getFromForOrderBy(ColumnInfo orderColumnInfo, String displayColumn) {
+		String fromClause = "";
+		MTable table = getTable(orderColumnInfo.getAD_Reference_Value_ID(), orderColumnInfo.getColumnName());
+		String tableName = table.getTableName();
+		if(table != null)
+			fromClause += tableName;
+
+		// join translation table
+		if(displayColumn.contains(table.getTableName()+"_Trl")) {
+			String tableNameTrl = tableName+"_Trl";
+			MTable tableTrl = MTable.get(Env.getCtx(), tableNameTrl);
+			String sqlSelect = orderColumnInfo.getSelectClause();
+			String[] keyCols = tableTrl.getKeyColumns();
+
+			fromClause += " JOIN " + tableNameTrl + " ON (";
+			for(int i = 0; i < keyCols.length; i++) {
+				String keyCol = keyCols[i];
+				
+				if(i > 0)
+					fromClause += " AND ";
+				
+				fromClause += tableNameTrl + "." + keyCol + " = ";
+				
+				if("AD_Language".equalsIgnoreCase(keyCol))
+					fromClause += " '" + Env.getAD_Language(Env.getCtx()) + "' ";
+				else
+					fromClause += sqlSelect;
+			}
+			fromClause += ") ";
+		}
+		return fromClause;
+	}
+	
+	/**
+	 * Get WHERE clause for ORDER BY
+	 * @param orderColumnInfo
+	 * @return String WHERE clause
+	 */
+	private String getWhereForOrderBy(ColumnInfo orderColumnInfo) {
+		MTable table = getTable(orderColumnInfo.getAD_Reference_Value_ID(), orderColumnInfo.getColumnName());
+		String tableName = table.getTableName();
+		String keyCol = table.getKeyColumns()[0];
+		String sqlSelect = orderColumnInfo.getSelectClause();
+		String whereClause = "";
+		
+		whereClause += tableName + "." + keyCol + " = " + sqlSelect;
+		return whereClause;
 	}
 	
     private void addDoubleClickListener() {
@@ -1399,6 +1479,21 @@ public abstract class InfoPanel extends Window implements EventListener<Event>,
 		contentPanel.addEventListener(Events.ON_SELECT, this);
 	}
 
+	/**
+	 * Get alias of the table, or the table name
+	 * @return String alias
+	 */
+	public String getAlias(String tableName) {
+		if(Util.isEmpty(tableName))
+			return "";
+		String alias = tableName;
+		for(TableInfo tableInfo : infoWindow.getTableInfos()) {
+			if(tableName.equalsIgnoreCase(tableInfo.getTableName()))
+				alias = !Util.isEmpty(tableInfo.getSynonym()) ? tableInfo.getSynonym() : tableName;
+		}
+		return alias;
+	}
+    
     /**
      * add paging component for list box
      */
@@ -2994,7 +3089,8 @@ public abstract class InfoPanel extends Window implements EventListener<Event>,
 		int col = lsc.getColumnIndex();
 		indexOrderColumn = col;
 		isColumnSortAscending = ascending;
-		sqlOrderColumn = p_layout[col].getColSQL().trim();
+		String displayColumn = p_layout[col].getDisplayColumn();
+		sqlOrderColumn = !Util.isEmpty(displayColumn) ? displayColumn : p_layout[col].getColSQL().trim();
 		m_sqlUserOrder = null; // clear cache value
 		
 		if (m_useDatabasePaging)
@@ -3008,6 +3104,24 @@ public abstract class InfoPanel extends Window implements EventListener<Event>,
 		renderItems();
 	}
 
+	/**
+	 * Get table name from AD_Ref_Table of Column Name
+	 * @param refValID
+	 * @param columnName
+	 * @return MTable[] tables
+	 */
+	private MTable getTable(int refValID, String columnName) {
+		if(refValID > 0) {
+			return MTable.get(Env.getCtx(), MRefTable.get(Env.getCtx(), refValID).getAD_Table_ID());
+		}
+		else if (columnName.endsWith("_ID") || columnName.endsWith("_UU")) {
+			return MTable.get(Env.getCtx(), columnName.substring(0, columnName.length() - 3));
+		}
+		else {
+			return null;
+		}
+	}
+	
 	/**
 	 * 
 	 * @return true if it is a lookup dialog

org.adempiere.ui/src/org/compiere/minigrid/ColumnInfo.java

@@ -75,7 +75,23 @@ public class ColumnInfo
 	 */
 	public ColumnInfo (String colHeader, String colSQL, Class<?> colClass, String keyPairColSQL, boolean readOnly)
 	{
-		this(colHeader, colSQL, colClass, readOnly, false, keyPairColSQL);
+		this(colHeader, colSQL, colClass, readOnly, false, keyPairColSQL, null);
+	}
+
+	/**
+	 *  Create Info Column (r/o and not color column)
+	 *
+	 *  @param colHeader Column Header
+	 *  @param colSQL    SQL select code for column
+	 *  @param colClass  class of column - determines display
+	 *  @param keyPairColSQL  SQL select for the ID of the for the displayed column
+	 *  @param readOnly  column is read only
+	 *  @param displayColumn SQL select code for display column
+	 *  @param selectClause AD_InfoColumn.SelectClause
+	 */
+	public ColumnInfo (String colHeader, String colSQL, Class<?> colClass, String keyPairColSQL, boolean readOnly, String displayColumn, String selectClause)
+	{
+		this(colHeader, colSQL, colClass, readOnly, false, keyPairColSQL, null, displayColumn, selectClause);
 	}   //  ColumnInfo
 
 	/**
@@ -91,7 +107,7 @@ public class ColumnInfo
 	public ColumnInfo (String colHeader, String colSQL, Class<?> colClass,
 		boolean readOnly, boolean colorColumn, String keyPairColSQL)
 	{
-		this(colHeader, colSQL, colClass, readOnly, false, keyPairColSQL, null);
+		this(colHeader, colSQL, colClass, readOnly, false, keyPairColSQL, null, null, null);
 	}
 
 	/**
@@ -106,7 +122,25 @@ public class ColumnInfo
 	 *  @param columnName Column Name
 	 */
 	public ColumnInfo (String colHeader, String colSQL, Class<?> colClass,
-		boolean readOnly, boolean colorColumn, String keyPairColSQL, String columnName)
+		boolean readOnly, boolean colorColumn, String keyPairColSQL, String columnName) {
+		this(colHeader, colSQL, colClass, readOnly, colorColumn, keyPairColSQL, columnName, null, null);
+	}
+
+	/**
+	 *  Create Info Column
+	 *
+	 *  @param colHeader Column Header
+	 *  @param colSQL    SQL select code for column
+	 *  @param colClass  class of column - determines display
+	 *  @param readOnly  column is read only
+	 *  @param colorColumn   if true, value of column determines foreground color
+	 *  @param keyPairColSQL  SQL select for the ID of the for the displayed column
+	 *  @param columnName Column Name
+	 *  @param displayColumn SQL select code for display column
+	 *  @param selectClause AD_InfoColumn.SelectClause
+	 */
+	public ColumnInfo (String colHeader, String colSQL, Class<?> colClass,
+		boolean readOnly, boolean colorColumn, String keyPairColSQL, String columnName, String displayColumn, String selectClause)
 	{
 		setColHeader(colHeader);
 		setColSQL(colSQL);
@@ -115,6 +149,8 @@ public class ColumnInfo
 		setColorColumn(colorColumn);
 		setColumnName(columnName);
 		setKeyPairColSQL(keyPairColSQL);
+		setDisplayColumn(displayColumn);
+		setSelectClause(selectClause);
 	}   //  ColumnInfo
 
 	/**
@@ -128,7 +164,7 @@ public class ColumnInfo
 	 */
 	public ColumnInfo (String colHeader, String colSQL, Class<?> colClass, boolean readOnly, String columnName)
 	{
-		this(colHeader, colSQL, colClass, readOnly, false, null, columnName); 
+		this(colHeader, colSQL, colClass, readOnly, false, null, columnName, null, null);
 	}   //  ColumnInfo
 
 	private String      m_colHeader;
@@ -142,6 +178,9 @@ public class ColumnInfo
 
 	private String colDescription;
 	private int AD_Reference_ID;
+	private String m_displayColumn;
+	private int AD_Reference_Value_ID;
+	private String selectClause;
 
 	/**
 	 * 	Get Col Class
@@ -298,4 +337,52 @@ public class ColumnInfo
 	public void setAD_Reference_ID(int AD_Reference_ID) {
 		this.AD_Reference_ID = AD_Reference_ID;
 	}
+
+	/**
+	 * Get Display Column
+	 * @return DisplayColumn
+	 */
+	public String getDisplayColumn() {
+		return m_displayColumn != null ? m_displayColumn : "";
+	}
+
+	/**
+	 * Set Display Column
+	 * @param displayColumn
+	 */
+	public void setDisplayColumn(String displayColumn) {
+		this.m_displayColumn = displayColumn;
+	}
+
+	/**
+	 * Get Reference Value
+	 * @return the aD_Reference_Value_ID
+	 */
+	public int getAD_Reference_Value_ID() {
+		return AD_Reference_Value_ID;
+	}
+
+	/**
+	 * Set Reference Value
+	 * @param AD_Reference_Value_ID the AD_Reference_Value_ID to set
+	 */
+	public void setAD_Reference_Value_ID(int AD_Reference_Value_ID) {
+		this.AD_Reference_Value_ID = AD_Reference_Value_ID;
+	}
+
+	/**
+	 * Get Sql SELECT
+	 * @return the selectClause
+	 */
+	public String getSelectClause() {
+		return selectClause;
+	}
+
+	/**
+	 * Set Sql SELECT
+	 * @param selectClause the selectClause to set
+	 */
+	public void setSelectClause(String selectClause) {
+		this.selectClause = selectClause;
+	}
 }   //  infoColumn