← Back to team overview

dhis2-devs team mailing list archive

[Branch ~dhis2-devs-core/dhis2/trunk] Rev 15968: Fix unapproved data hiding for some cases.

 

------------------------------------------------------------
revno: 15968
committer: jimgrace@xxxxxxxxx
branch nick: dhis2
timestamp: Sat 2014-07-05 09:27:29 +0300
message:
  Fix unapproved data hiding for some cases.
modified:
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalLevelService.java
  dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalLevelServiceTest.java


--
lp:dhis2
https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk

Your team DHIS 2 developers is subscribed to branch lp:dhis2.
To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalLevelService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalLevelService.java	2014-04-28 13:20:36 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalLevelService.java	2014-07-05 06:27:29 +0000
@@ -36,6 +36,9 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.hisp.dhis.dataelement.CategoryOptionGroup;
 import org.hisp.dhis.dataelement.CategoryOptionGroupSet;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
@@ -44,6 +47,7 @@
 import org.hisp.dhis.security.SecurityService;
 import org.hisp.dhis.user.CurrentUserService;
 import org.hisp.dhis.user.User;
+import org.hisp.dhis.user.UserCredentials;
 import org.springframework.transaction.annotation.Transactional;
 
 /**
@@ -53,6 +57,8 @@
 public class DefaultDataApprovalLevelService
     implements DataApprovalLevelService
 {
+    private final static Log log = LogFactory.getLog( DefaultDataApprovalService.class );
+
     // -------------------------------------------------------------------------
     // Dependencies
     // -------------------------------------------------------------------------
@@ -451,7 +457,7 @@
         {
             if ( level.getOrgUnitLevel() >= orgUnitLevel
                 && securityService.canRead( level )
-                && ( level.getCategoryOptionGroupSet() == null || canReadSomeCategory( level.getCategoryOptionGroupSet() ) )
+                && canReadCOGS( level.getCategoryOptionGroupSet() )
                 && level.getLevel() < getAllDataApprovalLevels().size() )
             {
                 required = level.getLevel() + 1;
@@ -463,14 +469,28 @@
     }
 
     /**
-     * Can the user read at least one category from inside a category option
-     * group set?
+     * Can the user read from this CategoryOptionGroupSet (COGS)?
+     * <p>
+     * If the COGS is null, then the user must have no dimension constraints.
+     * (In other words, the user must be able to read across all category
+     * option groups.)
+     * <p>
+     * If the COGS is not null, then the user must be able to read at least
+     * one category option group from the category option group set.
      *
      * @param cogs The category option group set to test
      * @return true if user can read at least one category option group.
      */
