=== 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	2013-12-27 16:53:58 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java	2013-12-27 17:35:45 +0000
@@ -206,7 +206,7 @@
      * also on whether there are higher-level approvals that the user is
      * authorized to unapprove.
      *
-     * @param source OrganisationUnit to check for approval.
+     * @param organisationUnit OrganisationUnit to check for approval.
      * @return true if the user may approve, otherwise false
      */
     private boolean isAuthorizedToUnapprove( OrganisationUnit organisationUnit )

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.java'
--- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.java	2013-12-27 14:02:54 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.java	2013-12-27 17:35:45 +0000
@@ -96,6 +96,10 @@
 
     private OrganisationUnit organisationUnitD;
 
+    private OrganisationUnit organisationUnitE;
+
+    private OrganisationUnit organisationUnitF;
+
     private User userA;
 
     private User userB;
@@ -130,15 +134,30 @@
         periodService.addPeriod( periodA );
         periodService.addPeriod( periodB );
 
+        //
+        // Organisation unit hierarchy:
+        //
+        //      A
+        //      |
+        //      B
+        //     / \
+        //    C   E
+        //    |   |
+        //    D   F
+        //
         organisationUnitA = createOrganisationUnit( 'A' );
         organisationUnitB = createOrganisationUnit( 'B', organisationUnitA );
         organisationUnitC = createOrganisationUnit( 'C', organisationUnitB );
         organisationUnitD = createOrganisationUnit( 'D', organisationUnitC );
+        organisationUnitE = createOrganisationUnit( 'E', organisationUnitB );
+        organisationUnitF = createOrganisationUnit( 'F', organisationUnitE );
 
         organisationUnitService.addOrganisationUnit( organisationUnitA );
         organisationUnitService.addOrganisationUnit( organisationUnitB );
         organisationUnitService.addOrganisationUnit( organisationUnitC );
         organisationUnitService.addOrganisationUnit( organisationUnitD );
+        organisationUnitService.addOrganisationUnit( organisationUnitE );
+        organisationUnitService.addOrganisationUnit( organisationUnitF );
 
         userA = createUser( 'A' );
         userB = createUser( 'B' );
@@ -301,83 +320,228 @@
     }
 
     @Test
+    public void testGetDataApprovalStateWithMultipleChildren() throws Exception
+    {
+        dataSetA.setApproveData( true );
+
+        organisationUnitD.addDataSet( dataSetA );
+        organisationUnitF.addDataSet( dataSetA );
+
+        Date date = new Date();
+
+        assertEquals( DataApprovalState.WAITING_FOR_LOWER_LEVEL_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitB, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.WAITING_FOR_LOWER_LEVEL_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitC, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitD, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.WAITING_FOR_LOWER_LEVEL_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitE, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitF, attributeOptionCombo ) );
+
+        dataApprovalService.addDataApproval( new DataApproval( dataSetA, periodA, organisationUnitF, attributeOptionCombo, date, userA ) );
+
+        assertEquals( DataApprovalState.WAITING_FOR_LOWER_LEVEL_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitB, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.WAITING_FOR_LOWER_LEVEL_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitC, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitD, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitE, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitF, attributeOptionCombo ) );
+
+        dataApprovalService.addDataApproval( new DataApproval( dataSetA, periodA, organisationUnitD, attributeOptionCombo, date, userA ) );
+
+        assertEquals( DataApprovalState.WAITING_FOR_LOWER_LEVEL_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitB, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitC, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitD, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitE, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitF, attributeOptionCombo ) );
+
+        dataApprovalService.addDataApproval( new DataApproval( dataSetA, periodA, organisationUnitE, attributeOptionCombo, date, userA ) );
+
+        assertEquals( DataApprovalState.WAITING_FOR_LOWER_LEVEL_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitB, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitC, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitD, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitE, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitF, attributeOptionCombo ) );
+
+        dataApprovalService.addDataApproval( new DataApproval( dataSetA, periodA, organisationUnitC, attributeOptionCombo, date, userA ) );
+
+        assertEquals( DataApprovalState.READY_FOR_APPROVAL, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitB, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitC, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitD, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitE, attributeOptionCombo ) );
+        assertEquals( DataApprovalState.APPROVED, dataApprovalService.getDataApprovalState( dataSetA, periodA, organisationUnitF, attributeOptionCombo ) );
+    }
+
+    @Test
     public void testMayApprove() throws Exception
     {
         Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
-        units.add( organisationUnitB );        
+        units.add( organisationUnitC );
         createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE );
 
         assertEquals( false, dataApprovalService.mayApprove( organisationUnitA ) );
