Skip to content

Commit

Permalink
fix: cannot get TE if there are no tracker programs (#19930)
Browse files Browse the repository at this point in the history
  • Loading branch information
teleivo authored Feb 14, 2025
1 parent 0313350 commit 72925ce
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 43 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

0 comments on commit 72925ce

Please sign in to comment.