dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #33749
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 17290: More approvals improvements
------------------------------------------------------------
revno: 17290
committer: jimgrace@xxxxxxxxx
branch nick: dhis2
timestamp: Sun 2014-10-26 03:59:12 -0400
message:
More approvals improvements
added:
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DataApprovalPermissionsEvaluator.java
modified:
dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataapproval/DataApproval.java
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/main/resources/META-INF/dhis/beans.xml
dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.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/dataapproval/DataApproval.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataapproval/DataApproval.java 2014-10-21 19:45:22 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/dataapproval/DataApproval.java 2014-10-26 07:59:12 +0000
@@ -55,6 +55,7 @@
public static final String AUTH_APPROVE = "F_APPROVE_DATA";
public static final String AUTH_APPROVE_LOWER_LEVELS = "F_APPROVE_DATA_LOWER_LEVELS";
public static final String AUTH_ACCEPT_LOWER_LEVELS = "F_ACCEPT_DATA_LOWER_LEVELS";
+ public static final String AUTH_VIEW_UNAPPROVED_DATA = "F_VIEW_UNAPPROVED_DATA";
private static final long serialVersionUID = -4034531921928532366L;
=== added file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DataApprovalPermissionsEvaluator.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DataApprovalPermissionsEvaluator.java 1970-01-01 00:00:00 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DataApprovalPermissionsEvaluator.java 2014-10-26 07:59:12 +0000
@@ -0,0 +1,153 @@
+package org.hisp.dhis.dataapproval;
+
+import org.hisp.dhis.setting.SystemSettingManager;
+import org.hisp.dhis.user.CurrentUserService;
+import org.hisp.dhis.user.User;
+
+import static org.hisp.dhis.setting.SystemSettingManager.KEY_ACCEPTANCE_REQUIRED_FOR_APPROVAL;
+import static org.hisp.dhis.setting.SystemSettingManager.KEY_HIDE_UNAPPROVED_DATA_IN_ANALYTICS;
+
+/**
+ * This package private class holds the context for deciding on data approval permissions.
+ * The context contains both system settings and some qualities of the user.
+ * <p>
+ * This class is especially efficient if the settings are set once and
+ * then used several times to generate ApprovalPermissions for different
+ * DataApproval objects.
+ *
+ * @author Jim Grace
+ */
+class DataApprovalPermissionsEvaluator
+{
+ DataApprovalLevelService dataApprovalLevelService;
+
+ User user;
+
+ private boolean acceptanceRequiredForApproval;
+ private boolean hideUnapprovedData;
+
+ private boolean authorizedToApprove;
+ private boolean authorizedToApproveAtLowerLevels;
+ private boolean authorizedToAcceptAtLowerLevels;
+ private boolean authorizedToViewUnapprovedData;
+
+ int maxApprovalLevel;
+
+ private DataApprovalPermissionsEvaluator()
+ {
+ }
+
+ /**
+ * Allocates and populates the context for determining user permissions
+ * on one or more DataApproval objects.
+ *
+ * @param currentUserService Current user service
+ * @param systemSettingManager System setting manager
+ * @param dataApprovalLevelService Data approval level service
+ * @return context for determining user permissions
+ */
+ static DataApprovalPermissionsEvaluator makePermissionsEvaluator( CurrentUserService currentUserService,
+ SystemSettingManager systemSettingManager, DataApprovalLevelService dataApprovalLevelService )
+ {
+ DataApprovalPermissionsEvaluator ev = new DataApprovalPermissionsEvaluator();
+
+ ev.dataApprovalLevelService = dataApprovalLevelService;
+
+ ev.user = currentUserService.getCurrentUser();
+
+ ev.acceptanceRequiredForApproval = (Boolean) systemSettingManager.getSystemSetting( KEY_ACCEPTANCE_REQUIRED_FOR_APPROVAL, false );
+ ev.hideUnapprovedData = (Boolean) systemSettingManager.getSystemSetting( KEY_HIDE_UNAPPROVED_DATA_IN_ANALYTICS, false );
+
+ ev.authorizedToApprove = ev.user.getUserCredentials().isAuthorized( DataApproval.AUTH_APPROVE );
+ ev.authorizedToApproveAtLowerLevels = ev.user.getUserCredentials().isAuthorized( DataApproval.AUTH_APPROVE_LOWER_LEVELS );
+ ev.authorizedToAcceptAtLowerLevels = ev.user.getUserCredentials().isAuthorized( DataApproval.AUTH_ACCEPT_LOWER_LEVELS );
+ ev.authorizedToViewUnapprovedData = ev.user.getUserCredentials().isAuthorized( DataApproval.AUTH_VIEW_UNAPPROVED_DATA );
+
+ ev.maxApprovalLevel = dataApprovalLevelService.getAllDataApprovalLevels().size();
+
+ tracePrint( "makePermissionsEvaluator acceptanceRequiredForApproval " + ev.acceptanceRequiredForApproval
+ + " hideUnapprovedData " + ev.hideUnapprovedData + " authorizedToApprove " + ev.authorizedToApprove
+ + " authorizedToAcceptAtLowerLevels " + ev.authorizedToAcceptAtLowerLevels
+ + " authorizedToViewUnapprovedData " + ev.authorizedToViewUnapprovedData + " maxApprovalLevel " + ev.maxApprovalLevel );
+
+ return ev;
+ }
+
+ /**
+ * Allocates and fills a data approval permissions object according to
+ * the context of system settings and user information.
+ * <p>
+ * If there is a data permissions state, also takes this into account.
+ *
+ * @param da the data approval object to evaluate
+ * @param status the data approval status (if any)
+ * @return the data approval permissions for the object
+ */
+ DataApprovalPermissions getPermissions( DataApproval da, DataApprovalStatus status )
+ {
+ DataApprovalPermissions permissions = new DataApprovalPermissions();
+
+ if ( da == null || da.getOrganisationUnit() == null )
+ {
+ return permissions; // No approval object or no org unit -> no permissions.
+ }
+
+ DataApprovalLevel userApprovalLevel = dataApprovalLevelService.getUserApprovalLevel( user, da.getOrganisationUnit(), false );
+
+ if ( userApprovalLevel == null )
+ {
+ return permissions; // Can't find user approval level, so no permissions are true.
+ }
+
+ boolean isApproved = ( da.getDataApprovalLevel() != null );
+ int userLevel = userApprovalLevel.getLevel();
+ int dataLevel = ( isApproved ? da.getDataApprovalLevel().getLevel() : maxApprovalLevel );
+ boolean isApprovable = true; // Unless the state tells us otherwise
+ boolean isAccepted = da.isAccepted();
+
+ if ( status != null && status.getState() != null )
+ {
+ DataApprovalState state = status.getState();
+
+ isApproved = state.isApproved() && state.isUnapprovable(); // Maybe approved, but not here.
+ isApprovable = state.isApprovable();
+ isAccepted = state.isAccepted();
+ }
+
+ boolean mayApproveOrUnapprove = ( authorizedToApprove && userLevel == dataLevel && !da.isAccepted() ) ||
+ ( authorizedToApproveAtLowerLevels && userLevel < dataLevel );
+
+ boolean mayApproveFromLowerLevel = ( userLevel + 1 == dataLevel && isApproved ) && acceptanceRequiredForApproval;
+
+ boolean mayApprove = isApprovable && ( !isApproved || userLevel < dataLevel )
+ && ( mayApproveOrUnapprove || mayApproveFromLowerLevel );
+
+ boolean mayAcceptOrUnaccept = authorizedToAcceptAtLowerLevels && isApproved &&
+ ( userLevel == dataLevel - 1 || ( userLevel < dataLevel && authorizedToApproveAtLowerLevels ) );
+
+ boolean mayUnapprove = isApproved && ( ( mayApproveOrUnapprove && !da.isAccepted() ) || mayAcceptOrUnaccept );
+
+ boolean mayReadData = authorizedToViewUnapprovedData || !hideUnapprovedData || mayApprove
+ || userLevel >= dataLevel;
+
+ tracePrint( "getPermissions orgUnit " + ( da.getOrganisationUnit() == null ? "(null)" : da.getOrganisationUnit().getName() )
+ + " combo " + da.getAttributeOptionCombo().getName()
+ + " state " + ( status == null || status.getState() == null ? "(null)" : status.getState().name() )
+ + " isApproved " + isApproved + " isAccepted " + isAccepted + " userLevel " + userLevel + " dataLevel " + dataLevel
+ + " mayApproveOrUnapprove " + mayApproveOrUnapprove + " mayApprove " + mayApprove + " mayUnapprove " + mayUnapprove
+ + " mayAcceptOrUnaccept " + mayAcceptOrUnaccept + " mayReadData " + mayReadData );
+
+ permissions.setMayApprove( mayApprove );
+ permissions.setMayUnapprove( mayUnapprove );
+ permissions.setMayAccept( mayAcceptOrUnaccept && !isAccepted );
+ permissions.setMayUnaccept( mayAcceptOrUnaccept && isAccepted );
+ permissions.setMayReadData( mayReadData );
+
+ return permissions;
+ }
+
+ private static void tracePrint( String s )
+ {
+// System.out.println( s );
+ }
+}
=== 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 20:49:53 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/DefaultDataApprovalService.java 2014-10-26 07:59:12 +0000
@@ -61,8 +61,8 @@
import org.hisp.dhis.period.PeriodService;
import org.hisp.dhis.period.PeriodType;
import org.hisp.dhis.security.SecurityService;
+import org.hisp.dhis.setting.SystemSettingManager;
import org.hisp.dhis.user.CurrentUserService;
-import org.hisp.dhis.user.User;
import org.springframework.transaction.annotation.Transactional;
/**
@@ -127,6 +127,13 @@
this.securityService = securityService;
}
+ private SystemSettingManager systemSettingManager;
+
+ public void setSystemSettingManager( SystemSettingManager systemSettingManager )
+ {
+ this.systemSettingManager = systemSettingManager;
+ }
+
// -------------------------------------------------------------------------
// DataApproval
// -------------------------------------------------------------------------
@@ -141,6 +148,8 @@
tracePrint( "checkedList ( " + checkedList.size() + " items )" );
+ DataApprovalPermissionsEvaluator permissionsEvaluator = makePermissionsEvaluator();
+
for ( Iterator<DataApproval> it = checkedList.iterator(); it.hasNext(); )
{
DataApproval da = it.next();
@@ -158,9 +167,11 @@
}
else if ( !status.getState().isApprovable() )
{
+ tracePrint("approveData: data is not approvable, state " + status.getState().name() );
+
throw new DataMayNotBeApprovedException();
}
- else if ( !mayApprove( da, status ) )
+ else if ( !permissionsEvaluator.getPermissions( da, status ).isMayApprove() )
{
throw new UserMayNotApproveDataException();
}
@@ -188,6 +199,8 @@
List<DataApproval> checkedList = checkApprovalsList( dataApprovalList, null, false );
List<DataApproval> storedDataApprovals = new ArrayList<>();
+ DataApprovalPermissionsEvaluator permissionsEvaluator = makePermissionsEvaluator();
+
for ( DataApproval da : checkedList )
{
DataApprovalStatus status = getStatus( da );
@@ -196,10 +209,14 @@
{
if ( !status.getState().isUnapprovable() )
{
+ tracePrint( "unapproveData: data may not be unapproved." );
+
throw new DataMayNotBeUnapprovedException();
}
- else if ( !mayUnapprove( da, status ) )
+ else if ( !permissionsEvaluator.getPermissions( da, status ).isMayUnapprove() )
{
+ tracePrint( "unapproveData: user may not unapprove the data." );
+
throw new UserMayNotUnapproveDataException();
}
@@ -231,6 +248,8 @@
List<DataApproval> checkedList = checkApprovalsList( dataApprovalList, null, false );
List<DataApproval> storedDataApprovals = new ArrayList<>();
+ DataApprovalPermissionsEvaluator permissionsEvaluator = makePermissionsEvaluator();
+
for ( DataApproval da : checkedList )
{
DataApprovalStatus status = getStatus( da );
@@ -239,14 +258,16 @@
{
if ( !status.getState().isAcceptable() )
{
- tracePrint("acceptData() state " + status.getState().name()
+ tracePrint("acceptData: state " + status.getState().name()
+ " accepted " + status.getState().isAccepted()
+ " acceptable " + status.getState().isAcceptable() );
throw new DataMayNotBeAcceptedException();
}
- else if ( !mayAcceptOrUnaccept( da, status ) )
+ else if ( !permissionsEvaluator.getPermissions( da, status ).isMayAccept() )
{
+ tracePrint( "acceptData: user may not accept the data." );
+
throw new UserMayNotAcceptDataException();
}
@@ -282,6 +303,8 @@
List<DataApproval> checkedList = checkApprovalsList( dataApprovalList, null, false );
List<DataApproval> storedDataApprovals = new ArrayList<>();
+ DataApprovalPermissionsEvaluator permissionsEvaluator = makePermissionsEvaluator();
+
for ( DataApproval da : checkedList )
{
DataApprovalStatus status = getStatus( da );
@@ -290,10 +313,16 @@
{
if ( !status.getState().isUnacceptable() )
{
+ tracePrint("acceptData: state " + status.getState().name()
+ + " accepted " + status.getState().isAccepted()
+ + " unacceptable " + status.getState().isUnacceptable() );
+
throw new DataMayNotBeUnacceptedException();
}
- else if ( !mayAcceptOrUnaccept( da, status ) )
+ else if ( !permissionsEvaluator.getPermissions( da, status ).isMayUnaccept() )
{
+ tracePrint( "unacceptData: user may not unaccept the data." );
+
throw new UserMayNotUnacceptDataException();
}
@@ -373,19 +402,20 @@
{
DataApprovalStatus status = getDataApprovalStatus( dataSet, period, organisationUnit, attributeOptionCombo );
- return getPermissions( status.getDataApprovalLevel(), status, status.getDataApproval() );
+ DataApprovalPermissionsEvaluator permissionsEvaluator = makePermissionsEvaluator();
+
+ DataApproval da = ( status.getDataApproval() != null ? status.getDataApproval() :
+ new DataApproval( null, dataSet, period, organisationUnit, attributeOptionCombo, false, null, null ) );
+
+ status.setPermissions( permissionsEvaluator.getPermissions( da, status ) );
+
+ return status;
}
@Override
public List<DataApprovalStatus> getUserDataApprovalsAndPermissions( Set<DataSet> dataSets, Set<Period> periods )
{
- User user = currentUserService.getCurrentUser();
-
- boolean authorizedToApprove = user.getUserCredentials().isAuthorized( DataApproval.AUTH_APPROVE );
- boolean authorizedToApproveAtLowerLevels = user.getUserCredentials().isAuthorized( DataApproval.AUTH_APPROVE_LOWER_LEVELS );
- boolean authorizedToAcceptAtLowerLevels = user.getUserCredentials().isAuthorized( DataApproval.AUTH_ACCEPT_LOWER_LEVELS );
-
- int maxApprovalLevel = dataApprovalLevelService.getAllDataApprovalLevels().size();
+ DataApprovalPermissionsEvaluator permissionsEvaluator = makePermissionsEvaluator();
List<DataApprovalStatus> statusList = new ArrayList<>();
@@ -393,35 +423,9 @@
for ( DataApproval da : approvals )
{
- DataApprovalPermissions permissions = new DataApprovalPermissions();
-
- if ( da.getOrganisationUnit() != null ) //TODO: Shouldn't be null -- fix the category option mappings to org units in the database.
+ if ( da.getOrganisationUnit() != null ) // Each CO in the database needs an org unit.
{
- DataApprovalLevel userApprovalLevel = dataApprovalLevelService.getUserApprovalLevel( user, da.getOrganisationUnit(), false );
-
- if ( userApprovalLevel != null )
- {
- boolean isApproved = ( da.getDataApprovalLevel() != null );
- int userLevel = userApprovalLevel.getLevel();
- int dataLevel = isApproved ? da.getDataApprovalLevel().getLevel() : maxApprovalLevel;
-
- boolean mayApprove = ( authorizedToApprove && userLevel == dataLevel && !da.isAccepted() ) ||
- authorizedToApproveAtLowerLevels && userLevel < dataLevel;
-
- boolean mayAcceptOrUnaccept = authorizedToAcceptAtLowerLevels && dataLevel <= maxApprovalLevel &&
- ( userLevel == dataLevel + 1 || ( userLevel < dataLevel && authorizedToApproveAtLowerLevels ) );
-
- boolean mayUnapprove = isApproved && mayApprove && ( !da.isAccepted() || mayAcceptOrUnaccept );
-
- permissions.setMayApprove( mayApprove );
- permissions.setMayUnapprove( mayUnapprove );
- permissions.setMayAccept( mayAcceptOrUnaccept );
- permissions.setMayUnaccept( mayAcceptOrUnaccept );
- }
-
- boolean mayReadData = true; //TODO: Fix.
-
- permissions.setMayReadData( mayReadData );
+ DataApprovalPermissions permissions = permissionsEvaluator.getPermissions( da, null );
statusList.add( new DataApprovalStatus( null, da, da.getDataApprovalLevel(), permissions ) );
}
@@ -439,6 +443,12 @@
// System.out.println( s );
}
+ private DataApprovalPermissionsEvaluator makePermissionsEvaluator()
+ {
+ return DataApprovalPermissionsEvaluator.makePermissionsEvaluator(
+ currentUserService, systemSettingManager, dataApprovalLevelService );
+ }
+
private DataApproval defensiveCopy( DataApproval da )
{
return da == null ? null : new DataApproval( da );
@@ -680,61 +690,6 @@
return expandedApprovals;
}
- private boolean mayApprove( DataApproval dataApproval, DataApprovalStatus dataApprovalStatus )
- {
- DataApprovalLevel userDal = dataApprovalLevelService.getUserApprovalLevel( dataApproval.getOrganisationUnit(), false );
-
- if ( userDal != null )
- {
- User user = currentUserService.getCurrentUser();
-
- boolean mayApprove = user.getUserCredentials().isAuthorized( DataApproval.AUTH_APPROVE );
- boolean mayApproveAtLowerLevels = user.getUserCredentials().isAuthorized( DataApproval.AUTH_APPROVE_LOWER_LEVELS );
-
- if ( ( mayApprove && userDal.getLevel() == dataApprovalStatus.getDataApprovalLevel().getLevel() )
- || ( mayApproveAtLowerLevels && userDal.getLevel() < dataApprovalStatus.getDataApprovalLevel().getLevel() ) )
- {
- tracePrint("mayApprove TRUE user level " + userDal.getLevel() + ", mayApprove " + mayApprove + ", mayApproveAtLL "
- + mayApproveAtLowerLevels + ", data level " + dataApprovalStatus.getDataApprovalLevel().getLevel() );
- return true;
- }
-
- tracePrint("mayApprove FALSE user level " + userDal.getLevel() + ", mayApprove " + mayApprove + ", mayApproveAtLL "
- + mayApproveAtLowerLevels + ", data level " + dataApprovalStatus.getDataApprovalLevel().getLevel() );
- }
- else
- {
- tracePrint("mayApprove FALSE, no user level" );
- }
-
- return false;
- }
-
- private boolean mayUnapprove( DataApproval dataApproval, DataApprovalStatus dataApprovalStatus )
- {
- return mayApprove( dataApproval, dataApprovalStatus ) || mayAcceptOrUnaccept( dataApproval, dataApprovalStatus );
- }
-
- private boolean mayAcceptOrUnaccept( DataApproval dataApproval, DataApprovalStatus dataApprovalStatus )
- {
- DataApprovalLevel userDal = dataApprovalLevelService.getUserApprovalLevel( dataApproval.getOrganisationUnit(), false );
-
- if ( userDal != null )
- {
- User user = currentUserService.getCurrentUser();
-
- boolean mayAcceptAtLowerLevels = user.getUserCredentials().isAuthorized( DataApproval.AUTH_ACCEPT_LOWER_LEVELS )
- || user.getUserCredentials().isAuthorized( DataApproval.AUTH_APPROVE_LOWER_LEVELS );
-
- if ( mayAcceptAtLowerLevels && userDal.getLevel() < dataApprovalStatus.getDataApprovalLevel().getLevel() )
- {
- return true;
- }
- }
-
- return false;
- }
-
private DataApprovalStatus getStatus( DataApproval dataApproval )
{
return doGetDataApprovalStatus( org.hisp.dhis.system.util.CollectionUtils.asList( dataApproval ), dataApproval );
@@ -747,67 +702,4 @@
return dataApprovalSelection.getDataApprovalStatus();
}
-
- /**
- * Return true if there are no category option groups, or if there is
- * one and the user can read it.
- *
- * @param categoryOptionGroups option groups (if any) for data selection
- * @return true if at most 1 option group and user can read, else false
- */
- boolean canReadOneCategoryOptionGroup( Collection<CategoryOptionGroup> categoryOptionGroups )
- {
- if ( categoryOptionGroups == null || categoryOptionGroups.size() == 0 )
- {
- return true;
- }
-
- if ( categoryOptionGroups.size() != 1 )
- {
- return false;
- }
-
- return (securityService.canRead( (CategoryOptionGroup) categoryOptionGroups.toArray()[0] ));
- }
-
- /**
- * Find permissions, and add to returned status.
- *
- * @param dal Data Approval Level we were looking for
- * @param status Approval Status that we found
- * @param da Original Data Approval describing what we were looking for
- * @return Permissions along with status
- */
- private DataApprovalStatus getPermissions( DataApprovalLevel dal, DataApprovalStatus status, DataApproval da )
- {
- DataApprovalPermissions permissions = new DataApprovalPermissions();
-
- tracePrint( "getPermissions - dal " + ( dal == null ? "(null)" : dal.getName() )
- + " dataApproval null? " + ( status.getDataApproval() == null ) );
-
- if ( da != null && dal != null && securityService.canRead( dal ) && status.getDataApproval() != null
- && ( dal.getCategoryOptionGroupSet() == null || securityService.canRead( dal.getCategoryOptionGroupSet() ) ) )
- {
- DataApprovalState state = status.getState();
-
- tracePrint( "getPermissions - state is " + state.name() );
-
- permissions.setMayApprove( state.isApprovable() && mayApprove( da, status ) );
- permissions.setMayUnapprove( state.isUnapprovable() && mayUnapprove( status.getDataApproval(), status ) );
- permissions.setMayAccept( state.isAcceptable() && mayAcceptOrUnaccept( status.getDataApproval(), status ) );
- permissions.setMayUnaccept( state.isUnacceptable() && mayAcceptOrUnaccept( status.getDataApproval(), status ) );
-
- log.debug( "Found permissions for " + da.getOrganisationUnit().getName()
- + " " + status.getState().name()
- + " may approve = " + permissions.isMayApprove()
- + " may unapprove = " + permissions.isMayUnapprove()
- + " may accept = " + permissions.isMayAccept()
- + " may unaccept = " + permissions.isMayUnaccept() );
- }
-
- status.setDataApproval( defensiveCopy( status.getDataApproval() ) );
- status.setPermissions( permissions );
-
- return status;
- }
}
=== 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 2014-10-25 07:40:39 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/dataapproval/hibernate/HibernateDataApprovalStore.java 2014-10-26 07:59:12 +0000
@@ -36,6 +36,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
+import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hibernate.Criteria;
@@ -55,6 +56,8 @@
import org.hisp.dhis.system.util.DateUtils;
import org.hisp.dhis.system.util.TextUtils;
import org.hisp.dhis.user.CurrentUserService;
+import org.hisp.dhis.user.User;
+import org.hisp.dhis.user.UserService;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.support.rowset.SqlRowSet;
@@ -107,6 +110,13 @@
this.currentUserService = currentUserService;
}
+ private UserService userService;
+
+ public void setUserService( UserService userService )
+ {
+ this.userService = userService;
+ }
+
private OrganisationUnitService organisationUnitService;
public void setOrganisationUnitService( OrganisationUnitService organisationUnitService )
@@ -175,6 +185,11 @@
@Override
public Set<DataApproval> getUserDataApprovals( Set<DataSet> dataSets, Set<Period> periods)
{
+ User user = currentUserService.getCurrentUser();
+
+ boolean canSeeDefaultOptionCombo = CollectionUtils.isEmpty( userService.getCoDimensionConstraints( user.getUserCredentials() ) )
+ && CollectionUtils.isEmpty( userService.getCogDimensionConstraints( user.getUserCredentials() ) );
+
Date minDate = null;
Date maxDate = null;
@@ -209,7 +224,7 @@
String limitCategoryOptionByOrgUnit = "";
String limitApprovalByOrgUnit = "";
- for ( OrganisationUnit orgUnit : currentUserService.getCurrentUser().getOrganisationUnits() )
+ for ( OrganisationUnit orgUnit : user.getOrganisationUnits() )
{
if ( orgUnit.getParent() == null ) // User has root org unit access
{
@@ -233,7 +248,7 @@
if ( !currentUserService.currentUserIsSuper() )
{
- limitBySharing = "and (ugm.userid = " + currentUserService.getCurrentUser().getId() + " or left(co.publicaccess,1) = 'r') ";
+ limitBySharing = "and (ugm.userid = " + user.getId() + " or left(co.publicaccess,1) = 'r') ";
}
String sql = "select ccoc.categoryoptioncomboid, da.periodid, dal.level, coo.organisationunitid, da.accepted " +
@@ -319,13 +334,16 @@
//TODO: currently special cased for PEFPAR's requirements. Can we make it more generic?
if ( ( level == null || level != 1 ) && optionCombo.equals( defaultOptionCombo ) )
{
- for ( OrganisationUnit unit : getUserOrgsAtLevel( 3 ) )
+ if ( canSeeDefaultOptionCombo )
{
- DataApproval da = new DataApproval( dataApprovalLevel, null, period, unit, optionCombo, accepted, null, null );
-
- userDataApprovals.add( da );
+ for ( OrganisationUnit unit : getUserOrgsAtLevel( 3 ) )
+ {
+ DataApproval da = new DataApproval( dataApprovalLevel, null, period, unit, optionCombo, accepted, null, null );
+
+ userDataApprovals.add( da );
+ }
}
-
+
continue;
}
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml'
--- dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml 2014-10-25 16:44:43 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml 2014-10-26 07:59:12 +0000
@@ -75,6 +75,7 @@
<property name="sessionFactory" ref="sessionFactory" />
<property name="periodService" ref="org.hisp.dhis.period.PeriodService" />
<property name="currentUserService" ref="org.hisp.dhis.user.CurrentUserService" />
+ <property name="userService" ref="org.hisp.dhis.user.UserService" />
<property name="organisationUnitService" ref="org.hisp.dhis.organisationunit.OrganisationUnitService" />
<property name="categoryService" ref="org.hisp.dhis.dataelement.DataElementCategoryService" />
<property name="dataApprovalLevelService" ref="org.hisp.dhis.dataapproval.DataApprovalLevelService" />
@@ -434,6 +435,7 @@
<property name="categoryService" ref="org.hisp.dhis.dataelement.DataElementCategoryService" />
<property name="periodService" ref="org.hisp.dhis.period.PeriodService" />
<property name="securityService" ref="org.hisp.dhis.security.SecurityService" />
+ <property name="systemSettingManager" ref="org.hisp.dhis.setting.SystemSettingManager" />
</bean>
<bean id="org.hisp.dhis.dataapproval.DataApprovalLevelService" class="org.hisp.dhis.dataapproval.DefaultDataApprovalLevelService">
=== 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 2014-10-23 21:47:57 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataapproval/DataApprovalServiceTest.java 2014-10-26 07:59:12 +0000
@@ -1217,7 +1217,7 @@
}
@Test
- public void testMayUnapproveWithUnacceptAuthority() throws Exception
+ public void testMayUnapproveWithAcceptAuthority() throws Exception
{
dataApprovalLevelService.addDataApprovalLevel( level1 );
dataApprovalLevelService.addDataApprovalLevel( level2 );
@@ -1253,7 +1253,7 @@
assertEquals( false, dataApprovalService.getDataApprovalStatusAndPermissions( dataSetA, periodA, organisationUnitA, defaultCombo ).getPermissions().isMayUnapprove());
assertEquals( false, dataApprovalService.getDataApprovalStatusAndPermissions( dataSetA, periodA, organisationUnitB, defaultCombo ).getPermissions().isMayUnapprove());
assertEquals( false, dataApprovalService.getDataApprovalStatusAndPermissions( dataSetA, periodA, organisationUnitC, defaultCombo ).getPermissions().isMayUnapprove());
- assertEquals( true, dataApprovalService.getDataApprovalStatusAndPermissions( dataSetA, periodA, organisationUnitD, defaultCombo ).getPermissions().isMayUnapprove());
+ assertEquals( false, dataApprovalService.getDataApprovalStatusAndPermissions( dataSetA, periodA, organisationUnitD, defaultCombo ).getPermissions().isMayUnapprove());
assertEquals( false, dataApprovalService.getDataApprovalStatusAndPermissions( dataSetA, periodA, organisationUnitE, defaultCombo ).getPermissions().isMayUnapprove());
assertEquals( false, dataApprovalService.getDataApprovalStatusAndPermissions( dataSetA, periodA, organisationUnitF, defaultCombo ).getPermissions().isMayUnapprove());
