← Back to team overview

dhis2-devs team mailing list archive

[Branch ~dhis2-devs-core/dhis2/trunk] Rev 22199: Check on POST dataValues for approval locking.

 

------------------------------------------------------------
revno: 22199
committer: jimgrace@xxxxxxxxx
branch nick: dhis2
timestamp: Tue 2016-03-08 12:02:03 -0500
message:
  Check on POST dataValues for approval locking.
modified:
  dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataset/DataSetService.java
  dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataset/DefaultDataSetService.java
  dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataset/DataSetServiceTest.java
  dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/dbms/HibernateDbmsManager.java
  dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.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-api/src/main/java/org/hisp/dhis/dataset/DataSetService.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataset/DataSetService.java	2016-03-07 19:55:24 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataset/DataSetService.java	2016-03-08 17:02:03 +0000
@@ -333,10 +333,11 @@
      * @param dataElement the data element.
      * @param period the period.
      * @param organisationUnit the organisation unit.
+     * @param attributeOptionCombo the attribute option combo.
      * @param now the base date for deciding locked date, current date if null.
      * @return true or false indicating whether the system is locked or not.
      */
-    boolean isLocked( DataElement dataElement, Period period, OrganisationUnit organisationUnit, Date now );
+    boolean isLocked( DataElement dataElement, Period period, OrganisationUnit organisationUnit, DataElementCategoryOptionCombo attributeOptionCombo, Date now );
 
     /**
      * Take

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataset/DefaultDataSetService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataset/DefaultDataSetService.java	2016-03-07 19:55:24 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataset/DefaultDataSetService.java	2016-03-08 17:02:03 +0000
@@ -419,13 +419,28 @@
     }
 
     @Override
-    public boolean isLocked( DataElement dataElement, Period period, OrganisationUnit organisationUnit, Date now )
+    public boolean isLocked( DataElement dataElement, Period period, OrganisationUnit organisationUnit,
+        DataElementCategoryOptionCombo attributeOptionCombo, Date now )
     {
         now = now != null ? now : new Date();
 
         boolean expired = dataElement.isExpired( period, now );
-        
-        return expired && lockExceptionStore.getCount( dataElement, period, organisationUnit ) == 0L;
+
+        if ( expired && lockExceptionStore.getCount( dataElement, period, organisationUnit ) == 0L )
+        {
+            return true;
+        }
+
+        DataSet dataSet = dataElement.getDataSet();
+
+        if ( dataSet == null || dataSet.getWorkflow() == null )
+        {
+            return false;
+        }
+
+        DataApprovalStatus dataApprovalStatus = dataApprovalService.getDataApprovalStatus( dataSet.getWorkflow(), period, organisationUnit, attributeOptionCombo );
+
+        return dataApprovalStatus.getState().isApproved();
     }
     
     @Override

=== modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataset/DataSetServiceTest.java'
--- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataset/DataSetServiceTest.java	2016-01-04 02:27:49 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataset/DataSetServiceTest.java	2016-03-08 17:02:03 +0000
@@ -28,6 +28,8 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+import static com.google.common.collect.Lists.newArrayList;
+import static com.google.common.collect.Sets.newHashSet;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -35,26 +37,42 @@
 import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
+import java.util.Date;
 import java.util.List;
 
-import org.hisp.dhis.DhisSpringTest;
+import org.hisp.dhis.DhisTest;
+import org.hisp.dhis.dataapproval.DataApproval;
+import org.hisp.dhis.dataapproval.DataApprovalLevel;
+import org.hisp.dhis.dataapproval.DataApprovalLevelService;
+import org.hisp.dhis.dataapproval.DataApprovalService;
+import org.hisp.dhis.dataapproval.DataApprovalStore;
+import org.hisp.dhis.dataapproval.DataApprovalWorkflow;
+import org.hisp.dhis.dataapproval.DataApprovalWorkflowService;
 import org.hisp.dhis.dataelement.DataElement;
+import org.hisp.dhis.dataelement.DataElementCategoryOptionCombo;
 import org.hisp.dhis.dataelement.DataElementService;
+import org.hisp.dhis.mock.MockCurrentUserService;
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.organisationunit.OrganisationUnitService;
 import org.hisp.dhis.period.MonthlyPeriodType;
 import org.hisp.dhis.period.Period;
 import org.hisp.dhis.period.PeriodService;
 import org.hisp.dhis.period.PeriodType;
+import org.hisp.dhis.setting.SystemSettingManager;
+import org.hisp.dhis.user.CurrentUserService;
+import org.hisp.dhis.user.User;
+import org.hisp.dhis.user.UserAuthorityGroup;
+import org.hisp.dhis.user.UserService;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.jdbc.core.JdbcTemplate;
 
 /**
  * @author Lars Helge Overland
  */
 public class DataSetServiceTest
