=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java 2015-03-31 17:17:03 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java 2015-06-01 19:49:10 +0000 @@ -454,8 +454,26 @@ */ Collection getOrganisationUnitByCoordinate( double longitude, double latitude, String topOrgUnitUid, Integer targetLevel ); - + + /** + * Indicates whether the given organisation unit is part of the hierarchy + * of the organisation units of the current user. + * + * @param organisationUnit the organisation unit. + * @return true if the given organisation unit is part of the hierarchy. + */ boolean isInUserHierarchy( OrganisationUnit organisationUnit ); + + /** + * Indicates whether the given organisation unit is part of the hierarchy + * of the given user organisation units. + * + * @param uid the uid of the organisation unit. + * @param organisationUnits the set of organisation units associated with a + * user. + * @return true if the organisation unit with the given uid is part of the hierarchy. + */ + boolean isInUserHierarchy( String uid, Set organisationUnits ); // ------------------------------------------------------------------------- // OrganisationUnitHierarchy === modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/CurrentUserService.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/CurrentUserService.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/CurrentUserService.java 2015-06-01 19:49:10 +0000 @@ -28,6 +28,9 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +import java.util.Set; + +import org.hisp.dhis.organisationunit.OrganisationUnit; /** * This interface defined methods for getting access to the currently logged in @@ -54,6 +57,12 @@ User getCurrentUser(); /** + * @return the data capture organisation units of the current user, empty set + * if no current user. + */ + Set getCurrentUserOrganisationUnits(); + + /** * @return true if the current logged in user has the ALL privileges set, false * otherwise. */ === modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/User.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/User.java 2015-05-29 15:01:41 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/User.java 2015-06-01 19:49:10 +0000 @@ -246,9 +246,10 @@ * of the organisation units of this user. * * @param organisationUnit the organisation unit. + * @param the user organisation units. * @return true if the given organisation unit is part of the hierarchy. */ - public boolean isInUserHierarchy( OrganisationUnit organisationUnit ) + public static boolean isInUserHierarchy( OrganisationUnit organisationUnit, Set organisationUnits ) { if ( organisationUnits == null ) { @@ -268,6 +269,18 @@ return false; } + /** + * Indicates whether the given organisation unit is part of the hierarchy + * of the organisation units of this user. + * + * @param organisationUnit the organisation unit. + * @return true if the given organisation unit is part of the hierarchy. + */ + public boolean isInUserHierarchy( OrganisationUnit organisationUnit ) + { + return User.isInUserHierarchy( organisationUnit, organisationUnits ); + } + public String getUsername() { return userCredentials != null ? userCredentials.getUsername() : null; === modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/DefaultOrganisationUnitService.java' --- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/DefaultOrganisationUnitService.java 2015-05-29 15:01:41 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/organisationunit/DefaultOrganisationUnitService.java 2015-06-01 19:49:10 +0000 @@ -763,6 +763,14 @@ return user != null ? user.isInUserHierarchy( organisationUnit ) : false; } + @Override + public boolean isInUserHierarchy( String uid, Set organisationUnits ) + { + OrganisationUnit organisationUnit = organisationUnitStore.getByUid( uid ); + + return User.isInUserHierarchy( organisationUnit, organisationUnits ); + } + // ------------------------------------------------------------------------- // OrganisationUnitHierarchy // ------------------------------------------------------------------------- === modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultCurrentUserService.java' --- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultCurrentUserService.java 2015-01-17 07:41:26 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultCurrentUserService.java 2015-06-01 19:49:10 +0000 @@ -28,6 +28,10 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +import java.util.HashSet; +import java.util.Set; + +import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.security.spring.AbstractSpringSecurityCurrentUserService; import org.springframework.transaction.annotation.Transactional; @@ -93,6 +97,14 @@ return userCredentials.isSuper(); } + + @Override + public Set getCurrentUserOrganisationUnits() + { + User user = getCurrentUser(); + + return user != null ? new HashSet( user.getOrganisationUnits() ) : new HashSet(); + } @Override public boolean currenUserIsAuthorized( String auth ) === modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/organisationunit/OrganisationUnitServiceTest.java' --- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/organisationunit/OrganisationUnitServiceTest.java 2015-05-29 15:01:41 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/organisationunit/OrganisationUnitServiceTest.java 2015-06-01 19:49:10 +0000 @@ -41,6 +41,7 @@ import java.util.Date; import java.util.Iterator; import java.util.List; +import java.util.Set; import org.hisp.dhis.DhisSpringTest; import org.hisp.dhis.user.User; @@ -981,7 +982,8 @@ organisationUnitService.addOrganisationUnit( ouG ); User user = createUser( 'A' ); - user.setOrganisationUnits( Sets.newHashSet( ouB ) ); + Set organisationUnits = Sets.newHashSet( ouB ); + user.setOrganisationUnits( organisationUnits ); assertTrue( user.isInUserHierarchy( ouB ) ); assertTrue( user.isInUserHierarchy( ouD ) ); @@ -991,5 +993,14 @@ assertFalse( user.isInUserHierarchy( ouC ) ); assertFalse( user.isInUserHierarchy( ouF ) ); assertFalse( user.isInUserHierarchy( ouG ) ); + + assertTrue( organisationUnitService.isInUserHierarchy( ouB.getUid(), organisationUnits ) ); + assertTrue( organisationUnitService.isInUserHierarchy( ouD.getUid(), organisationUnits ) ); + assertTrue( organisationUnitService.isInUserHierarchy( ouE.getUid(), organisationUnits ) ); + + assertFalse( organisationUnitService.isInUserHierarchy( ouA.getUid(), organisationUnits ) ); + assertFalse( organisationUnitService.isInUserHierarchy( ouC.getUid(), organisationUnits ) ); + assertFalse( organisationUnitService.isInUserHierarchy( ouF.getUid(), organisationUnits ) ); + assertFalse( organisationUnitService.isInUserHierarchy( ouG.getUid(), organisationUnits ) ); } } === modified file 'dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataValueSetService.java' --- dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataValueSetService.java 2015-05-29 15:24:03 +0000 +++ dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/datavalueset/DefaultDataValueSetService.java 2015-06-01 19:49:10 +0000 @@ -46,6 +46,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; import org.amplecode.quick.BatchHandler; import org.amplecode.quick.BatchHandlerFactory; @@ -145,9 +146,16 @@ @Autowired private Notifier notifier; + // Set methods for test purposes + public void setBatchHandlerFactory( BatchHandlerFactory batchHandlerFactory ) { - this.batchHandlerFactory = batchHandlerFactory; // Test purpose + this.batchHandlerFactory = batchHandlerFactory; + } + + public void setCurrentUserService( CurrentUserService currentUserService ) + { + this.currentUserService = currentUserService; } //-------------------------------------------------------------------------- @@ -630,6 +638,7 @@ CachingMap orgUnitMap = new CachingMap<>(); CachingMap optionComboMap = new CachingMap<>(); CachingMap periodMap = new CachingMap<>(); + CachingMap orgUnitInHierarchyMap = new CachingMap<>(); //---------------------------------------------------------------------- // Load meta-data maps @@ -708,8 +717,9 @@ summary.setDataSetComplete( Boolean.FALSE.toString() ); } - String currentUser = currentUserService.getCurrentUsername(); - + final String currentUser = currentUserService.getCurrentUsername(); + final Set currentOrgUnits = currentUserService.getCurrentUserOrganisationUnits(); + BatchHandler batchHandler = batchHandlerFactory.createBatchHandler( DataValueBatchHandler.class ).init(); int importCount = 0; @@ -733,10 +743,10 @@ totalCount++; - DataElement dataElement = dataElementMap.get( trimToNull( dataValue.getDataElement() ), dataElementCallable.setId( trimToNull( dataValue.getDataElement() ) ) ); - Period period = outerPeriod != null ? outerPeriod : + final DataElement dataElement = dataElementMap.get( trimToNull( dataValue.getDataElement() ), dataElementCallable.setId( trimToNull( dataValue.getDataElement() ) ) ); + final Period period = outerPeriod != null ? outerPeriod : periodMap.get( trimToNull( dataValue.getPeriod() ), periodCallable.setId( trimToNull( dataValue.getPeriod() ) ) ); - OrganisationUnit orgUnit = outerOrgUnit != null ? outerOrgUnit : + final OrganisationUnit orgUnit = outerOrgUnit != null ? outerOrgUnit : orgUnitMap.get( trimToNull( dataValue.getOrgUnit() ), orgUnitCallable.setId( trimToNull( dataValue.getOrgUnit() ) ) ); DataElementCategoryOptionCombo categoryOptionCombo = optionComboMap.get( trimToNull( dataValue.getCategoryOptionCombo() ), optionComboCallable.setId( trimToNull( dataValue.getCategoryOptionCombo() ) ) ); @@ -786,6 +796,20 @@ { attrOptionCombo = fallbackCategoryOptionCombo; } + + boolean inUserHierarchy = orgUnitInHierarchyMap.get( orgUnit.getUid(), new Callable() + { + public Boolean call() throws Exception + { + return organisationUnitService.isInUserHierarchy( orgUnit.getUid(), currentOrgUnits ); + } + } ); + + if ( !inUserHierarchy ) + { + summary.getConflicts().add( new ImportConflict( dataValue.getOrgUnit(), "Organisation unit not in hierarchy of current user: " + currentUser ) ); + continue; + } if ( dataValue.getValue() == null && dataValue.getComment() == null ) { === modified file 'dhis-2/dhis-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/datavalueset/DataValueSetServiceTest.java' --- dhis-2/dhis-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/datavalueset/DataValueSetServiceTest.java 2015-05-28 20:16:59 +0000 +++ dhis-2/dhis-services/dhis-service-dxf2/src/test/java/org/hisp/dhis/dxf2/datavalueset/DataValueSetServiceTest.java 2015-06-01 19:49:10 +0000 @@ -56,6 +56,7 @@ import org.hisp.dhis.dxf2.importsummary.ImportSummary; import org.hisp.dhis.dxf2.common.ImportOptions; import org.hisp.dhis.jdbc.batchhandler.DataValueBatchHandler; +import org.hisp.dhis.mock.MockCurrentUserService; import org.hisp.dhis.mock.batchhandler.MockBatchHandler; import org.hisp.dhis.mock.batchhandler.MockBatchHandlerFactory; import org.hisp.dhis.organisationunit.OrganisationUnit; @@ -64,10 +65,14 @@ import org.hisp.dhis.period.Period; import org.hisp.dhis.period.PeriodService; import org.hisp.dhis.period.PeriodType; +import org.hisp.dhis.user.CurrentUserService; +import org.hisp.dhis.user.User; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.io.ClassPathResource; +import com.google.common.collect.Sets; + /** * @author Lars Helge Overland */ @@ -110,9 +115,12 @@ private DataSet dsA; private OrganisationUnit ouA; private OrganisationUnit ouB; + private OrganisationUnit ouC; private Period peA; private Period peB; + private User user; + private InputStream in; private MockBatchHandler mockDataValueBatchHandler = null; @@ -141,6 +149,7 @@ dsA = createDataSet( 'A', new MonthlyPeriodType() ); ouA = createOrganisationUnit( 'A' ); ouB = createOrganisationUnit( 'B' ); + ouC = createOrganisationUnit( 'C' ); peA = createPeriod( PeriodType.getByNameIgnoreCase( MonthlyPeriodType.NAME ), getDate( 2012, 1, 1 ), getDate( 2012, 1, 31 ) ); peB = createPeriod( PeriodType.getByNameIgnoreCase( MonthlyPeriodType.NAME ), getDate( 2012, 2, 1 ), getDate( 2012, 2, 29 ) ); @@ -152,6 +161,7 @@ dsA.setUid( "pBOMPrpg1QX" ); ouA.setUid( "DiszpKrYNg8" ); ouB.setUid( "BdfsJfj87js" ); + ouC.setUid( "j7Hg26FpoIa" ); ocA.setCode( "OC_A" ); ocB.setCode( "OC_B" ); @@ -162,6 +172,7 @@ dsA.setCode( "DS_A" ); ouA.setCode( "OU_A" ); ouB.setCode( "OU_B" ); + ouC.setCode( "OU_C" ); categoryService.addDataElementCategoryOption( categoryOptionA ); categoryService.addDataElementCategoryOption( categoryOptionB ); @@ -177,8 +188,14 @@ dataSetService.addDataSet( dsA ); organisationUnitService.addOrganisationUnit( ouA ); organisationUnitService.addOrganisationUnit( ouB ); + organisationUnitService.addOrganisationUnit( ouC ); periodService.addPeriod( peA ); - periodService.addPeriod( peB ); + periodService.addPeriod( peB ); + + user = createUser( 'A' ); + user.setOrganisationUnits( Sets.newHashSet( ouA, ouB ) ); + CurrentUserService currentUserService = new MockCurrentUserService( user ); + setDependency( dataValueSetService, "currentUserService", currentUserService ); } // ------------------------------------------------------------------------- @@ -196,7 +213,7 @@ assertNotNull( summary ); assertNotNull( summary.getImportCount() ); assertEquals( ImportStatus.SUCCESS, summary.getStatus() ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); Collection dataValues = mockDataValueBatchHandler.getInserts(); @@ -226,7 +243,7 @@ assertNotNull( summary ); assertNotNull( summary.getImportCount() ); assertEquals( ImportStatus.SUCCESS, summary.getStatus() ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); Collection dataValues = mockDataValueBatchHandler.getInserts(); @@ -253,7 +270,7 @@ ImportSummary summary = dataValueSetService.saveDataValueSet( in ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); assertEquals( 12, summary.getImportCount().getImported() ); assertEquals( 0, summary.getImportCount().getUpdated() ); assertEquals( 0, summary.getImportCount().getDeleted() ); @@ -272,7 +289,7 @@ ImportOptions options = new ImportOptions( CODE, CODE, CODE, false, true, NEW_AND_UPDATES, false ); ImportSummary summary = dataValueSetService.saveDataValueSet( in, options ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); assertEquals( 12, summary.getImportCount().getImported() ); assertEquals( 0, summary.getImportCount().getUpdated() ); assertEquals( 0, summary.getImportCount().getDeleted() ); @@ -291,7 +308,7 @@ ImportOptions options = new ImportOptions( CODE, CODE, CODE, false, false, NEW_AND_UPDATES, false ); ImportSummary summary = dataValueSetService.saveDataValueSet( in, options ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); assertEquals( 12, summary.getImportCount().getImported() ); assertEquals( 0, summary.getImportCount().getUpdated() ); assertEquals( 0, summary.getImportCount().getDeleted() ); @@ -309,7 +326,7 @@ ImportSummary summary = dataValueSetService.saveDataValueSetCsv( in, null, null ); - assertEquals( 1, summary.getConflicts().size() ); // Header row + assertEquals( summary.getConflicts().toString(), 1, summary.getConflicts().size() ); // Header row assertEquals( 12, summary.getImportCount().getImported() ); assertEquals( 0, summary.getImportCount().getUpdated() ); assertEquals( 0, summary.getImportCount().getDeleted() ); @@ -330,7 +347,7 @@ ImportSummary summary = dataValueSetService.saveDataValueSet( in, options ); assertEquals( ImportStatus.SUCCESS, summary.getStatus() ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); Collection dataValues = mockDataValueBatchHandler.getInserts(); @@ -348,7 +365,7 @@ ImportSummary summary = dataValueSetService.saveDataValueSet( in, options ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); assertEquals( 0, summary.getImportCount().getImported() ); assertEquals( 0, summary.getImportCount().getUpdated() ); assertEquals( 0, summary.getImportCount().getDeleted() ); @@ -367,7 +384,7 @@ { ImportSummary summary = dataValueSetService.saveDataValueSet( new ClassPathResource( "datavalueset/dataValueSetC.xml" ).getInputStream() ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); assertEquals( 3, summary.getImportCount().getImported() ); assertEquals( 0, summary.getImportCount().getUpdated() ); assertEquals( 0, summary.getImportCount().getDeleted() ); @@ -389,7 +406,7 @@ ImportSummary summary = dataValueSetService.saveDataValueSet( in ); assertEquals( ImportStatus.SUCCESS, summary.getStatus() ); - assertEquals( 0, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 0, summary.getConflicts().size() ); Collection dataValues = mockDataValueBatchHandler.getInserts(); @@ -401,6 +418,24 @@ } @Test + public void testImportDataValuesWithOrgUnitOutsideHierarchy() + throws Exception + { + in = new ClassPathResource( "datavalueset/dataValueSetE.xml" ).getInputStream(); + + ImportSummary summary = dataValueSetService.saveDataValueSet( in ); + + assertEquals( ImportStatus.SUCCESS, summary.getStatus() ); + assertEquals( summary.getConflicts().toString(), 2, summary.getConflicts().size() ); + + Collection dataValues = mockDataValueBatchHandler.getInserts(); + + assertNotNull( dataValues ); + assertEquals( 1, dataValues.size() ); + assertTrue( dataValues.contains( new DataValue( deA, peA, ouA, ocDef, ocA ) ) ); + } + + @Test public void testImportDataValuesWithInvalidAttributeOptionCombo() throws Exception { @@ -425,7 +460,7 @@ ImportSummary summary = dataValueSetService.saveDataValueSet( in ); - assertEquals( 2, summary.getConflicts().size() ); + assertEquals( summary.getConflicts().toString(), 2, summary.getConflicts().size() ); assertEquals( 1, summary.getImportCount().getImported() ); assertEquals( 0, summary.getImportCount().getUpdated() ); assertEquals( 0, summary.getImportCount().getDeleted() ); === modified file 'dhis-2/dhis-services/dhis-service-dxf2/src/test/resources/datavalueset/dataValueSetE.xml' --- dhis-2/dhis-services/dhis-service-dxf2/src/test/resources/datavalueset/dataValueSetE.xml 2014-11-19 19:11:32 +0000 +++ dhis-2/dhis-services/dhis-service-dxf2/src/test/resources/datavalueset/dataValueSetE.xml 2015-06-01 19:49:10 +0000 @@ -1,6 +1,5 @@ - - - + + \ No newline at end of file === modified file 'dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/mock/MockCurrentUserService.java' --- dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/mock/MockCurrentUserService.java 2015-04-15 11:58:48 +0000 +++ dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/mock/MockCurrentUserService.java 2015-06-01 19:49:10 +0000 @@ -29,6 +29,7 @@ */ import java.util.Arrays; +import java.util.HashSet; import java.util.Set; import org.hisp.dhis.organisationunit.OrganisationUnit; @@ -89,6 +90,12 @@ } @Override + public Set getCurrentUserOrganisationUnits() + { + return currentUser != null ? currentUser.getOrganisationUnits() : new HashSet(); + } + + @Override public boolean currentUserIsSuper() { return superUserFlag;