-    private boolean canReadSomeCategory( CategoryOptionGroupSet cogs )
+    private boolean canReadCOGS( CategoryOptionGroupSet cogs )
     {
+        if ( cogs == null )
+        {
+            UserCredentials userCredentials = currentUserService.getCurrentUser().getUserCredentials();
+
+            return CollectionUtils.isEmpty( userCredentials.getCogsDimensionConstraints() )
+                && CollectionUtils.isEmpty( userCredentials.getCatDimensionConstraints() );
+        }
+
         for ( CategoryOptionGroup cog : cogs.getMembers() )
         {
             if ( securityService.canRead(cog) )

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalLevelServiceTest.java'
--- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalLevelServiceTest.java	2014-04-28 18:13:09 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalLevelServiceTest.java	2014-07-05 06:27:29 +0000
@@ -524,40 +524,15 @@
         assertEquals( 4, (int) readApprovalLevels.get( organisationUnitF ) );
         assertEquals( 3, (int) readApprovalLevels.get( organisationUnitB ) );
     }
-
-    /*
+/*
     @Test
     public void testGetUserReadApprovalLevels_2A() throws Exception
     {
         //
         // Test 2... TBD
         //
-        organisationUnitService.addOrganisationUnit( organisationUnitE, false );
-        organisationUnitService.addOrganisationUnit( organisationUnitF, false );
-        organisationUnitService.addOrganisationUnit( organisationUnitG, false );
-        organisationUnitService.addOrganisationUnit( organisationUnitH, false );
-
-        dataApprovalLevelService.addDataApprovalLevel( level1 );
-        dataApprovalLevelService.addDataApprovalLevel( level2 );
-        dataApprovalLevelService.addDataApprovalLevel( level3 );
-        dataApprovalLevelService.addDataApprovalLevel( level4 );
-        dataApprovalLevelService.addDataApprovalLevel( level5 );
-
-        Set<OrganisationUnit> assignedOrgUnits = new HashSet<OrganisationUnit>();
-        assignedOrgUnits.add( organisationUnitC );
-
-        Set<OrganisationUnit> dataViewOrgUnits = new HashSet<OrganisationUnit>();
-        dataViewOrgUnits.add( organisationUnitB );
-
-        createUserAndInjectSecurityContext( assignedOrgUnits, dataViewOrgUnits, false );
-
-        Map<OrganisationUnit, Integer> readApprovalLevels = dataApprovalLevelService.getUserReadApprovalLevels();
-        assertEquals( 2, readApprovalLevels.size() );
-
-        assertEquals( 4, (int) readApprovalLevels.get( organisationUnitC ) );
-        assertEquals( 3, (int) readApprovalLevels.get( organisationUnitB ) );
     }
-
+*/
     @Test
     public void testGetUserDataApprovalLevelsApproveHere() throws Exception
     {
@@ -574,9 +549,14 @@
         dataApprovalLevelService.addDataApprovalLevel( level1A );
         dataApprovalLevelService.addDataApprovalLevel( level1 );
 
-        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
-        units.add( organisationUnitB );
-        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE );
+        Set<OrganisationUnit> assignedOrgUnits = new HashSet<OrganisationUnit>();
+        assignedOrgUnits.add( organisationUnitB );
+
+        Set<OrganisationUnit> dataViewOrgUnits = new HashSet<OrganisationUnit>();
+        dataViewOrgUnits.add( organisationUnitB );
+
+        CurrentUserService currentUserService = new MockCurrentUserService( assignedOrgUnits, dataViewOrgUnits, DataApproval.AUTH_APPROVE );
+        setDependency( dataApprovalLevelService, "currentUserService", currentUserService, CurrentUserService.class );
 
         List<DataApprovalLevel> levels = dataApprovalLevelService.getUserDataApprovalLevels();
 
@@ -603,9 +583,14 @@
         dataApprovalLevelService.addDataApprovalLevel( level1A );
         dataApprovalLevelService.addDataApprovalLevel( level1 );
 
-        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
-        units.add( organisationUnitB );
-        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+        Set<OrganisationUnit> assignedOrgUnits = new HashSet<OrganisationUnit>();
+        assignedOrgUnits.add( organisationUnitB );
+
+        Set<OrganisationUnit> dataViewOrgUnits = new HashSet<OrganisationUnit>();
+        dataViewOrgUnits.add( organisationUnitB );
+
+        CurrentUserService currentUserService = new MockCurrentUserService( assignedOrgUnits, dataViewOrgUnits, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+        setDependency( dataApprovalLevelService, "currentUserService", currentUserService, CurrentUserService.class );
 
         List<DataApprovalLevel> levels = dataApprovalLevelService.getUserDataApprovalLevels();
         
@@ -636,9 +621,14 @@
         dataApprovalLevelService.addDataApprovalLevel( level1A );
         dataApprovalLevelService.addDataApprovalLevel( level1 );
 
-        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
-        units.add( organisationUnitB );
-        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+        Set<OrganisationUnit> assignedOrgUnits = new HashSet<OrganisationUnit>();
+        assignedOrgUnits.add( organisationUnitB );
+
+        Set<OrganisationUnit> dataViewOrgUnits = new HashSet<OrganisationUnit>();
+        dataViewOrgUnits.add( organisationUnitB );
+
+        CurrentUserService currentUserService = new MockCurrentUserService( assignedOrgUnits, dataViewOrgUnits, DataApproval.AUTH_APPROVE, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+        setDependency( dataApprovalLevelService, "currentUserService", currentUserService, CurrentUserService.class );
 
         List<DataApprovalLevel> levels = dataApprovalLevelService.getUserDataApprovalLevels();
         
@@ -670,9 +660,14 @@
         dataApprovalLevelService.addDataApprovalLevel( level1A );
         dataApprovalLevelService.addDataApprovalLevel( level1 );
 
-        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
-        units.add( organisationUnitB );
-        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_ACCEPT_LOWER_LEVELS );
+        Set<OrganisationUnit> assignedOrgUnits = new HashSet<OrganisationUnit>();
+        assignedOrgUnits.add( organisationUnitB );
+
+        Set<OrganisationUnit> dataViewOrgUnits = new HashSet<OrganisationUnit>();
+        dataViewOrgUnits.add( organisationUnitB );
+
+        CurrentUserService currentUserService = new MockCurrentUserService( assignedOrgUnits, dataViewOrgUnits, DataApproval.AUTH_ACCEPT_LOWER_LEVELS );
+        setDependency( dataApprovalLevelService, "currentUserService", currentUserService, CurrentUserService.class );
 
         List<DataApprovalLevel> levels = dataApprovalLevelService.getUserDataApprovalLevels();
         
@@ -681,6 +676,4 @@
         assertEquals( "2B", levels.get( 1 ).getName() );
         assertEquals( "03", levels.get( 2 ).getName() );
     }
-    */
-
 }