-    extends DhisSpringTest
+    extends DhisTest
 {
     private PeriodType periodType;
 
@@ -66,19 +84,47 @@
     private OrganisationUnit unitD;
     private OrganisationUnit unitE;
     private OrganisationUnit unitF;
-    
+
+    DataElementCategoryOptionCombo attributeOptionCombo;
+
+    private CurrentUserService mockCurrentUserService;
+
     @Autowired
     private DataSetService dataSetService;
     
     @Autowired
     private DataElementService dataElementService;
-    
+
     @Autowired
     private OrganisationUnitService organisationUnitService;
     
     @Autowired
     private PeriodService periodService;
-    
+
+    @Autowired
+    protected UserService _userService;
+
+    @Autowired
+    private DataApprovalService approvalService;
+
+    @Autowired
+    private CurrentUserService currentUserService;
+
+    @Autowired
+    private DataApprovalStore approvalStore;
+
+    @Autowired
+    private DataApprovalLevelService levelService;
+
+    @Autowired
+    private DataApprovalWorkflowService workflowService;
+
+    @Autowired
+    private JdbcTemplate jdbcTemplate;
+
+    @Autowired
+    private SystemSettingManager systemSettingManager;
+
     // -------------------------------------------------------------------------
     // Fixture
     // -------------------------------------------------------------------------
@@ -87,6 +133,8 @@
     public void setUpTest()
         throws Exception
     {
+        userService = _userService;
+
         periodType = new MonthlyPeriodType();
 
         period = createPeriod( periodType, getDate( 2000, 3, 1 ), getDate( 2000, 3, 31 ) );
@@ -105,6 +153,49 @@
         organisationUnitService.addOrganisationUnit( unitD );
         organisationUnitService.addOrganisationUnit( unitE );
         organisationUnitService.addOrganisationUnit( unitF );
+
+        attributeOptionCombo = categoryService.getDefaultDataElementCategoryOptionCombo();
+
+        mockCurrentUserService = new MockCurrentUserService( true, newHashSet( unitA ), newHashSet( unitA ), UserAuthorityGroup.AUTHORITY_ALL );
+        setDependency( approvalService, "currentUserService", mockCurrentUserService, CurrentUserService.class );
+        setDependency( approvalStore, "currentUserService", mockCurrentUserService, CurrentUserService.class );
+
+        User user = mockCurrentUserService.getCurrentUser();
+        user.setFirstName( "John" );
+        user.setSurname( "Doe" );
+        userService.addUser( mockCurrentUserService.getCurrentUser() );
+
+        jdbcTemplate.execute(
+            "CREATE TABLE _orgunitstructure " +
+                "(" +
+                "  organisationunitid integer NOT NULL, " +
+                "  organisationunituid character(11) NOT NULL, " +
+                "  level integer, " +
+                "  idlevel1 integer, " +
+                "  CONSTRAINT _orgunitstructure_pkey PRIMARY KEY (organisationunitid)" +
+                ");" );
+
+        jdbcTemplate.execute( "INSERT INTO _orgunitstructure VALUES (" + unitA.getId() + ", '" + unitA.getUid() + "', 1, " + unitA.getId() + ");" );
+        jdbcTemplate.execute( "INSERT INTO _orgunitstructure VALUES (" + unitB.getId() + ", '" + unitB.getUid() + "', 2, " + unitB.getId() + ");" );
+        jdbcTemplate.execute( "INSERT INTO _orgunitstructure VALUES (" + unitC.getId() + ", '" + unitC.getUid() + "', 3, " + unitC.getId() + ");" );
+        jdbcTemplate.execute( "INSERT INTO _orgunitstructure VALUES (" + unitD.getId() + ", '" + unitD.getUid() + "', 4, " + unitD.getId() + ");" );
+        jdbcTemplate.execute( "INSERT INTO _orgunitstructure VALUES (" + unitE.getId() + ", '" + unitE.getUid() + "', 3, " + unitE.getId() + ");" );
+        jdbcTemplate.execute( "INSERT INTO _orgunitstructure VALUES (" + unitF.getId() + ", '" + unitF.getUid() + "', 4, " + unitF.getId() + ");" );
+
+        systemSettingManager.invalidateCache();
+    }
+
+    @Override
+    public boolean emptyDatabaseAfterTest()
+    {
+        return true;
+    }
+
+    @Override
+    public void tearDownTest()
+    {
+        setDependency( approvalService, "currentUserService", currentUserService, CurrentUserService.class );
+        setDependency( approvalStore, "currentUserService", currentUserService, CurrentUserService.class );
     }
 
     // -------------------------------------------------------------------------
@@ -118,6 +209,22 @@
         assertEquals( periodType, dataSet.getPeriodType() );
     }
 