+        assertEquals( false, dataApprovalService.mayApprove( organisationUnitB ) );
+        assertEquals( true, dataApprovalService.mayApprove( organisationUnitC ) );
+        assertEquals( false, dataApprovalService.mayApprove( organisationUnitD ) );
+    }
+
+    @Test
+    public void testMayApproveLowerLevels() throws Exception
+    {
+        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
+        units.add( organisationUnitB );
+        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+
+        assertEquals( false, dataApprovalService.mayApprove( organisationUnitA ) );
+        assertEquals( false, dataApprovalService.mayApprove( organisationUnitB ) );
+        assertEquals( true, dataApprovalService.mayApprove( organisationUnitC ) );
+        assertEquals( true, dataApprovalService.mayApprove( organisationUnitD ) );
+    }
+
+    @Test
+    public void testMayApproveSameAndLowerLevels() throws Exception
+    {
+        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
+        units.add( organisationUnitB );
+        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+
+        assertEquals( false, dataApprovalService.mayApprove( organisationUnitA ) );
         assertEquals( true, dataApprovalService.mayApprove( organisationUnitB ) );
-        assertEquals( false, dataApprovalService.mayApprove( organisationUnitC ) );
-        assertEquals( false, dataApprovalService.mayApprove( organisationUnitD ) );
+        assertEquals( true, dataApprovalService.mayApprove( organisationUnitC ) );
+        assertEquals( true, dataApprovalService.mayApprove( organisationUnitD ) );
     }
 
     @Test
-    public void testMayApproveLowerLevels() throws Exception
+    public void testMayApproveNoAuthority() throws Exception
     {
         Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
         units.add( organisationUnitC );
-        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+        createUserAndInjectSecurityContext( units, false );
 
+        assertEquals( false, dataApprovalService.mayApprove( organisationUnitA ) );
         assertEquals( false, dataApprovalService.mayApprove( organisationUnitB ) );
         assertEquals( false, dataApprovalService.mayApprove( organisationUnitC ) );
-        assertEquals( true, dataApprovalService.mayApprove( organisationUnitD ) );
+        assertEquals( false, dataApprovalService.mayApprove( organisationUnitD ) );
     }
-    
-    //TODO better test coverage
-    //TODO split testMayUnapprove into multiple tests with an injected user in each
-    //TODO remove ignore from testMayUnapprove
-    
+
     @Test
