From 39a5fe56636bd096009a6a65865f6574026b63fe Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Tue, 14 Jun 2022 13:19:19 +0200 Subject: [PATCH] IDEMPIERE-5315 Mismatch in queries WWFActivity.getWhereActivities and DPActivitiesModel.getWorkflowCount (#1363) * IDEMPIERE-5315 Mismatch in queries WWFActivity.getWhereActivities and DPActivitiesModel.getWorkflowCount - Found that DPActivities is deprecated, so deprecated too the DPActivitiesModel - Also found that DefaultDesktop had a dependency to show the total of DPActivities, refactored to do the same with the new DPDocumentStatus - Moved the query to MWFActivity to use where required - Refactored JDBC to Query * Better name for method * serialVersionUID --- .../src/org/compiere/wf/MWFActivity.java | 31 ++++- .../apps/graph/WDocumentStatusIndicator.java | 13 +- .../apps/graph/WDocumentStatusPanel.java | 10 ++ .../adempiere/webui/apps/wf/WWFActivity.java | 127 ++++-------------- .../webui/dashboard/DPActivities.java | 10 +- .../webui/dashboard/DPActivitiesModel.java | 58 +------- .../webui/desktop/DefaultDesktop.java | 41 +----- 7 files changed, 87 insertions(+), 203 deletions(-) diff --git a/org.adempiere.base/src/org/compiere/wf/MWFActivity.java b/org.adempiere.base/src/org/compiere/wf/MWFActivity.java index 6719eb6bb8..07f563cac9 100644 --- a/org.adempiere.base/src/org/compiere/wf/MWFActivity.java +++ b/org.adempiere.base/src/org/compiere/wf/MWFActivity.java @@ -83,9 +83,9 @@ import org.compiere.util.Util; public class MWFActivity extends X_AD_WF_Activity implements Runnable { /** - * + * */ - private static final long serialVersionUID = -3282235931100223816L; + private static final long serialVersionUID = 8180781075902940080L; private static final String CURRENT_WORKFLOW_PROCESS_INFO_ATTR = "Workflow.ProcessInfo"; @@ -2126,4 +2126,31 @@ public class MWFActivity extends X_AD_WF_Activity implements Runnable trx.removeTrxEventListener(this); } } + + /** + * Where to get the pending activities related to a User (unprocessed and suspended) + * The where returned requires the AD_User_ID parameter 5 times, and then AD_Client_ID + * @return Where Clause + */ + public static String getWhereUserPendingActivities() { + final String where = + "AD_WF_Activity.Processed='N' AND AD_WF_Activity.WFState='OS' AND (" + // Owner of Activity + + " AD_WF_Activity.AD_User_ID=?" // #1 + // Invoker (if no invoker = all) + + " OR EXISTS (SELECT * FROM AD_WF_Responsible r WHERE AD_WF_Activity.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID" + + " AND r.ResponsibleType='H' AND COALESCE(r.AD_User_ID,0)=0 AND COALESCE(r.AD_Role_ID,0)=0 AND (AD_WF_Activity.AD_User_ID=? OR AD_WF_Activity.AD_User_ID IS NULL))" // #2 + // Responsible User + + " OR EXISTS (SELECT * FROM AD_WF_Responsible r WHERE AD_WF_Activity.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID" + + " AND r.ResponsibleType='H' AND r.AD_User_ID=?)" // #3 + // Responsible Role + + " OR EXISTS (SELECT * FROM AD_WF_Responsible r INNER JOIN AD_User_Roles ur ON (r.AD_Role_ID=ur.AD_Role_ID)" + + " WHERE AD_WF_Activity.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID AND r.ResponsibleType='R' AND ur.AD_User_ID=? AND ur.isActive = 'Y')" // #4 + ///* Manual Responsible */ + + " OR EXISTS (SELECT * FROM AD_WF_ActivityApprover r " + + " WHERE AD_WF_Activity.AD_WF_Activity_ID=r.AD_WF_Activity_ID AND r.AD_User_ID=? AND r.isActive = 'Y')" + + ") AND AD_WF_Activity.AD_Client_ID=?"; // #5 + return where; + } + } // MWFActivity diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/graph/WDocumentStatusIndicator.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/graph/WDocumentStatusIndicator.java index 5dca13aced..87b2700515 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/graph/WDocumentStatusIndicator.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/graph/WDocumentStatusIndicator.java @@ -50,10 +50,7 @@ import org.zkoss.zul.Div; * Document Status Indicator */ public class WDocumentStatusIndicator extends Panel implements EventListener { - /** - * - */ - private static final long serialVersionUID = 794746556509546913L; + private static final long serialVersionUID = -9076405331101242792L; /** * Constructor @@ -173,4 +170,12 @@ public class WDocumentStatusIndicator extends Panel implements EventListener queue = EventQueues.lookup(IDesktop.ACTIVITIES_EVENT_QUEUE, true); + Event event = new Event(IDesktop.ON_ACTIVITIES_CHANGED_EVENT, null, count); + queue.publish(event); } public void updateUI() { diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/wf/WWFActivity.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/wf/WWFActivity.java index 9722a812b7..bc5aa65e97 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/wf/WWFActivity.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/apps/wf/WWFActivity.java @@ -13,10 +13,9 @@ *****************************************************************************/ package org.adempiere.webui.apps.wf; -import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import java.util.logging.Level; @@ -47,10 +46,9 @@ import org.compiere.model.MLookup; import org.compiere.model.MLookupFactory; import org.compiere.model.MQuery; import org.compiere.model.MRefList; -import org.compiere.model.MRole; import org.compiere.model.MSysConfig; +import org.compiere.model.Query; import org.compiere.util.CLogger; -import org.compiere.util.DB; import org.compiere.util.DisplayType; import org.compiere.util.Env; import org.compiere.util.Msg; @@ -66,11 +64,11 @@ import org.zkoss.zk.ui.event.Events; import org.zkoss.zk.ui.util.Clients; import org.zkoss.zul.Borderlayout; import org.zkoss.zul.Center; -import org.zkoss.zul.North; -import org.zkoss.zul.South; import org.zkoss.zul.Div; import org.zkoss.zul.Hbox; import org.zkoss.zul.Html; +import org.zkoss.zul.North; +import org.zkoss.zul.South; /** * Direct port from WFActivity @@ -319,42 +317,13 @@ public class WWFActivity extends ADForm implements EventListener */ public int getActivitiesCount() { - int count = 0; - - String sql = "SELECT COUNT(*) FROM AD_WF_Activity a " - + "WHERE " + getWhereActivities(); int AD_User_ID = Env.getAD_User_ID(Env.getCtx()); int AD_Client_ID = Env.getAD_Client_ID(Env.getCtx()); - MRole role = MRole.get(Env.getCtx(), Env.getAD_Role_ID(Env.getCtx())); - sql = role.addAccessSQL(sql, "a", true, false); - PreparedStatement pstmt = null; - ResultSet rs = null; - try - { - pstmt = DB.prepareStatement (sql, null); - pstmt.setInt (1, AD_User_ID); - pstmt.setInt (2, AD_User_ID); - pstmt.setInt (3, AD_User_ID); - pstmt.setInt (4, AD_User_ID); - pstmt.setInt (5, AD_User_ID); - pstmt.setInt (6, AD_Client_ID); - rs = pstmt.executeQuery (); - if (rs.next ()) { - count = rs.getInt(1); - } - } - catch (Exception e) - { - log.log(Level.SEVERE, sql, e); - } - finally - { - DB.close(rs, pstmt); - rs = null; pstmt = null; - } - + int count = new Query(Env.getCtx(), MWFActivity.Table_Name, MWFActivity.getWhereUserPendingActivities(), null) + .setApplyAccessFilter(true, false) + .setParameters(AD_User_ID, AD_User_ID, AD_User_ID, AD_User_ID, AD_User_ID, AD_Client_ID) + .count(); return count; - } /** @@ -369,52 +338,29 @@ public class WWFActivity extends ADForm implements EventListener model = new ListModelTable(); - ArrayList list = new ArrayList(); - String sql = "SELECT * FROM AD_WF_Activity a " - + "WHERE " + getWhereActivities() - + " ORDER BY a.Priority DESC, Created"; int AD_User_ID = Env.getAD_User_ID(Env.getCtx()); int AD_Client_ID = Env.getAD_Client_ID(Env.getCtx()); - MRole role = MRole.get(Env.getCtx(), Env.getAD_Role_ID(Env.getCtx())); - sql = role.addAccessSQL(sql, "a", true, false); - PreparedStatement pstmt = null; - ResultSet rs = null; - try - { - pstmt = DB.prepareStatement (sql, null); - pstmt.setInt (1, AD_User_ID); - pstmt.setInt (2, AD_User_ID); - pstmt.setInt (3, AD_User_ID); - pstmt.setInt (4, AD_User_ID); - pstmt.setInt (5, AD_User_ID); - pstmt.setInt (6, AD_Client_ID); - - rs = pstmt.executeQuery(); - while (rs.next ()) + Iterator it = new Query(Env.getCtx(), MWFActivity.Table_Name, MWFActivity.getWhereUserPendingActivities(), null) + .setApplyAccessFilter(true, false) + .setParameters(AD_User_ID, AD_User_ID, AD_User_ID, AD_User_ID, AD_User_ID, AD_Client_ID) + .setOrderBy("AD_WF_Activity.Priority DESC, AD_WF_Activity.Created") + .iterate(); + + List list = new ArrayList(); + while (it.hasNext()) { + MWFActivity activity = it.next(); + list.add (activity); + List rowData = new ArrayList(); + rowData.add(activity.getPriority()); + rowData.add(activity.getNodeName()); + rowData.add(activity.getSummary()); + model.add(rowData); + if (list.size() > MAX_ACTIVITIES_IN_LIST && MAX_ACTIVITIES_IN_LIST > 0) { - MWFActivity activity = new MWFActivity(Env.getCtx(), rs, null); - list.add (activity); - List rowData = new ArrayList(); - rowData.add(activity.getPriority()); - rowData.add(activity.getNodeName()); - rowData.add(activity.getSummary()); - model.add(rowData); - if (list.size() > MAX_ACTIVITIES_IN_LIST && MAX_ACTIVITIES_IN_LIST > 0) - { - log.warning("More then 200 Activities - ignored"); - break; - } + log.warning("More than " + MAX_ACTIVITIES_IN_LIST + " Activities - ignored"); + break; } } - catch (Exception e) - { - log.log(Level.SEVERE, sql, e); - } - finally - { - DB.close(rs, pstmt); - rs = null; pstmt = null; - } m_activities = new MWFActivity[list.size ()]; list.toArray (m_activities); // @@ -446,27 +392,6 @@ public class WWFActivity extends ADForm implements EventListener return m_activities.length; } // loadActivities - private String getWhereActivities() { - final String where = - "a.Processed='N' AND a.WFState='OS' AND (" - // Owner of Activity - + " a.AD_User_ID=?" // #1 - // Invoker (if no invoker = all) - + " OR EXISTS (SELECT * FROM AD_WF_Responsible r WHERE a.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID" - + " AND r.ResponsibleType='H' AND COALESCE(r.AD_User_ID,0)=0 AND COALESCE(r.AD_Role_ID,0)=0 AND (a.AD_User_ID=? OR a.AD_User_ID IS NULL))" // #2 - // Responsible User - + " OR EXISTS (SELECT * FROM AD_WF_Responsible r WHERE a.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID" - + " AND r.ResponsibleType='H' AND r.AD_User_ID=?)" // #3 - // Responsible Role - + " OR EXISTS (SELECT * FROM AD_WF_Responsible r INNER JOIN AD_User_Roles ur ON (r.AD_Role_ID=ur.AD_Role_ID)" - + " WHERE a.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID AND r.ResponsibleType='R' AND ur.AD_User_ID=? AND ur.isActive = 'Y')" // #4 - ///* Manual Responsible */ - + " OR EXISTS (SELECT * FROM AD_WF_ActivityApprover r " - + " WHERE a.AD_WF_Activity_ID=r.AD_WF_Activity_ID AND r.AD_User_ID=? AND r.isActive = 'Y')" - + ") AND a.AD_Client_ID=?"; // #5 - return where; - } - /** * Reset Display * @param selIndex select index diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivities.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivities.java index 019b149497..eb240118aa 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivities.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivities.java @@ -13,9 +13,6 @@ *****************************************************************************/ package org.adempiere.webui.dashboard; -import java.util.HashMap; -import java.util.Map; - import org.adempiere.webui.component.Button; import org.adempiere.webui.desktop.IDesktop; import org.adempiere.webui.session.SessionManager; @@ -148,12 +145,7 @@ public class DPActivities extends DashboardPanel implements EventListener btnUnprocessed.setLabel(labelU + " : " + noOfUnprocessed); EventQueue queue = EventQueues.lookup(IDesktop.ACTIVITIES_EVENT_QUEUE, true); - Map map = new HashMap(); - map.put("notice", noOfNotice); - map.put("request", noOfRequest); - map.put("workflow", noOfWorkflow); - map.put("unprocessed", noOfUnprocessed); - Event event = new Event(IDesktop.ON_ACTIVITIES_CHANGED_EVENT, null, map); + Event event = new Event(IDesktop.ON_ACTIVITIES_CHANGED_EVENT, null, noOfNotice + noOfRequest+ noOfWorkflow + noOfUnprocessed); queue.publish(event); } diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivitiesModel.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivitiesModel.java index a8a6c9c6e9..b755b3563a 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivitiesModel.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/dashboard/DPActivitiesModel.java @@ -13,14 +13,11 @@ *****************************************************************************/ package org.adempiere.webui.dashboard; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.util.logging.Level; - import org.compiere.model.MRole; -import org.compiere.util.CLogger; +import org.compiere.model.Query; import org.compiere.util.DB; import org.compiere.util.Env; +import org.compiere.wf.MWFActivity; /** * Dashboard item: Workflow activities, notices and requests @@ -34,10 +31,9 @@ import org.compiere.util.Env; * Contributors: * Deepak Pansheriya - showing only notes message */ +@Deprecated // replaced with DPDocumentStatus public class DPActivitiesModel { - private static final CLogger logger = CLogger.getCLogger(DPActivitiesModel.class); - public static boolean isShowUnprocessed() { return (Env.getAD_Client_ID(Env.getCtx()) > 0); } @@ -77,52 +73,12 @@ public class DPActivitiesModel { */ public static int getWorkflowCount() { - int count = 0; - - String sql = "SELECT count(*) FROM AD_WF_Activity a " - + "WHERE " - + "a.Processed='N' AND a.WFState='OS' AND (" - // Owner of Activity - + " a.AD_User_ID=?" // #1 - // Invoker (if no invoker = all) - + " OR EXISTS (SELECT * FROM AD_WF_Responsible r WHERE a.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID" - + " AND r.ResponsibleType='H' AND COALESCE(r.AD_User_ID,0)=0 AND COALESCE(r.AD_Role_ID,0)=0 AND (a.AD_User_ID=? OR a.AD_User_ID IS NULL))" // #2 - // Responsible User - + " OR EXISTS (SELECT * FROM AD_WF_Responsible r WHERE a.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID" - + " AND r.ResponsibleType='H' AND r.AD_User_ID=?)" // #3 - // Responsible Role - + " OR EXISTS (SELECT * FROM AD_WF_Responsible r INNER JOIN AD_User_Roles ur ON (r.AD_Role_ID=ur.AD_Role_ID)" - + " WHERE a.AD_WF_Responsible_ID=r.AD_WF_Responsible_ID AND r.ResponsibleType='R' AND ur.AD_User_ID=?)" // #4 - // - + ") AND a.AD_Client_ID=?"; // #5 int AD_User_ID = Env.getAD_User_ID(Env.getCtx()); int AD_Client_ID = Env.getAD_Client_ID(Env.getCtx()); - sql = MRole.getDefault().addAccessSQL(sql, "a", true, false); - PreparedStatement pstmt = null; - ResultSet rs = null; - try - { - pstmt = DB.prepareStatement (sql, null); - pstmt.setInt (1, AD_User_ID); - pstmt.setInt (2, AD_User_ID); - pstmt.setInt (3, AD_User_ID); - pstmt.setInt (4, AD_User_ID); - pstmt.setInt (5, AD_Client_ID); - rs = pstmt.executeQuery (); - if (rs.next ()) { - count = rs.getInt(1); - } - } - catch (Exception e) - { - logger.log(Level.SEVERE, sql, e); - } - finally - { - DB.close(rs, pstmt); - rs = null; pstmt = null; - } - + int count = new Query(Env.getCtx(), MWFActivity.Table_Name, MWFActivity.getWhereUserPendingActivities(), null) + .setApplyAccessFilter(true, false) + .setParameters(AD_User_ID, AD_User_ID, AD_User_ID, AD_User_ID, AD_User_ID, AD_Client_ID) + .count(); return count; } diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/desktop/DefaultDesktop.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/desktop/DefaultDesktop.java index 1cc578a1c7..ab6b147fe5 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/desktop/DefaultDesktop.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/desktop/DefaultDesktop.java @@ -22,7 +22,6 @@ import static org.compiere.model.SystemIDs.TREE_MENUPRIMARY; import java.io.Serializable; import java.math.BigDecimal; import java.util.List; -import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -137,13 +136,7 @@ public class DefaultDesktop extends TabbedDesktop implements MenuListener, Seria private Borderlayout layout; - private int noOfNotice; - - private int noOfRequest; - - private int noOfWorkflow; - - private int noOfUnprocessed; + private int noCount; private Tabpanel homeTab; @@ -700,28 +693,11 @@ public class DefaultDesktop extends TabbedDesktop implements MenuListener, Seria } else if (eventName.equals(ON_ACTIVITIES_CHANGED_EVENT)) { - @SuppressWarnings("unchecked") - Map map = (Map) event.getData(); - Integer notice = (Integer) map.get("notice"); - Integer request = (Integer) map.get("request"); - Integer workflow = (Integer) map.get("workflow"); - Integer unprocessed = (Integer) map.get("unprocessed"); + Integer count = (Integer) event.getData(); boolean change = false; - if (notice != null && notice.intValue() != noOfNotice) + if (count != null && count.intValue() != noCount) { - noOfNotice = notice.intValue(); change = true; - } - if (request != null && request.intValue() != noOfRequest) - { - noOfRequest = request.intValue(); change = true; - } - if (workflow != null && workflow.intValue() != noOfWorkflow) - { - noOfWorkflow = workflow.intValue(); change = true; - } - if (unprocessed != null && unprocessed.intValue() != noOfUnprocessed) - { - noOfUnprocessed = unprocessed.intValue(); change = true; + noCount = count.intValue(); change = true; } if (change) updateUI(); @@ -851,14 +827,7 @@ public class DefaultDesktop extends TabbedDesktop implements MenuListener, Seria } public void updateUI() { - int total = noOfNotice + noOfRequest + noOfWorkflow + noOfUnprocessed; - windowContainer.setTabTitle(0, Util.cleanAmp(Msg.getMsg(Env.getCtx(), "Home")) - + " (" + total + ")", - Msg.translate(Env.getCtx(), "AD_Note_ID") + " : " + noOfNotice - + ", " + Msg.translate(Env.getCtx(), "R_Request_ID") + " : " + noOfRequest - + ", " + Util.cleanAmp(Msg.getMsg (Env.getCtx(), "WorkflowActivities")) + " : " + noOfWorkflow - + (noOfUnprocessed>0 ? ", " + Msg.getMsg (Env.getCtx(), "UnprocessedDocs") + " : " + noOfUnprocessed : "") - ); + windowContainer.setTabTitle(0, Util.cleanAmp(Msg.getMsg(Env.getCtx(), "Home")) + " (" + noCount + ")", null); } //use _docClick undocumented api. need verification after major zk release update