Skip to content

Commit

Permalink
fix: cannot get TE if there are no tracker programs
Browse files Browse the repository at this point in the history
  • Loading branch information
teleivo committed Feb 14, 2025
1 parent e1d15ab commit 048e3a1
Show file tree
Hide file tree
Showing 10 changed files with 192 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ public class Page<T> {
private final Integer prevPage;
private final Integer nextPage;

public static <T> Page<T> empty() {
return new Page<>(List.of(), 0, 0, 0L, null, null);
}

/**
* Create a new page based on an existing one but with given {@code items}. Page related counts
* will not be changed so make sure the given {@code items} match the previous page size.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -375,10 +375,12 @@ private List<TrackedEntity> getTrackedEntities(
for (TrackedEntity trackedEntity : trackedEntities) {
if (operationParams.getTrackedEntityParams().isIncludeProgramOwners()) {
trackedEntity.setProgramOwners(
getTrackedEntityProgramOwners(trackedEntity, queryParams.getProgram()));
getTrackedEntityProgramOwners(
trackedEntity, queryParams.getEnrolledInTrackerProgram()));
}
trackedEntity.setTrackedEntityAttributeValues(
getTrackedEntityAttributeValues(trackedEntity, queryParams.getProgram()));
getTrackedEntityAttributeValues(
trackedEntity, queryParams.getEnrolledInTrackerProgram()));
}
trackedEntityAuditService.addTrackedEntityAudit(SEARCH, user.getUsername(), trackedEntities);
return trackedEntities;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ public HibernateTrackedEntityStore(
}

public List<TrackedEntityIdentifiers> getTrackedEntityIds(TrackedEntityQueryParams params) {
// A TE which is not enrolled can only be accessed by a user that is able to enroll it into a
// tracker program. Return an empty result if there are no tracker programs or the user does
// not have access to one.
if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) {
return List.of();
}

String sql = getQuery(params, null);
SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql);

Expand All @@ -163,6 +170,13 @@ public List<TrackedEntityIdentifiers> getTrackedEntityIds(TrackedEntityQueryPara

public Page<TrackedEntityIdentifiers> getTrackedEntityIds(
TrackedEntityQueryParams params, PageParams pageParams) {
// A TE which is not enrolled can only be accessed by a user that is able to enroll it into a
// tracker program. Return an empty result if there are no tracker programs or the user does
// not have access to one.
if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) {
return Page.empty();
}

String sql = getQuery(params, pageParams);
SqlRowSet rowSet = jdbcTemplate.queryForRowSet(sql);

Expand Down Expand Up @@ -206,12 +220,26 @@ private void checkMaxTrackedEntityCountReached(
}
}

public Long getTrackedEntityCount(TrackedEntityQueryParams params) {
private Long getTrackedEntityCount(TrackedEntityQueryParams params) {
// A TE which is not enrolled can only be accessed by a user that is able to enroll it into a
// tracker program. Return an empty result if there are no tracker programs or the user does
// not have access to one.
if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) {
return 0L;
}

String sql = getCountQuery(params);
return jdbcTemplate.queryForObject(sql, Long.class);
}

public int getTrackedEntityCountWithMaxTrackedEntityLimit(TrackedEntityQueryParams params) {
// A TE which is not enrolled can only be accessed by a user that is able to enroll it into a
// tracker program. Return an empty result if there are no tracker programs or the user does
// not have access to one.
if (!params.hasEnrolledInTrackerProgram() && params.getAccessibleTrackerPrograms().isEmpty()) {
return 0;
}

String sql = getCountQueryWithMaxTrackedEntityLimit(params);
return jdbcTemplate.queryForObject(sql, Integer.class);
}
Expand Down Expand Up @@ -306,8 +334,8 @@ private String getCountQueryWithMaxTrackedEntityLimit(TrackedEntityQueryParams p
+ getQuerySelect(params)
+ "FROM "
+ getFromSubQuery(params, true, null)
+ (params.getProgram().getMaxTeiCountToReturn() > 0
? getLimitClause(params.getProgram().getMaxTeiCountToReturn() + 1)
+ (params.getEnrolledInTrackerProgram().getMaxTeiCountToReturn() > 0
? getLimitClause(params.getEnrolledInTrackerProgram().getMaxTeiCountToReturn() + 1)
: "")
+ " ) tecount";
}
Expand Down Expand Up @@ -439,10 +467,10 @@ private String joinPrograms(TrackedEntityQueryParams params) {
trackedEntity.append(" INNER JOIN program P ");
trackedEntity.append(" ON P.trackedentitytypeid = TE.trackedentitytypeid ");

if (!params.hasProgram()) {
if (!params.hasEnrolledInTrackerProgram()) {
trackedEntity
.append("AND P.programid IN (")
.append(getCommaDelimitedString(getIdentifiers(params.getPrograms())))
.append(getCommaDelimitedString(getIdentifiers(params.getAccessibleTrackerPrograms())))
.append(")");
}

Expand Down Expand Up @@ -472,7 +500,7 @@ private String getFromSubQueryTrackedEntityConditions(
.append(whereAnd.whereAnd())
.append("TE.trackedentitytypeid = ")
.append(params.getTrackedEntityType().getId());
} else if (!params.hasProgram()) {
} else if (!params.hasEnrolledInTrackerProgram()) {
trackedEntity
.append(whereAnd.whereAnd())
.append("TE.trackedentitytypeid in (")
Expand Down Expand Up @@ -594,10 +622,10 @@ private String getFromSubQueryJoinOrderByAttributes(TrackedEntityQueryParams par
* @return a SQL INNER JOIN for program owner, or a LEFT JOIN if no program is specified.
*/
private String getFromSubQueryJoinProgramOwnerConditions(TrackedEntityQueryParams params) {
if (params.hasProgram()) {
if (params.hasEnrolledInTrackerProgram()) {
return " INNER JOIN trackedentityprogramowner PO "
+ " ON PO.programid = "
+ params.getProgram().getId()
+ params.getEnrolledInTrackerProgram().getId()
+ " AND PO.trackedentityid = TE.trackedentityid "
+ " AND P.programid = PO.programid";
}
Expand Down Expand Up @@ -662,7 +690,7 @@ private void handleOrganisationUnits(TrackedEntityQueryParams params) {
}

private String getOwnerOrgUnit(TrackedEntityQueryParams params) {
if (params.hasProgram()) {
if (params.hasEnrolledInTrackerProgram()) {
return "PO.organisationunitid ";
}

Expand Down Expand Up @@ -736,10 +764,10 @@ private String getFromSubQueryJoinEnrollmentConditions(TrackedEntityQueryParams
ON %1$s.trackedentityid = TE.trackedentityid
""";

return !params.hasProgram()
return !params.hasEnrolledInTrackerProgram()
? join.formatted(ENROLLMENT_ALIAS)
: join.concat(" AND %1$s.programid = %2$s")
.formatted(ENROLLMENT_ALIAS, params.getProgram().getId());
.formatted(ENROLLMENT_ALIAS, params.getEnrolledInTrackerProgram().getId());
}

return "";
Expand All @@ -757,7 +785,7 @@ private String getFromSubQueryEnrollmentConditions(
SqlHelper whereAnd, TrackedEntityQueryParams params) {
StringBuilder program = new StringBuilder();

if (!params.hasProgram()) {
if (!params.hasEnrolledInTrackerProgram()) {
return "";
}

Expand All @@ -774,7 +802,7 @@ private String getFromSubQueryEnrollmentConditions(
program
.append("WHERE EN.trackedentityid = TE.trackedentityid ")
.append("AND EN.programid = ")
.append(params.getProgram().getId())
.append(params.getEnrolledInTrackerProgram().getId())
.append(SPACE);

if (params.hasEnrollmentStatus()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ public TrackedEntityQueryParams map(
program, requestedTrackedEntityType, operationParams, orgUnits, params);

params
.setProgram(program)
.setPrograms(programs)
.setEnrolledInTrackerProgram(program)
.setAccessibleTrackerPrograms(programs)
.setProgramStage(programStage)
.setEnrollmentStatus(operationParams.getEnrollmentStatus())
.setFollowUp(operationParams.getFollowUp())
Expand Down Expand Up @@ -297,16 +297,17 @@ private void validateGlobalSearchParameters(TrackedEntityQueryParams params, Use
private List<UID> getSearchableAttributeIds(TrackedEntityQueryParams params) {
List<UID> searchableAttributeIds = new ArrayList<>();

if (params.hasProgram()) {
searchableAttributeIds.addAll(UID.of(params.getProgram().getSearchableAttributeIds()));
if (params.hasEnrolledInTrackerProgram()) {
searchableAttributeIds.addAll(
UID.of(params.getEnrolledInTrackerProgram().getSearchableAttributeIds()));
}

if (params.hasTrackedEntityType()) {
searchableAttributeIds.addAll(
UID.of(params.getTrackedEntityType().getSearchableAttributeIds()));
}

if (!params.hasProgram() && !params.hasTrackedEntityType()) {
if (!params.hasEnrolledInTrackerProgram() && !params.hasTrackedEntityType()) {
searchableAttributeIds.addAll(
trackedEntityAttributeService.getAllSystemWideUniqueTrackedEntityAttributes().stream()
.map(UID::of)
Expand Down Expand Up @@ -346,13 +347,13 @@ private int getMaxTeiLimit(TrackedEntityQueryParams params) {
}
}

if (params.hasProgram()) {
maxTeiLimit = params.getProgram().getMaxTeiCountToReturn();
if (params.hasEnrolledInTrackerProgram()) {
maxTeiLimit = params.getEnrolledInTrackerProgram().getMaxTeiCountToReturn();

if (!params.hasTrackedEntities() && isProgramMinAttributesViolated(params)) {
throw new IllegalQueryException(
"At least "
+ params.getProgram().getMinAttributesRequiredToSearch()
+ params.getEnrolledInTrackerProgram().getMinAttributesRequiredToSearch()
+ " attributes should be mentioned in the search criteria.");
}
}
Expand Down Expand Up @@ -405,9 +406,11 @@ private boolean isProgramMinAttributesViolated(TrackedEntityQueryParams params)
return false;
}

return (!params.hasFilters() && params.getProgram().getMinAttributesRequiredToSearch() > 0)
return (!params.hasFilters()
&& params.getEnrolledInTrackerProgram().getMinAttributesRequiredToSearch() > 0)
|| (params.hasFilters()
&& params.getFilters().size() < params.getProgram().getMinAttributesRequiredToSearch());
&& params.getFilters().size()
< params.getEnrolledInTrackerProgram().getMinAttributesRequiredToSearch());
}

private void checkIfMaxTeiLimitIsReached(TrackedEntityQueryParams params, int maxTeiLimit) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,17 @@ public class TrackedEntityQueryParams {
*/
private Set<OrganisationUnit> orgUnits = new HashSet<>();

/** Program for which instances in the response must be enrolled in. */
private Program program;
/**
* Tracker program the tracked entity must be enrolled in. This should not be set when {@link
* #accessibleTrackerPrograms} is set. The user must have data read access to this program.
*/
private Program enrolledInTrackerProgram;

/** Programs to fetch. */
private List<Program> programs = List.of();
/**
* Tracker programs the user has data read access to. This should not be set when {@link
* #enrolledInTrackerProgram} is set.
*/
private List<Program> accessibleTrackerPrograms = List.of();

/** Status of a tracked entities enrollment into a given program. */
private EnrollmentStatus enrollmentStatus;
Expand Down Expand Up @@ -172,8 +178,8 @@ public boolean hasOrganisationUnits() {
}

/** Indicates whether these parameters specify a program. */
public boolean hasProgram() {
return program != null;
public boolean hasEnrolledInTrackerProgram() {
return enrolledInTrackerProgram != null;
}

/** Indicates whether these parameters specify an enrollment status. */
Expand Down Expand Up @@ -308,21 +314,22 @@ public TrackedEntityQueryParams setOrgUnits(Set<OrganisationUnit> accessibleOrgU
return this;
}

public Program getProgram() {
return program;
public Program getEnrolledInTrackerProgram() {
return enrolledInTrackerProgram;
}

public TrackedEntityQueryParams setProgram(Program program) {
this.program = program;
public TrackedEntityQueryParams setEnrolledInTrackerProgram(Program enrolledInTrackerProgram) {
this.enrolledInTrackerProgram = enrolledInTrackerProgram;
return this;
}

public List<Program> getPrograms() {
return programs;
public List<Program> getAccessibleTrackerPrograms() {
return accessibleTrackerPrograms;
}

public TrackedEntityQueryParams setPrograms(List<Program> programs) {
this.programs = programs;
public TrackedEntityQueryParams setAccessibleTrackerPrograms(
List<Program> accessibleTrackerPrograms) {
this.accessibleTrackerPrograms = accessibleTrackerPrograms;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ public List<TrackedEntity> find(

Stream<String> teUidStream = trackedEntities.keySet().parallelStream();

if (user.isPresent() && queryParams.hasProgram()) {
if (user.isPresent() && queryParams.hasEnrolledInTrackerProgram()) {
teUidStream = teUidStream.filter(ownedTeis::containsKey);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,10 @@ private Multimap<String, String> getOwnedTrackedEntitiesPartitioned(

String sql;

if (ctx.getQueryParams().hasProgram()) {
if (ctx.getQueryParams().hasEnrolledInTrackerProgram()) {
sql = getTrackedEntitiesOwnershipSqlForSpecificProgram(skipUserScopeValidation);
paramSource.addValue("programUid", ctx.getQueryParams().getProgram().getUid());
paramSource.addValue(
"programUid", ctx.getQueryParams().getEnrolledInTrackerProgram().getUid());
} else if (checkForOwnership) {
sql = getTrackedEntitiesOwnershipSqlForAllPrograms(skipUserScopeValidation);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ void testMappingProgram() throws BadRequestException, ForbiddenException {

TrackedEntityQueryParams params = mapper.map(operationParams, user);

assertEquals(program, params.getProgram());
assertEquals(program, params.getEnrolledInTrackerProgram());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@
import org.hisp.dhis.trackedentity.TrackedEntityTypeAttribute;
import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue;
import org.hisp.dhis.tracker.acl.TrackedEntityProgramOwnerService;
import org.hisp.dhis.tracker.export.Page;
import org.hisp.dhis.tracker.export.PageParams;
import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityOperationParams.TrackedEntityOperationParamsBuilder;
import org.hisp.dhis.tracker.trackedentityattributevalue.TrackedEntityAttributeValueService;
import org.hisp.dhis.user.User;
Expand Down Expand Up @@ -258,6 +260,7 @@ void setUp() {
new TrackedEntityTypeAttribute(trackedEntityTypeA, teaB)));
trackedEntityTypeA.getSharing().setOwner(user);
trackedEntityTypeA.setPublicAccess(AccessStringHelper.FULL);
trackedEntityTypeA.setMinAttributesRequiredToSearch(0);
manager.save(trackedEntityTypeA, false);

CategoryCombo defaultCategoryCombo = manager.getByName(CategoryCombo.class, "default");
Expand Down Expand Up @@ -1896,6 +1899,39 @@ void shouldFailWhenRequestingSingleTEAndTETNotAccessible() {
exception.getMessage());
}

@Test
void shouldReturnEmptyResultIfUserHasNoAccessToAnyTrackerProgram()
throws ForbiddenException, BadRequestException, NotFoundException {
injectSecurityContextUser(getAdminUser());
makeProgramMetadataAccessibleOnly(programA);
makeProgramMetadataAccessibleOnly(programB);
makeProgramMetadataAccessibleOnly(programC);
injectSecurityContextUser(user);

TrackedEntityOperationParams operationParams =
TrackedEntityOperationParams.builder().trackedEntityType(trackedEntityTypeA).build();

assertIsEmpty(trackedEntityService.getTrackedEntities(operationParams));
}

@Test
void shouldReturnEmptyPageIfUserHasNoAccessToAnyTrackerProgram()
throws ForbiddenException, BadRequestException, NotFoundException {
injectSecurityContextUser(getAdminUser());
makeProgramMetadataAccessibleOnly(programA);
makeProgramMetadataAccessibleOnly(programB);
makeProgramMetadataAccessibleOnly(programC);
injectSecurityContextUser(user);

TrackedEntityOperationParams operationParams =
TrackedEntityOperationParams.builder().trackedEntityType(trackedEntityTypeA).build();

Page<TrackedEntity> trackedEntities =
trackedEntityService.getTrackedEntities(operationParams, new PageParams(1, 3, false));

assertIsEmpty(trackedEntities.getItems());
}

@Test
void shouldFailWhenRequestingSingleTEAndNoMetadataAccessToAnyProgram() {
injectAdminIntoSecurityContext();
Expand Down
Loading

0 comments on commit 048e3a1

Please sign in to comment.