Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add prev/nextPage to /tracker/trackedEntities DHIS2-19021 #20031

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

teleivo
Copy link
Contributor

@teleivo teleivo commented Feb 21, 2025

following #20029

This change affects paginated TE search when there is no TE limit setup

} else if (limit == 0) {
return limitOffset
.append(LIMIT)
.append(SPACE)
.append(pageParams.getPageSize())
.append(SPACE)
.append(OFFSET)
.append(SPACE)
.append(pageParams.getOffset())
.append(SPACE)
.toString();
} else if (pageParams != null) {

I need to investigate how pagination affects the TE limit that in configurable in some way

} else if (pageParams != null) {
return limitOffset
.append(LIMIT)
.append(SPACE)
.append(Math.min(limit + 1, pageParams.getPageSize()))
.append(SPACE)
.append(OFFSET)
.append(SPACE)
.append(pageParams.getOffset())
.append(SPACE)
.toString();
} else {

I'll do this in another PR. I first need to figure out how to test it.

@teleivo teleivo marked this pull request as ready for review February 21, 2025 15:11
@teleivo teleivo requested a review from a team as a code owner February 21, 2025 15:11
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 New issue
1 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

params.setMaxTeLimit(maxTeiLimit);
int maxTeLimit = getMaxTeLimit(params);
checkIfMaxTeLimitIsReached(params, maxTeLimit);
params.setMaxTeLimit(maxTeLimit);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you move this like one up, then you don't need to send the maxTeLimit as a parameter to the checkIfMaxTeLimitIsReached function anymore, as it is already contained in the params parameter.

private void checkIfMaxTeiLimitIsReached(TrackedEntityQueryParams params, int maxTeiLimit) {
if (maxTeiLimit > 0) {
private void checkIfMaxTeLimitIsReached(TrackedEntityQueryParams params, int maxTeLimit) {
if (maxTeLimit > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NitPicky, but if the maxTeLimit is not sent as an additional parameter, and if reusing the one inside params , a method like hasMaxTeLimit inside TrackedEntityQueryParams could be used here instead, that does the same check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should not have renamed the existing code 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants