Fixed issue 35065: A deactivated organization can not be activated again.
authorInigo Sanchez <inigo.sanchez@openbravo.com>
Fri, 27 Jan 2017 12:04:54 +0100
changeset 31617 09301dc0612e
parent 31296 31c79a32ce3a
child 31618 21055fdc01ac
Fixed issue 35065: A deactivated organization can not be activated again.

When an organization was disabled you could not activate it again as the row was not ediable. This
problem was introduced in a fix related with the code of addWritableAttribute() method of
DefaultJsonDataService class. Before this fix, It was managing the reported case properly (Activate
again a deactivated organization) because this method was handling the particular case of Organizations.

Now is taking into account this particular case (deactivate organizations) in order to manages this
particular case properly. Now it is possible to activate again a deactivate organization.
modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java
src/org/openbravo/dal/core/OBContext.java
src/org/openbravo/dal/security/SecurityChecker.java
--- a/modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java	Tue Jan 24 05:37:18 2017 +0000
+++ b/modules/org.openbravo.service.json/src/org/openbravo/service/json/DefaultJsonDataService.java	Fri Jan 27 12:04:54 2017 +0100
@@ -11,7 +11,7 @@
  * under the License. 
  * The Original Code is Openbravo ERP. 
  * The Initial Developer of the Original Code is Openbravo SLU 
- * All portions are Copyright (C) 2009-2016 Openbravo SLU 
+ * All portions are Copyright (C) 2009-2017 Openbravo SLU 
  * All Rights Reserved. 
  * Contributor(s):  ______________________________________.
  ************************************************************************
@@ -762,6 +762,14 @@
             break;
           }
         }
+        if (isOrganizationEntity(jsonObject)) {
+          for (String orgId : OBContext.getOBContext().getDeactivatedOrganizations()) {
+            if (orgId.equals(rowOrganization)) {
+              writable = true;
+              break;
+            }
+          }
+        }
         if (!writable) {
           jsonObject.put("_readOnly", true);
         }
@@ -769,6 +777,14 @@
     }
   }
 