-    @Ignore
     public void testMayUnapprove() throws Exception
     {
-        userA.addOrganisationUnit( organisationUnitA );
-        userB.addOrganisationUnit( organisationUnitB );
-
-        Date date = new Date();
-        DataApproval dataApprovalA = new DataApproval( dataSetA, periodA, organisationUnitA, attributeOptionCombo, date, userA );
-        DataApproval dataApprovalB = new DataApproval( dataSetA, periodA, organisationUnitB, attributeOptionCombo, date, userA );
-        DataApproval dataApprovalC = new DataApproval( dataSetA, periodA, organisationUnitC, attributeOptionCombo, date, userA );
-        DataApproval dataApprovalD = new DataApproval( dataSetA, periodA, organisationUnitD, attributeOptionCombo, date, userA );
-
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalC ) );
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalD ) );
-
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
-
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalB ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
-
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalB ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
-
-        // If the organisation unit has no parent:
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalA ) );
-
-        dataApprovalService.addDataApproval( dataApprovalB );
-        dataApprovalService.addDataApproval( dataApprovalC );
-        dataApprovalService.addDataApproval( dataApprovalD );
-
-        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalB ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
-        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
-
-        dataApprovalService.addDataApproval( dataApprovalA );
+        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
+        units.add( organisationUnitB );
+        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE );
+
+        Date date = new Date();
+        DataApproval dataApprovalA = new DataApproval( dataSetA, periodA, organisationUnitA, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalB = new DataApproval( dataSetA, periodA, organisationUnitB, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalC = new DataApproval( dataSetA, periodA, organisationUnitC, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalD = new DataApproval( dataSetA, periodA, organisationUnitD, attributeOptionCombo, date, userA );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
+
+        // These approvals will not block un-approving
+        dataApprovalService.addDataApproval( dataApprovalB );
+        dataApprovalService.addDataApproval( dataApprovalC );
+        dataApprovalService.addDataApproval( dataApprovalD );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
+
+        // Approval at A will block them all
+        dataApprovalService.addDataApproval( dataApprovalA );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalD ) );
+    }
+
+    @Test
+    public void testMayUnapproveLowerLevels() throws Exception
+    {
+        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
+        units.add( organisationUnitB );
+        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+
+        Date date = new Date();
+        DataApproval dataApprovalA = new DataApproval( dataSetA, periodA, organisationUnitA, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalB = new DataApproval( dataSetA, periodA, organisationUnitB, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalC = new DataApproval( dataSetA, periodA, organisationUnitC, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalD = new DataApproval( dataSetA, periodA, organisationUnitD, attributeOptionCombo, date, userA );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
+
+        // These approvals will not block un-approving
+        dataApprovalService.addDataApproval( dataApprovalC );
+        dataApprovalService.addDataApproval( dataApprovalD );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
+
+        // Approval at B will block them all
+        dataApprovalService.addDataApproval( dataApprovalB );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalD ) );
+    }
+
+    @Test
+    public void testMayUnapproveSameAndLowerLevels() throws Exception
+    {
+        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
+        units.add( organisationUnitB );
+        createUserAndInjectSecurityContext( units, false, DataApproval.AUTH_APPROVE );
+
+        Date date = new Date();
+        DataApproval dataApprovalA = new DataApproval( dataSetA, periodA, organisationUnitA, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalB = new DataApproval( dataSetA, periodA, organisationUnitB, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalC = new DataApproval( dataSetA, periodA, organisationUnitC, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalD = new DataApproval( dataSetA, periodA, organisationUnitD, attributeOptionCombo, date, userA );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
+
+        // These approvals will not block un-approving
+        dataApprovalService.addDataApproval( dataApprovalB );
+        dataApprovalService.addDataApproval( dataApprovalC );
+        dataApprovalService.addDataApproval( dataApprovalD );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( true, dataApprovalService.mayUnapprove( dataApprovalD ) );
+
+        // Approval at A will block them all
+        dataApprovalService.addDataApproval( dataApprovalA );
+
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalC ) );
+        assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalD ) );
+    }
+
+    @Test
+    public void testMayUnapproveNoAuthority() throws Exception
+    {
+        Set<OrganisationUnit> units = new HashSet<OrganisationUnit>();
+        units.add( organisationUnitB );
+        createUserAndInjectSecurityContext( units, false );
+
+        Date date = new Date();
+        DataApproval dataApprovalA = new DataApproval( dataSetA, periodA, organisationUnitA, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalB = new DataApproval( dataSetA, periodA, organisationUnitB, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalC = new DataApproval( dataSetA, periodA, organisationUnitC, attributeOptionCombo, date, userA );
+        DataApproval dataApprovalD = new DataApproval( dataSetA, periodA, organisationUnitD, attributeOptionCombo, date, userA );
 
         assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalA ) );
         assertEquals( false, dataApprovalService.mayUnapprove( dataApprovalB ) );