+    private void approveData( DataSet dataSet, Period period, OrganisationUnit unit )
+    {
+        DataApprovalLevel level = new DataApprovalLevel ("Level A", unit.getLevel(), null );
+        levelService.addDataApprovalLevel( level );
+
+        DataApprovalWorkflow workflow = new DataApprovalWorkflow( "Workflow A", period.getPeriodType(), newHashSet( level ) );
+        workflowService.addWorkflow( workflow );
+
+        dataSet.setWorkflow( workflow );
+        dataSetService.updateDataSet( dataSet );
+
+        User user = mockCurrentUserService.getCurrentUser();
+        DataApproval approval = new DataApproval( level, workflow, period, unit, attributeOptionCombo, false, new Date(), user );
+        approvalService.approveData( newArrayList( approval ) );
+    }
+
     // -------------------------------------------------------------------------
     // DataSet
     // -------------------------------------------------------------------------
@@ -317,11 +424,11 @@
         // Expiry days
         // ---------------------------------------------------------------------
 
-        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 1 ) ) );
-        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 5 ) ) );
-        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 15 ) ) );
-        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 25 ) ) );
-        assertFalse( dataSetService.isLocked( dataElementB, period, unitA, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 1 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 5 ) ) );
+        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 15 ) ) );
+        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
 
         // ---------------------------------------------------------------------
         // Lock exception
@@ -330,11 +437,23 @@
         LockException lockException = new LockException( period, unitA, dataSetA );
         dataSetService.addLockException( lockException );
 
