dhis2-devs team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 17474
committer: jimgrace@xxxxxxxxx
branch nick: dhis2
timestamp: Fri 2014-11-14 15:34:54 -0500
message:
  Approval fixes.
modified:
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DataApprovalPermissionsEvaluator.java
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.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/DataApprovalPermissionsEvaluator.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DataApprovalPermissionsEvaluator.java	2014-11-11 12:55:12 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DataApprovalPermissionsEvaluator.java	2014-11-14 20:34:54 +0000
@@ -33,6 +33,9 @@
 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.hisp.dhis.setting.SystemSettingManager;
 import org.hisp.dhis.user.CurrentUserService;
 import org.hisp.dhis.user.User;
@@ -52,7 +55,9 @@
  * @author Jim Grace
  */
 class DataApprovalPermissionsEvaluator
-{    
+{
+    private final static Log log = LogFactory.getLog( DataApprovalPermissionsEvaluator.class );
+
     private DataApprovalLevelService dataApprovalLevelService;
 
     private User user;
@@ -103,7 +108,7 @@
 
         ev.maxApprovalLevel = dataApprovalLevelService.getAllDataApprovalLevels().size();
 
-        tracePrint( "makePermissionsEvaluator acceptanceRequiredForApproval " + ev.acceptanceRequiredForApproval
+        log.debug( "makePermissionsEvaluator acceptanceRequiredForApproval " + ev.acceptanceRequiredForApproval
                 + " hideUnapprovedData " + ev.hideUnapprovedData + " authorizedToApprove " + ev.authorizedToApprove
                 + " authorizedToAcceptAtLowerLevels " + ev.authorizedToAcceptAtLowerLevels
                 + " authorizedToViewUnapprovedData " + ev.authorizedToViewUnapprovedData + " maxApprovalLevel " + ev.maxApprovalLevel );
@@ -130,7 +135,7 @@
 
         if ( da == null || da.getOrganisationUnit() == null )
         {
-            tracePrint( "getPermissions da " + da + ( da != null ? ( " " + da.getOrganisationUnit() ) : "" ) );
+            log.debug( "getPermissions da " + da + ( da != null ? ( " " + da.getOrganisationUnit() ) : "" ) );
 
             return permissions; // No permissions are set.
         }
@@ -147,6 +152,10 @@
                 }
             } ) );
         }
+        catch ( CacheLoader.InvalidCacheLoadException ex )
+        {
+            userApprovalLevel = null; // Google cache doesn't like to cache a null value even when that's what we need.
+        }
         catch ( ExecutionException ex )
         {
             throw new RuntimeException( ex );
@@ -154,7 +163,7 @@
 
         if ( userApprovalLevel == null )
         {
-            tracePrint( "getPermissions userApprovalLevel null for user " + ( user == null ? "(null)" : user.getUsername() ) + " orgUnit " +  da.getOrganisationUnit().getName() );
+            log.debug( "getPermissions userApprovalLevel null for user " + ( user == null ? "(null)" : user.getUsername() ) + " orgUnit " +  da.getOrganisationUnit().getName() );
 
             return permissions; // Can't find user approval level, so no permissions are set.
         }
@@ -163,13 +172,18 @@
 
         int dataLevel = ( da.getDataApprovalLevel() != null ? da.getDataApprovalLevel().getLevel() : maxApprovalLevel );
 
+        int nextApproveDataLevel = s.isApproved() ? dataLevel - 1 : dataLevel;
+
         boolean mayApproveOrUnapproveAtLevel = ( authorizedToApprove && userLevel == dataLevel && !da.isAccepted() ) ||
                         ( authorizedToApproveAtLowerLevels && userLevel < dataLevel );
 
+        boolean mayApproveAtNextLevel = ( s == DataApprovalState.ACCEPTED_HERE || ( s == DataApprovalState.APPROVED_HERE && ! acceptanceRequiredForApproval ) )
+                && ( ( authorizedToApprove && userLevel == nextApproveDataLevel ) || ( authorizedToApproveAtLowerLevels && userLevel < nextApproveDataLevel ) );
+
         boolean mayAcceptOrUnacceptAtLevel = authorizedToAcceptAtLowerLevels &&
                 ( userLevel == dataLevel - 1 || ( userLevel < dataLevel && authorizedToApproveAtLowerLevels ) );
 
-        boolean mayApprove = s.isApprovable() && mayApproveOrUnapproveAtLevel;
+        boolean mayApprove = ( s.isApprovable() && mayApproveOrUnapproveAtLevel ) || mayApproveAtNextLevel;
 
         boolean mayUnapprove = s.isUnapprovable() && ( ( mayApproveOrUnapproveAtLevel && !da.isAccepted() ) || mayAcceptOrUnacceptAtLevel );
 
@@ -180,17 +194,15 @@
         boolean mayReadData = authorizedToViewUnapprovedData || !hideUnapprovedData || mayApprove
                 || userLevel >= dataLevel;
 
-        /*
-        tracePrint( "getPermissions orgUnit " + ( da.getOrganisationUnit() == null ? "(null)" : da.getOrganisationUnit().getName() )
+        log.debug( "getPermissions orgUnit " + ( da.getOrganisationUnit() == null ? "(null)" : da.getOrganisationUnit().getName() )
                 + " combo " + da.getAttributeOptionCombo().getName() + " state " + s.name()
-                + " isApproved " + s.isApproved()+ " isApprovable " + s.isApprovable()+ " isUnapprovable " + s.isUnapprovable()
+                + " isApproved " + s.isApproved() + " isApprovable " + s.isApprovable() + " isUnapprovable " + s.isUnapprovable()
                 + " isAccepted " + s.isAccepted() + " isAcceptable " + s.isAcceptable() + " isUnacceptable " + s.isUnacceptable()
                 + " userLevel " + userLevel + " dataLevel " + dataLevel
                 + " mayApproveOrUnapproveAtLevel " + mayApproveOrUnapproveAtLevel + " mayAcceptOrUnacceptAtLevel " + mayAcceptOrUnacceptAtLevel
                 + " mayApprove " + mayApprove + " mayUnapprove " + mayUnapprove
                 + " mayAccept " + mayAccept + " mayUnaccept " + mayUnaccept
                 + " mayReadData " + mayReadData );
-        */
 
         permissions.setMayApprove( mayApprove );
         permissions.setMayUnapprove( mayUnapprove );
@@ -200,9 +212,4 @@
 
         return permissions;
     }
-
-    private static void tracePrint( String s )
-    {
-        //System.out.println( s );
-    }
 }

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java	2014-11-11 13:42:06 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java	2014-11-14 20:34:54 +0000
@@ -120,9 +120,17 @@
         {
             DataApprovalStatus status = getStatus( da, statusMap );
 
-            if ( da.getDataApprovalLevel() == null )
+            if ( da.getDataApprovalLevel() == null ) // Determine the approval level.
             {
-                da.setDataApprovalLevel( status.getDataApproval().getDataApprovalLevel() );
+                if ( status.getState().isApproved() ) // If approved already, approve at next level up (lower level number).
+                {
+                    da.setDataApprovalLevel( dataApprovalLevelService.getDataApprovalLevelByLevelNumber(
+                            status.getDataApproval().getDataApprovalLevel().getLevel() - 1 ) );
+                }
+                else
+                {
+                    da.setDataApprovalLevel( status.getDataApproval().getDataApprovalLevel() );
+                }
             }
 
             if ( status != null && status.getState().isApproved() &&