IDEMPIERE-4836 Adding Access tab on Document Status (Activity) / peer review - fix multi-tenant issue (#1061)

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

@@ -41,7 +41,7 @@ public class MDocumentStatus extends X_PA_DocumentStatus {
 	/**
 	 * 
 	 */
-	private static final long serialVersionUID = 5908220133480463782L;
+	private static final long serialVersionUID = 4028519324986534673L;
 
 	public MDocumentStatus(Properties ctx, int PA_DocumentStatus_ID, String trxName) {
 		super(ctx, PA_DocumentStatus_ID, trxName);
@@ -76,29 +76,27 @@ public class MDocumentStatus extends X_PA_DocumentStatus {
 		if (AD_User_ID < 0)
 			return new MDocumentStatus[0];
 
-		String whereClause = "PA_DocumentStatus.AD_Client_ID IN (0,?) AND ((dsa.AD_User_ID IS NULL OR dsa.AD_User_ID=?) "
+		String whereClause = "AD_Client_ID IN (0,?)";
-				+ "AND ( dsa.AD_Role_ID IS NULL OR dsa.AD_Role_ID=?) AND (dsa.AD_Client_ID IS NULL OR dsa.AD_Client_ID IN (0,?)))";
-		String joinClause = "LEFT JOIN PA_DocumentStatusAccess dsa ON PA_DocumentStatus.PA_DocumentStatus_ID = dsa.PA_DocumentStatus_ID "
-				+ "AND dsa.IsActive = 'Y' ";
 
 		List<MDocumentStatus> list = new Query(ctx, MDocumentStatus.Table_Name, whereClause, trxName)
 				.setOnlyActiveRecords(true)
 				.setOrderBy(MDocumentStatus.COLUMNNAME_SeqNo)
-				.addJoinClause(joinClause)
+				.setParameters(Env.getAD_Client_ID(ctx))
-				.setParameters(Env.getAD_Client_ID(ctx), AD_User_ID, AD_Role_ID,Env.getAD_Client_ID(ctx))
 				.list();
 
 		/* Verify access for user/role */
 		List<MDocumentStatus> listWithAccess = new ArrayList<MDocumentStatus>();
 		for (MDocumentStatus ds : list) {
-			if (ds.getAD_Window_ID() > 0) {
+			if (ds.canAccess(ctx, AD_User_ID, AD_Role_ID, trxName)) {
-				Boolean access = MRole.getDefault().getWindowAccess(ds.getAD_Window_ID());
+				if (ds.getAD_Window_ID() > 0) {
-				if (access != null)
+					Boolean access = MRole.getDefault().getWindowAccess(ds.getAD_Window_ID());
-					listWithAccess.add(ds);
+					if (access != null)
-			} else if (ds.getAD_Form_ID() > 0) {
+						listWithAccess.add(ds);
-				Boolean access = MRole.getDefault().getFormAccess(ds.getAD_Form_ID());
+				} else if (ds.getAD_Form_ID() > 0) {
-				if (access != null)
+					Boolean access = MRole.getDefault().getFormAccess(ds.getAD_Form_ID());
-					listWithAccess.add(ds);
+					if (access != null)
+						listWithAccess.add(ds);
+				}
 			}
 		}
 
@@ -145,4 +143,32 @@ public class MDocumentStatus extends X_PA_DocumentStatus {
 		return sb.toString();
 	}
 
+	/**
+	 * Verify access against the table PA_DocumentStatusAccess
+	 * @param userId  AD_User_ID
+	 * @param roleId  AD_Role_ID
+	 * @return true if the user/role has access
+	 */
+	private boolean canAccess(Properties ctx, int userId, int roleId, String trxName) {
+		List<MDocumentStatusAccess> accessList = new Query(ctx, MDocumentStatusAccess.Table_Name, "PA_DocumentStatus_ID=? AND AD_Client_ID IN (0,?)", trxName)
+				.setOnlyActiveRecords(true)
+				.setParameters(getPA_DocumentStatus_ID(), Env.getAD_Client_ID(ctx))
+				.list();
+
+		if (accessList.size() == 0)
+			return true; // no permissions set on System or Tenant - allow access
+
+		for (MDocumentStatusAccess access : accessList) {
+			/* the only problem here is that is not easy to hide things from System role or System user
+			 * but as they are the administrators is not a problem
+			 */
+			if (access.getAD_Role_ID() == roleId)
+				return true;
+			if (access.getAD_User_ID() == userId)
+				return true;
+		}
+
+		return false;
+	}
+
 }

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

@@ -0,0 +1,45 @@
+/**********************************************************************
+* 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:                                                       *
+* - Carlos Ruiz                                                       *
+**********************************************************************/
+
+package org.compiere.model;
+
+import java.sql.ResultSet;
+import java.util.Properties;
+
+public class MDocumentStatusAccess extends X_PA_DocumentStatusAccess {
+	/**
+	 * 
+	 */
+	private static final long serialVersionUID = -6018040839563733491L;
+
+	public MDocumentStatusAccess(Properties ctx, int PA_DocumentStatusAccess_ID, String trxName) {
+		super(ctx, PA_DocumentStatusAccess_ID, trxName);
+	}
+	
+	public MDocumentStatusAccess(Properties ctx, ResultSet rs, String trxName) {
+		super(ctx, rs, trxName);
+	}
+
+}

org.idempiere.test/src/org/idempiere/test/base/DocumentStatusTest.java

@@ -34,7 +34,6 @@ import org.compiere.model.MEntityType;
 import org.compiere.model.MInvoice;
 import org.compiere.model.MOrder;
 import org.compiere.model.X_PA_DocumentStatusAccess;
-import org.compiere.util.DB;
 import org.compiere.util.Env;
 import org.idempiere.test.AbstractTestCase;
 import org.junit.jupiter.api.Test;
@@ -160,6 +159,5 @@ public class DocumentStatusTest extends AbstractTestCase {
 		assertFalse(Arrays.asList(documentStatusIndicators).contains(WorkflowActivitiesDS),
 				"User assignment not visible, Should not Visible");
 
-		rollback();
 	}
 }