-        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 1 ) ) );
-        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 5 ) ) );
-        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 15 ) ) );
-        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, getDate( 2000, 4, 25 ) ) );
-        assertFalse( dataSetService.isLocked( dataElementB, period, unitA, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 1 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 5 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 15 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+
+        // ---------------------------------------------------------------------
+        // Approved
+        // ---------------------------------------------------------------------
+
+        approveData( dataSetA, period, unitA );
+
+        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 1 ) ) );
+        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 5 ) ) );
+        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 15 ) ) );
+        assertTrue( dataSetService.isLocked( dataElementA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataElementB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
     }
 
     @Test
@@ -354,12 +473,12 @@
         // Expiry days
         // ---------------------------------------------------------------------
 
-        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 1 ) ) );
-        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 5 ) ) );
-        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 15 ) ) );
-        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 25 ) ) );
-        assertFalse( dataSetService.isLocked( dataSetB, period, unitA, null, getDate( 2000, 4, 10 ) ) );
-        assertTrue( dataSetService.isLocked( dataSetB, period, unitA, null, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 1 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 5 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 15 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 10 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
 
         // ---------------------------------------------------------------------
         // Lock exception
@@ -368,11 +487,24 @@
         LockException lockException = new LockException( period, unitA, dataSetA );
         dataSetService.addLockException( lockException );
 
-        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 1 ) ) );
-        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 5 ) ) );
-        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 15 ) ) );
-        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, null, getDate( 2000, 4, 25 ) ) );
-        assertFalse( dataSetService.isLocked( dataSetB, period, unitA, null, getDate( 2000, 4, 10 ) ) );
-        assertTrue( dataSetService.isLocked( dataSetB, period, unitA, null, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 1 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 5 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 15 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 10 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+
+        // ---------------------------------------------------------------------
+        // Approved
+        // ---------------------------------------------------------------------
+
+        approveData( dataSetA, period, unitA );
+
+        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 1 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 5 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 15 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetA, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
+        assertFalse( dataSetService.isLocked( dataSetB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 10 ) ) );
+        assertTrue( dataSetService.isLocked( dataSetB, period, unitA, attributeOptionCombo, getDate( 2000, 4, 25 ) ) );
     }
 }

=== modified file 'dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/dbms/HibernateDbmsManager.java'
--- dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/dbms/HibernateDbmsManager.java	2016-02-02 22:45:48 +0000
+++ dhis-2/dhis-support/dhis-support-hibernate/src/main/java/org/hisp/dhis/dbms/HibernateDbmsManager.java	2016-03-08 17:02:03 +0000
@@ -146,6 +146,8 @@
 
         emptyTable( "dataapproval" );
 
+        emptyTable( "lockexception" );
+
         emptyTable( "datasetsource" );
         emptyTable( "datasetmembers" );
         emptyTable( "datasetindicators" );

=== modified file 'dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.java'
--- dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.java	2016-02-10 16:39:06 +0000
+++ dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.java	2016-03-08 17:02:03 +0000
@@ -198,7 +198,7 @@
         // Locking validation
         // ---------------------------------------------------------------------
 
-        validateDataSetNotLocked( dataElement, period, organisationUnit );
+        validateDataSetNotLocked( dataElement, period, organisationUnit, attributeOptionCombo );
 
         // ---------------------------------------------------------------------
         // Assemble and save data value
@@ -333,7 +333,7 @@
         // Locking validation
         // ---------------------------------------------------------------------
 
-        validateDataSetNotLocked( dataElement, period, organisationUnit );
+        validateDataSetNotLocked( dataElement, period, organisationUnit, attributeOptionCombo );
 
         // ---------------------------------------------------------------------
         // Delete data value
@@ -382,7 +382,7 @@
         // Locking validation
         // ---------------------------------------------------------------------
 
-        validateDataSetNotLocked( dataElement, period, organisationUnit );
+        validateDataSetNotLocked( dataElement, period, organisationUnit, attributeOptionCombo );
 
         // ---------------------------------------------------------------------
         // Get data value
@@ -440,7 +440,7 @@
         // Locking validation
         // ---------------------------------------------------------------------
 
-        validateDataSetNotLocked( dataElement, period, organisationUnit );
+        validateDataSetNotLocked( dataElement, period, organisationUnit, attributeOptionCombo );
 
         // ---------------------------------------------------------------------
         // Get data value
@@ -633,10 +633,11 @@
         }
     }
 
-    private void validateDataSetNotLocked( DataElement dataElement, Period period, OrganisationUnit organisationUnit )
+    private void validateDataSetNotLocked( DataElement dataElement, Period period,
+        OrganisationUnit organisationUnit, DataElementCategoryOptionCombo attributeOptionCombo )
         throws WebMessageException
     {
-        if ( dataSetService.isLocked( dataElement, period, organisationUnit, null ) )
+        if ( dataSetService.isLocked( dataElement, period, organisationUnit, attributeOptionCombo, null ) )
         {
             throw new WebMessageException( WebMessageUtils.conflict( "Data set is locked" ) );
         }