+  private boolean isOrganizationEntity(JSONObject json) throws JSONException {
+    if (json.has(JsonConstants.ENTITYNAME)
+        && (Organization.ENTITY_NAME.equals(json.get(JsonConstants.ENTITYNAME)))) {
+      return true;
+    }
+    return false;
+  }
+
   /**
    * Returns the value for a FK property, in case the entity of the row is the referencedEntity for
    * that FK, it returns the row id.
--- a/src/org/openbravo/dal/core/OBContext.java	Tue Jan 24 05:37:18 2017 +0000
+++ b/src/org/openbravo/dal/core/OBContext.java	Fri Jan 27 12:04:54 2017 +0100
@@ -11,7 +11,7 @@
  * under the License. 
  * The Original Code is Openbravo ERP. 
  * The Initial Developer of the Original Code is Openbravo SLU 
- * All portions are Copyright (C) 2008-2016 Openbravo SLU 
+ * All portions are Copyright (C) 2008-2017 Openbravo SLU 
  * All Rights Reserved. 
  * Contributor(s):  ______________________________________.
  ************************************************************************
@@ -548,9 +548,11 @@
   private boolean translationInstalled;
   private Warehouse warehouse;
   private List<String> organizationList;
+  private List<String> deactivatedOrganizationList;
   private String[] readableOrganizations;
   private String[] readableClients;
   private Set<String> writableOrganizations;
+  private Set<String> deactivatedOrganizations;
   private String userLevel;
   private Map<String, OrganizationStructureProvider> organizationStructureProviderByClient;
   private Map<String, AcctSchemaStructureProvider> acctSchemaStructureProviderByClient;
@@ -615,7 +617,7 @@
       writableOrganizations.add("0");
     }
 
-    final List<String> os = getOrganizationList(role);
+    final List<String> os = getActiveOrganizationList(role);
     for (final String o : os) {
       writableOrganizations.add(o);
     }
@@ -626,25 +628,52 @@
     writableOrganizations.addAll(additionalWritableOrganizations);
   }
 
-  @SuppressWarnings("unchecked")
-  private List<String> getOrganizationList(Role thisRole) {
-    if (organizationList != null) {
-      return new ArrayList<String>(organizationList);
+  private List<String> getActiveOrganizationList(Role thisRole) {
+    return getOrganizationList(thisRole, organizationList, additionalWritableOrganizations, true);
+  }
+
+  private void setDeactivatedOrganizations(Role role) {
+    deactivatedOrganizations = new HashSet<String>();
+    final List<String> os = getDeactivatedOrganizationList(role);
+    for (final String o : os) {
+      deactivatedOrganizations.add(o);
     }
+  }
+
+  private List<String> getDeactivatedOrganizationList(Role thisRole) {
+    return getOrganizationList(thisRole, deactivatedOrganizationList, null, false);
+  }
+
+  private List<String> getOrganizationList(Role targetRole, List<String> orgList,
+      Set<String> additionalOrgs, boolean isActiveOrganization) {
+
+    if (orgList != null) {
+      return new ArrayList<String>(orgList);
+    }
+
+    String propertyActive = "N";
+    if (isActiveOrganization) {
+      propertyActive = "Y";
+    }
+
     final Query qry = SessionHandler.getInstance().createQuery(
         "select o.id from " + Organization.class.getName() + " o, "
             + RoleOrganization.class.getName() + " roa where o." + Organization.PROPERTY_ID
             + "=roa." + RoleOrganization.PROPERTY_ORGANIZATION + "." + Organization.PROPERTY_ID
             + " and roa." + RoleOrganization.PROPERTY_ROLE + "." + Organization.PROPERTY_ID + "='"
-            + thisRole.getId() + "' and roa." + RoleOrganization.PROPERTY_ACTIVE + "='Y' and o."
-            + Organization.PROPERTY_ACTIVE + "='Y'");
-    organizationList = qry.list();
-    for (final String orgId : additionalWritableOrganizations) {
-      if (!organizationList.contains(orgId)) {
-        organizationList.add(orgId);
+            + targetRole.getId() + "' and roa." + RoleOrganization.PROPERTY_ACTIVE + "='Y' and o."
+            + Organization.PROPERTY_ACTIVE + "='" + propertyActive + "'");
+    @SuppressWarnings("unchecked")
+    List<String> currentOrgList = qry.list();
+
+    if (additionalOrgs != null) {
+      for (final String orgId : additionalOrgs) {
+        if (!currentOrgList.contains(orgId)) {
+          currentOrgList.add(orgId);
+        }
       }
     }
-    return new ArrayList<String>(organizationList);
+    return new ArrayList<String>(currentOrgList);
   }
 
   @SuppressWarnings("unchecked")
@@ -658,7 +687,7 @@
   }
 
   private void setReadableOrganizations(Role role) {
-    final List<String> os = getOrganizationList(role);
+    final List<String> os = getActiveOrganizationList(role);
     final Set<String> readableOrgs = new HashSet<String>();
     for (final String o : os) {
       readableOrgs.addAll(getOrganizationStructureProvider().getNaturalTree(o));
@@ -720,8 +749,10 @@
     additionalWritableOrganizations.add(orgId);
     // nullify will be recomputed at first occasion
     organizationList = null;
+    deactivatedOrganizationList = null;
     readableOrganizations = null;
     writableOrganizations = null;
+    deactivatedOrganizations = null;
   }
 
   /**
@@ -775,9 +806,11 @@
     language = null;
     warehouse = null;
     organizationList = null;
+    deactivatedOrganizationList = null;
     readableOrganizations = null;
     readableClients = null;
     writableOrganizations = null;
+    deactivatedOrganizations = null;
     userLevel = null;
     organizationStructureProviderByClient = null;
     acctSchemaStructureProviderByClient = null;
@@ -1041,6 +1074,7 @@
     setUserLevel(role.getUserLevel());
     entityAccessChecker = null;
     writableOrganizations = null;
+    deactivatedOrganizations = null;
     readableClients = null;
     readableOrganizations = null;
     this.role = role;
@@ -1093,6 +1127,13 @@
     return new HashSet<String>(writableOrganizations);
   }
 
+  public Set<String> getDeactivatedOrganizations() {
+    if (deactivatedOrganizations == null) {
+      setDeactivatedOrganizations(getRole());
+    }
+    return new HashSet<String>(deactivatedOrganizations);
+  }
+
   public String[] getReadableClients() {
     if (readableClients == null) {
       setReadableClients(getRole());
--- a/src/org/openbravo/dal/security/SecurityChecker.java	Tue Jan 24 05:37:18 2017 +0000
+++ b/src/org/openbravo/dal/security/SecurityChecker.java	Fri Jan 27 12:04:54 2017 +0100
@@ -11,7 +11,7 @@
  * under the License. 
  * The Original Code is Openbravo ERP. 
  * The Initial Developer of the Original Code is Openbravo SLU 
- * All portions are Copyright (C) 2008-2016 Openbravo SLU 
+ * All portions are Copyright (C) 2008-2017 Openbravo SLU 
  * All Rights Reserved. 
  * Contributor(s):  ______________________________________.
  ************************************************************************
@@ -122,10 +122,12 @@
     }
 
     String orgId = "";
+    boolean isOrganization = false;
     if (obj instanceof OrganizationEnabled && ((OrganizationEnabled) obj).getOrganization() != null) {
       orgId = ((OrganizationEnabled) obj).getOrganization().getId();
     } else if (obj instanceof Organization) {
       orgId = ((Organization) obj).getId();
+      isOrganization = true;
     }
 
     final Entity entity = ((BaseOBObject) obj).getEntity();
@@ -154,8 +156,11 @@
         boolean checkOrgAccess = !(entity.getTableName().equals("AD_Role_OrgAccess")
             && OBContext.getOBContext().getRole().isClientAdmin() && OBContext.getOBContext()
             .getRole().getClient().getId().equals(clientId));
+        boolean notWritableOrganization = !obContext.getWritableOrganizations().contains(orgId);
+        boolean isDisabledOrganization = isOrganization
+            && obContext.getDeactivatedOrganizations().contains(orgId);
 
-        if (checkOrgAccess && !obContext.getWritableOrganizations().contains(orgId)) {
+        if (checkOrgAccess && notWritableOrganization && !isDisabledOrganization) {
           // TODO: maybe move rollback to exception throwing
           SessionHandler.getInstance().setDoRollback(true);
           throw new OBSecurityException("Organization " + orgId + " of object (" + obj