dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #37242
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 19063: Enforce match of approval org unit with approval level (e.g. must be global OU for level 1 approv...
------------------------------------------------------------
revno: 19063
committer: jimgrace@xxxxxxxxx
branch nick: dhis2
timestamp: Wed 2015-04-29 14:13:26 -0500
message:
Enforce match of approval org unit with approval level (e.g. must be global OU for level 1 approval in DATIM.)
modified:
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/hibernate/HibernateDataApprovalStore.java
dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceCategoryOptionGroupTest.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/DefaultDataApprovalService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java 2015-04-15 11:58:48 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java 2015-04-29 19:13:26 +0000
@@ -141,8 +141,8 @@
da.setDataApprovalLevel( status.getDataApproval().getDataApprovalLevel() );
}
}
-
- if ( status != null && status.getState().isApproved() &&
+
+ if ( status != null && status.getState().isApproved() && da.getDataApprovalLevel() != null &&
da.getDataApprovalLevel().getLevel() >= status.getDataApprovalLevel().getLevel() )
{
continue; // Already approved at or above this level
@@ -156,6 +156,15 @@
throw new DataMayNotBeApprovedException();
}
+ if ( organisationUnitService.getLevelOfOrganisationUnit( da.getOrganisationUnit() ) != da.getDataApprovalLevel().getOrgUnitLevel() )
+ {
+ log.warn( "approveData: org unit " + da.getOrganisationUnit().getUid() + " '" + da.getOrganisationUnit().getName() +
+ "' has wrong org unit level " + organisationUnitService.getLevelOfOrganisationUnit( da.getOrganisationUnit() ) +
+ " for approval level " + da.getDataApprovalLevel().getLevel());
+
+ throw new DataMayNotBeApprovedException();
+ }
+
checkedList.add ( da );
}
@@ -196,8 +205,8 @@
throw new DataMayNotBeUnapprovedException();
}
- if ( !status.getState().isApproved() ||
- da.getDataApprovalLevel().getLevel() < status.getDataApprovalLevel().getLevel() )
+ if ( !status.getState().isApproved() || ( da.getDataApprovalLevel() != null &&
+ da.getDataApprovalLevel().getLevel() < status.getDataApprovalLevel().getLevel() ) )
{
continue; // Already unapproved at or below this level
}
@@ -306,7 +315,7 @@
da.setDataApprovalLevel( status.getDataApproval().getDataApprovalLevel() );
}
- if ( status == null || ( !status.getState().isAccepted() && da.getDataApprovalLevel().getLevel() == status.getDataApprovalLevel().getLevel() ) ||
+ if ( status == null || ( !status.getState().isAccepted() && da.getDataApprovalLevel() != null && da.getDataApprovalLevel().getLevel() == status.getDataApprovalLevel().getLevel() ) ||
da.getDataApprovalLevel().getLevel() < status.getDataApprovalLevel().getLevel() )
{
continue; // Already unaccepted at or not approved up to this level
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/hibernate/HibernateDataApprovalStore.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/hibernate/HibernateDataApprovalStore.java 2015-03-31 00:55:04 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/hibernate/HibernateDataApprovalStore.java 2015-04-29 19:13:26 +0000
@@ -275,21 +275,43 @@
categoryComboIds = TextUtils.getCommaDelimitedString( catComboIds );
}
- int orgUnitLevel = 1;
+ List<DataApprovalLevel> approvalLevels = dataApprovalLevelService.getAllDataApprovalLevels();
+
+ if ( CollectionUtils.isEmpty( approvalLevels ) )
+ {
+ log.warn( " No approval levels configured." );
+
+ return new ArrayList<>(); // Unapprovable.
+ }
+
+ int orgUnitLevel;
String orgUnitJoinOn;
+ String highestApprovedOrgUnitCompare;
if ( orgUnit != null )
{
orgUnitLevel = organisationUnitService.getLevelOfOrganisationUnit( orgUnit );
orgUnitJoinOn = "o.organisationunitid = " + orgUnit.getId();
+ highestApprovedOrgUnitCompare = "da.organisationunitid = o.organisationunitid ";
}
else
{
- for ( DataApprovalLevel dal : dataApprovalLevelService.getAllDataApprovalLevels() )
+ highestApprovedOrgUnitCompare = "";
+ orgUnitLevel = 0;
+
+ for ( DataApprovalLevel dal : approvalLevels )
{
- orgUnitLevel = dal.getOrgUnitLevel(); // Get lowest (last level -> greatest number) level.
+ if ( dal.getOrgUnitLevel() != orgUnitLevel )
+ {
+ orgUnitLevel = dal.getOrgUnitLevel(); // Remember lowest (last level -> greatest number) level.
+
+ highestApprovedOrgUnitCompare += ( highestApprovedOrgUnitCompare.length() == 0 ? "(" : " or" )
+ + " da.organisationunitid = o.idlevel" + orgUnitLevel;
+ }
}
-
+
+ highestApprovedOrgUnitCompare += ") ";
+
orgUnitJoinOn = "o.level = " + orgUnitLevel;
}
@@ -314,17 +336,15 @@
int orgUnitLevelAbove = 0;
- List<DataApprovalLevel> approvalLevels = dataApprovalLevelService.getAllDataApprovalLevels();
-
- if ( CollectionUtils.isEmpty( approvalLevels ) )
- {
- log.warn( " No approval levels configured." );
-
- return new ArrayList<>(); // Unapprovable.
- }
+ int highestApprovalOrgUnitLevel = 99999999;
for ( DataApprovalLevel dal : approvalLevels )
{
+ if ( dal.getOrgUnitLevel() < highestApprovalOrgUnitLevel )
+ {
+ highestApprovalOrgUnitLevel = dal.getOrgUnitLevel();
+ }
+
if ( dal.getOrgUnitLevel() < orgUnitLevel )
{
orgUnitLevelAbove = dal.getOrgUnitLevel(); // Keep getting the lowest org unit level above ours.
@@ -337,6 +357,16 @@
if ( dal.getOrgUnitLevel() > orgUnitLevel ) // If there is a lower (higher number) approval orgUnit level.
{
+ String coOrgUnitConstraint = ""; // Constrain lower level orgUnit by categoryOption orgUnit(s) (if any).
+
+ if ( !isDefaultCombo )
+ {
+ for ( int i = 1; i <= dal.getOrgUnitLevel(); i++ )
+ {
+ coOrgUnitConstraint += ( i == 1 ? "" : "or " ) + "( o2.level = " + i + " and ous.idlevel" + i + " = c_o.organisationunitid ) ";
+ }
+ }
+
boolean acceptanceRequiredForApproval = (Boolean) systemSettingManager.getSystemSetting( KEY_ACCEPTANCE_REQUIRED_FOR_APPROVAL, false );
readyBelowSubquery = "not exists (select 1 from _orgunitstructure ous " +
@@ -348,14 +378,20 @@
( acceptanceRequiredForApproval ? "and da.accepted " : "" ) +
") " +
"and ous.idlevel" + orgUnitLevel + " = o.organisationunitid " +
- "and ous.level = " + dal.getOrgUnitLevel() + ") ";
+ "and ous.level = " + dal.getOrgUnitLevel() + " " +
+ ( isDefaultCombo ? "" :
+ "and ( not exists ( select 1 from categoryoption_organisationunits c_o where c_o.categoryoptionid = cocco.categoryoptionid ) " +
+ "or exists ( select 1 from categoryoption_organisationunits c_o " +
+ "join _orgunitstructure o2 on o2.organisationunitid = c_o.organisationunitid " +
+ "where c_o.categoryoptionid = cocco.categoryoptionid and (" + coOrgUnitConstraint + ") ) ) " ) +
+ ")";
break;
}
}
String approvedAboveSubquery = "false"; // Not approved above if this is the highest (lowest number) approval orgUnit level.
- if ( orgUnitLevelAbove > 0 )
+ if ( orgUnitLevelAbove > 0 && orgUnit != null )
{
approvedAboveSubquery = "exists(select 1 from dataapproval da " +
"join dataapprovallevel dal on dal.dataapprovallevelid = da.dataapprovallevelid " +
@@ -365,18 +401,12 @@
final String sql =
"select cocco.categoryoptioncomboid, o.organisationunitid, " +
- "(select min(coalesce(dal.level, 0)) from period p " +
- "left join dataapproval da on da.datasetid in (" + dataSetIds + ") and da.periodid = p.periodid " +
- "left join dataapprovallevel dal on dal.dataapprovallevelid = da.dataapprovallevelid " +
- "where p.periodid in (" + periodIds + ") " +
- "and da.attributeoptioncomboid = cocco.categoryoptioncomboid and da.organisationunitid = o.organisationunitid " +
- ") as highest_approved_level, " +
- "(select substring(min(concat(100000 + coalesce(dal.level, 0), coalesce(da.accepted, FALSE))) from 7) from period p " +
- "left join dataapproval da on da.datasetid in (" + dataSetIds + ") and da.periodid = p.periodid " +
- "left join dataapprovallevel dal on dal.dataapprovallevelid = da.dataapprovallevelid " +
- "where p.periodid in (" + periodIds + ") " +
- "and da.attributeoptioncomboid = cocco.categoryoptioncomboid and da.organisationunitid = o.organisationunitid " +
- ") as accepted_at_highest_level, " +
+ "(select min(coalesce(dal.level + case when da.accepted then .0 else .1 end, 0)) from period p " +
+ "left join dataapproval da on da.datasetid in (" + dataSetIds + ") and da.periodid = p.periodid " +
+ "left join dataapprovallevel dal on dal.dataapprovallevelid = da.dataapprovallevelid " +
+ "where p.periodid in (" + periodIds + ") " +
+ "and da.attributeoptioncomboid = cocco.categoryoptioncomboid and " + highestApprovedOrgUnitCompare +
+ ") as highest_approved, " +
readyBelowSubquery + " as ready_below, " +
approvedAboveSubquery + " as approved_above " +
"from categoryoptioncombos_categoryoptions cocco " +
@@ -410,14 +440,14 @@
{
final Integer aoc = rowSet.getInt( 1 );
final Integer ouId = rowSet.getInt( 2 );
- final Integer level = rowSet.getInt( 3 );
- final String acceptedString = rowSet.getString( 4 );
- final boolean readyBelow = rowSet.getBoolean( 5 );
- final boolean approvedAbove = rowSet.getBoolean( 6 );
-
- final boolean accepted = ( acceptedString == null ? false : acceptedString.substring( 0, 1 ).equalsIgnoreCase( "t" ) );
-
- DataApprovalLevel statusLevel = ( level == null || level == 0 ? null : levelMap.get( level ) ); // null if not approved
+ final Double highestApproved = rowSet.getDouble( 3 );
+ final boolean readyBelow = rowSet.getBoolean( 4 );
+ final boolean approvedAbove = rowSet.getBoolean( 5 );
+
+ final int level = highestApproved == null ? 0 : highestApproved.intValue();
+ final boolean accepted = ( highestApproved == level );
+
+ DataApprovalLevel statusLevel = ( level == 0 ? null : levelMap.get( level ) ); // null if not approved
DataApprovalLevel daLevel = ( statusLevel == null ? lowestApprovalLevelForOrgUnit : statusLevel );
DataElementCategoryOptionCombo optionCombo = ( aoc == null || aoc == 0 ? null : optionComboCache.get( aoc, new Callable<DataElementCategoryOptionCombo>()
@@ -458,9 +488,9 @@
statusList.add( new DataApprovalStatus( state, da, statusLevel, null ) );
log.debug( "Get approval result: level " + level + " dataApprovalLevel " + ( daLevel != null ? daLevel.getLevel() : "[none]" )
- + " approved " + ( statusLevel != null )
- + " readyBelow " + readyBelow + " approvedAbove " + approvedAbove
- + " accepted " + accepted + " state " + ( state != null ? state.name() : "[none]" ) + " " + da );
+ + " approved " + ( statusLevel != null )
+ + " readyBelow " + readyBelow + " approvedAbove " + approvedAbove
+ + " accepted " + accepted + " state " + ( state != null ? state.name() : "[none]" ) + " " + da );
}
}
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceCategoryOptionGroupTest.java'
--- dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceCategoryOptionGroupTest.java 2015-04-16 00:11:41 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceCategoryOptionGroupTest.java 2015-04-29 19:13:26 +0000
@@ -1670,29 +1670,31 @@
// Approve ChinaA1_1 at level 1
// ---------------------------------------------------------------------
- assertTrue( approve( superUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertTrue( unapprove( superUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertTrue( approve( globalConsultant, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertTrue( unapprove( globalConsultant, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( approve( globalReadEverything, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( approve( brazilInteragencyUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( chinaInteragencyUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( indiaInteragencyUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( approve( brazilAgencyAUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( chinaAgencyAUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( chinaAgencyBUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( indiaAgencyAUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( approve( brazilPartner1User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( chinaPartner1User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( chinaPartner2User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( approve( indiaPartner1User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertTrue( approve( globalUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
+ assertFalse( approve( superUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) ); // Wrong org unit.
+
+ assertTrue( approve( superUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertTrue( unapprove( superUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertTrue( approve( globalConsultant, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertTrue( unapprove( globalConsultant, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( approve( globalReadEverything, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( approve( brazilInteragencyUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( chinaInteragencyUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( indiaInteragencyUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( approve( brazilAgencyAUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( chinaAgencyAUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( chinaAgencyBUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( indiaAgencyAUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( approve( brazilPartner1User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( chinaPartner1User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( chinaPartner2User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( approve( indiaPartner1User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertTrue( approve( globalUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
// ---------------------------------------------------------------------
// ChinaA1_1 is approved at level 1
@@ -1782,29 +1784,29 @@
// Unapprove ChinaA1_1 at level 1
// ---------------------------------------------------------------------
- assertTrue( unapprove( superUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertTrue( approve( superUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertTrue( unapprove( globalConsultant, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertTrue( approve( globalConsultant, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( unapprove( globalReadEverything, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( unapprove( brazilInteragencyUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( chinaInteragencyUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( indiaInteragencyUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( unapprove( brazilAgencyAUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( chinaAgencyAUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( chinaAgencyBUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( indiaAgencyAUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertFalse( unapprove( brazilPartner1User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( chinaPartner1User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( chinaPartner2User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
- assertFalse( unapprove( indiaPartner1User, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
-
- assertTrue( unapprove( globalUser, globalLevel1, dataSetA, periodA, china, chinaA1_1Combo ) );
+ assertTrue( unapprove( superUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertTrue( approve( superUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertTrue( unapprove( globalConsultant, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertTrue( approve( globalConsultant, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( unapprove( globalReadEverything, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( unapprove( brazilInteragencyUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( chinaInteragencyUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( indiaInteragencyUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( unapprove( brazilAgencyAUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( chinaAgencyAUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( chinaAgencyBUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( indiaAgencyAUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertFalse( unapprove( brazilPartner1User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( chinaPartner1User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( chinaPartner2User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+ assertFalse( unapprove( indiaPartner1User, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
+
+ assertTrue( unapprove( globalUser, globalLevel1, dataSetA, periodA, global, chinaA1_1Combo ) );
assertArrayEquals( new String[] {
"ou=Brazil mechanism=BrazilA1 level=4 UNAPPROVED_READY approve=T unapprove=F accept=F unaccept=F read=T",
