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

fix: Always add default order as last order param in tracker exporters [DHIS2-18659] #19551

Merged
merged 4 commits into from
Dec 20, 2024

Conversation

enricocolasante
Copy link
Contributor

@enricocolasante enricocolasante commented Dec 20, 2024

Always add default order as a last order parameter to have a consistent order between results.

@enricocolasante enricocolasante marked this pull request as ready for review December 20, 2024 13:34
@enricocolasante enricocolasante requested a review from a team as a code owner December 20, 2024 13:34
@enricocolasante enricocolasante requested a review from a team December 20, 2024 13:44
@enricocolasante enricocolasante enabled auto-merge (squash) December 20, 2024 13:44
@@ -279,7 +279,7 @@ private static String orderBy(List<Order> orders) {
orderJoiner.add(
order.getField() + " " + (order.getDirection().isAscending() ? "asc" : "desc"));
}
return " order by " + orderJoiner;
return " order by " + orderJoiner + ", " + DEFAULT_ORDER;
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there is no consequence if the same DEFAULT_ORDER is also part of the orderJoiner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default order, aka entity id, it is not part of orderable fields, so it cannot ever be part of the orderJoiner here

@enricocolasante enricocolasante merged commit 6cb0d1f into master Dec 20, 2024
17 checks passed
@enricocolasante enricocolasante deleted the DHIS2-18659 branch December 20, 2024 13:57
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.

3 participants