=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataapproval/DataApprovalLevelService.java' --- dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataapproval/DataApprovalLevelService.java 2014-10-25 10:06:33 +0000 +++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataapproval/DataApprovalLevelService.java 2014-10-25 20:49:53 +0000 @@ -77,17 +77,13 @@ DataApprovalLevel getDataApprovalLevelByLevelNumber( int levelNumber ); /** - * Gets the highest approval level for a given organisation unit and - * (optionally) a set of attribute option groups. Returns the first - * approval level matching both the orgUnit's level and (optionally) - * having a category option group set containing one of the category - * option groups. + * Gets the highest approval at which the current user may approve the + * organisation unit. * * @param orgUnit organisation unit to look for. - * @param cogs attribute option groups (if any) to look for. * @return a data approval level, or null if not found. */ - DataApprovalLevel getHighestDataApprovalLevel( OrganisationUnit orgUnit, Set cogs ); + DataApprovalLevel getHighestDataApprovalLevel( OrganisationUnit orgUnit ); /** * Gets the lowest approval level for a given organisation unit and === 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-10-25 16:44:43 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalLevelService.java 2014-10-25 20:49:53 +0000 @@ -141,26 +141,8 @@ } @Override - public DataApprovalLevel getHighestDataApprovalLevel( OrganisationUnit orgUnit, Set cogs ) + public DataApprovalLevel getHighestDataApprovalLevel( OrganisationUnit orgUnit ) { - Set cogSets = null; - - tracePrint( "getHighestDataApprovalLevel - org unit: " + orgUnit.getName() ); - if ( cogs != null && !cogs.isEmpty() ) - { - cogSets = new HashSet<>(); - - for ( CategoryOptionGroup cog : cogs ) - { - tracePrint( "getHighestDataApprovalLevel - COG: " + cog.getName() ); - if ( cog.getGroupSet() != null ) - { - tracePrint( "getHighestDataApprovalLevel - COGS: " + cog.getGroupSet().getName() ); - cogSets.add( cog.getGroupSet() ); - } - } - } - int orgUnitLevel = organisationUnitService.getLevelOfOrganisationUnit( orgUnit ); DataApprovalLevel levelAbove = null; @@ -169,25 +151,19 @@ tracePrint( "getHighestDataApprovalLevel - data approval level count: " + getAllDataApprovalLevels().size() ); - for ( DataApprovalLevel level : getAllDataApprovalLevels() ) + for ( DataApprovalLevel level : getUserDataApprovalLevels() ) { tracePrint( "getHighestDataApprovalLevel - data approval level: " + level.getName() ); - if ( ( level.getCategoryOptionGroupSet() == null && cogSets == null ) - || ( level.getCategoryOptionGroupSet() != null - && cogSets != null - && cogSets.contains( level.getCategoryOptionGroupSet() ) ) ) - { - if ( level.getOrgUnitLevel() == orgUnitLevel ) - { - return level; // Exact match on org unit level. - } - else if ( level.getOrgUnitLevel() > levelAboveOrgUnitLevel ) - { - levelAbove = level; // Must be first matching approval level for this org unit level. + if ( level.getOrgUnitLevel() == orgUnitLevel ) + { + return level; // Exact match on org unit level. + } + else if ( level.getOrgUnitLevel() > levelAboveOrgUnitLevel ) + { + levelAbove = level; // Must be first matching approval level for this org unit level. - levelAboveOrgUnitLevel = level.getOrgUnitLevel(); - } + levelAboveOrgUnitLevel = level.getOrgUnitLevel(); } } === 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-10-25 10:28:03 +0000 +++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java 2014-10-25 20:49:53 +0000 @@ -139,6 +139,8 @@ List checkedList = checkApprovalsList( dataApprovalList, null, false ); + tracePrint( "checkedList ( " + checkedList.size() + " items )" ); + for ( Iterator it = checkedList.iterator(); it.hasNext(); ) { DataApproval da = it.next(); @@ -150,6 +152,8 @@ if ( status.getState().isApproved() && status.getDataApprovalLevel().getLevel() >= da.getDataApprovalLevel().getLevel() ) { + tracePrint( "approveData: data already approved." ); + it.remove(); // Already approved at this level, no action needed } else if ( !status.getState().isApprovable() ) @@ -544,6 +548,8 @@ { List daList = new ArrayList<>(); + tracePrint( "checkApprovalsList checking " + dataApprovalList.size() + " items." ); + for ( DataApproval dataApproval : dataApprovalList ) { DataApproval da = checkDataApproval( dataApproval, isGetStatus ); @@ -580,16 +586,18 @@ if ( !da.getDataSet().isApproveData() ) { + tracePrint("checkDataApproval - data set '" + da.getDataSet().getName() + "' is not marked for approval." ); + throw new DataSetNotMarkedForApprovalException(); } if ( da.getAttributeOptionCombo() == null ) { da.setAttributeOptionCombo( categoryService.getDefaultDataElementCategoryOptionCombo() ); - - tracePrint( "getDefaultDataElementCategoryOptionCombo() -> " + ( da.getAttributeOptionCombo() == null ? "(null)" : da.getAttributeOptionCombo().getName() ) ); } + tracePrint( "getDefaultDataElementCategoryOptionCombo() -> " + ( da.getAttributeOptionCombo() == null ? "(null)" : da.getAttributeOptionCombo().getName() ) ); + DataApprovalLevel dal = dataApprovalLevelService.getUserApprovalLevel( da.getOrganisationUnit(), includeDataViewOrgUnits ); int userLevel = ( dal == null ? 99999 : dal.getLevel() ); @@ -599,6 +607,8 @@ if ( userLevel > da.getDataApprovalLevel().getLevel() ) { + log.info( "User level " + userLevel + " cannot access approvalLevel " + da.getDataApprovalLevel().getLevel() ); + throw new UserCannotAccessApprovalLevelException(); } === modified file 'dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataApprovalController.java' --- dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataApprovalController.java 2014-10-25 11:43:34 +0000 +++ dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataApprovalController.java 2014-10-25 20:49:53 +0000 @@ -58,6 +58,7 @@ import org.hisp.dhis.dataapproval.DataApprovalStateResponse; import org.hisp.dhis.dataapproval.DataApprovalStateResponses; import org.hisp.dhis.dataapproval.DataApprovalStatus; +import org.hisp.dhis.dataapproval.exceptions.DataApprovalException; import org.hisp.dhis.dataelement.DataElementCategoryOptionCombo; import org.hisp.dhis.dataelement.DataElementCategoryService; import org.hisp.dhis.dataset.DataSet; @@ -330,7 +331,7 @@ return; } - DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit, null ); //TODO fix category stuff + DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit ); if ( dataApprovalLevel == null ) { @@ -354,8 +355,15 @@ { ContextUtils.conflictResponse( response, "Approval must have data sets, periods and category option combos" ); } - - dataApprovalService.approveData( getDataApprovalList( dataApproval ) ); + + try + { + dataApprovalService.approveData( getDataApprovalList( dataApproval ) ); + } + catch ( DataApprovalException ex ) + { + ContextUtils.conflictResponse( response, ex.getClass().getName() ); + } } @RequestMapping( value = APPROVALS_PATH + "/unapprovals", method = RequestMethod.POST ) @@ -366,8 +374,15 @@ { ContextUtils.conflictResponse( response, "Approval must have data sets, periods and category option combos" ); } - - dataApprovalService.unapproveData( getDataApprovalList( dataApproval ) ); + + try + { + dataApprovalService.unapproveData( getDataApprovalList( dataApproval ) ); + } + catch ( DataApprovalException ex ) + { + ContextUtils.conflictResponse( response, ex.getClass().getName() ); + } } @PreAuthorize( "hasRole('ALL') or hasRole('F_APPROVE_DATA') or hasRole('F_APPROVE_DATA_LOWER_LEVELS')" ) @@ -404,7 +419,7 @@ return; } - DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit, null ); //TODO fix category stuff + DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit ); if ( dataApprovalLevel == null ) { @@ -462,7 +477,7 @@ return; } - DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit, null ); //TODO fix category stuff + DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit ); if ( dataApprovalLevel == null ) { @@ -486,8 +501,15 @@ { ContextUtils.conflictResponse( response, "Acceptance must have data sets, periods and category option combos" ); } - - dataApprovalService.acceptData( getDataApprovalList( dataApproval ) ); + + try + { + dataApprovalService.acceptData( getDataApprovalList( dataApproval ) ); + } + catch ( DataApprovalException ex ) + { + ContextUtils.conflictResponse( response, ex.getClass().getName() ); + } } @RequestMapping( value = ACCEPTANCES_PATH + "/unacceptances", method = RequestMethod.POST ) @@ -498,7 +520,15 @@ { ContextUtils.conflictResponse( response, "Acceptance must have data sets, periods and category option combos" ); } - + + try + { + dataApprovalService.acceptData( getDataApprovalList( dataApproval ) ); + } + catch ( DataApprovalException ex ) + { + ContextUtils.conflictResponse( response, ex.getClass().getName() ); + } dataApprovalService.unacceptData( getDataApprovalList( dataApproval ) ); } @@ -536,7 +566,7 @@ return; } - DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit, null ); //TODO fix category stuff + DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit ); if ( dataApprovalLevel == null ) { @@ -593,7 +623,7 @@ return; } - DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit, null ); //TODO fix category stuff + DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit ); if ( dataApprovalLevel == null ) { @@ -647,7 +677,7 @@ return; } - DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit, null ); //TODO fix category stuff + DataApprovalLevel dataApprovalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( organisationUnit ); if ( dataApprovalLevel == null ) { @@ -688,16 +718,16 @@ User user = currentUserService.getCurrentUser(); OrganisationUnit unit = user.getOrganisationUnit(); //TODO - DataApprovalLevel approvalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( unit, null ); + DataApprovalLevel approvalLevel = dataApprovalLevelService.getHighestDataApprovalLevel( unit ); Date date = new Date(); List approvals = new ArrayList<>(); - + for ( DataSet dataSet : dataSets ) { Set dataSetOptionCombos = dataSet.hasCategoryCombo() ? dataSet.getCategoryCombo().getOptionCombos() : null; - + for ( Period period : periods ) { for ( DataElementCategoryOptionCombo optionCombo : optionCombos )