crowdin
Add the `orderBy` parameter to list methods
The Crowdin API has been enhanced with a new orderBy parameter in a number of list methods. It allows sorting the results by a specific field in ascending or descending order.
The list of methods that support the new parameter:
- List projects
- List branches
- List directories
- List files
- List strings
- List Translation Approvals
- List Language Translations
- List String Translations
- List String Comments
- List Screenshots
- List Concepts
- List Glossaries
- List Terms
- List TMs
- List TM Segments
- List Tasks
- List User Tasks
- List Project Members
- List Labels
- List Groups (Enterprise only)
- List Users (Enterprise only)
- List Teams (Enterprise only)
References:
@andrii-bodnar Starting out with open source contributions, I would like to take this up - by when would you want this to be done? Also I believe that this is the functionality that you are looking to implement: https://developer.crowdin.com/api/v2/#section/Introduction/Sorting with the request URL like "https://<hostname>/api/<version>/resource?orderBy=createdAt desc,name,priority asc,title desc"
@halfwind22 the new parameter should be added to the methods listed in the description.
For example: the following is the corresponding function for the "List project" method - https://github.com/crowdin/crowdin-api-client-java/blob/master/src/main/java/com/crowdin/client/projectsgroups/ProjectsGroupsApi.java#L122
Yes @andrii-bodnar , I absolutely get your point - my question was more around how would the request url would look like https://<hostname>/api/<version>/resource?orderBy=createdAt desc,name,priority asc,title desc probably?
@andrii-bodnar I propose to allow users to pass in a Linked Hash map for specifying ordering key and the corresponding ordering sequence(ASC/DESC - enums) . Or would it be better to allow the ordering specification in plaintext. My concern for the second approach is that an incorrect string might be passed , like ?orderBy name,id dsc or maybe missing out on a comma.
@halfwind22 could you please provide some examples of what this might look like? I agree that passing raw parameters is unsafe and error-prone.
// Client side code
Map<,SortOrder> orderByMap = new LinkedHashmap<String,SortOrder>();
orderByMap.put(name, SortOrder.DESC)
orderByMap.put(id, SortOrder.ASC)
// Method Signature
listProjects(Long groupId, Integer hasManagerAccess, Integer limit, Integer offset, LinkedHashMap<String, SortOrder> orderByMap)
@andrii-bodnar Above is how regular users would invoke the method. Now users might also want to just skip the SortOrder (which makes it ASC order) , so we might need to have a wrapper around the put() of the Map interface, to handle that scenario. Same goes for duplicate keys. Thoughts??
@halfwind22 I'm also curious about how to make the sort order value optional.
I don't think we should care about duplicate values, it's too complicated for this feature.
A wrapper over the put() method of Map interface should suffice. A simpler solution would be user entering a key with no value (null type) into the map and then we interpret that null as ASCENDING. We could also probably ignore it ,because the backend REST endpoint can still work with query params coming without a sorting order like ?orderBy name,id desc .
Hello @andrii-bodnar , a couple of endpoints are available to only enterprise customers, while I am on a free plan. Can I get access to some test account (or temporarily elevating my account privileges) , for testing those functionalities?
Hello @halfwind22, you can create an Enterprise organization here - https://accounts.crowdin.com/workspace/create
It will provide you with a 30-day trial period.
@andrii-bodnar Have a question: The sort keys for each of the components are different. For example groups can be sorted by id,name,description,createdAt,updatedAt , whereas directories are sorted by id, name,title,createdAt,updatedAt,exportPattern,priority. Currently, there is no check in place to ensure that only the right keys are used with each endpoint, and the onus is on the developer. Do we need to warn the user whenever an ineligible sort key is passed? Also if such a key is passed, what is the behavior on the server side?
@halfwind22 Let's proceed without checking for each specific endpoint. The server will return an error if a wrong key is passed.
@andrii-bodnar Done with the dev part of the code change, for unit tests, we need to change the JSON content, by adding more than response to account for list of items in sorted order. Can I go ahead with those? Also do you have a better approach for unit testing this feature?
@halfwind22, there is definitely no need to change all the tests for the list methods. It would be enough to add a separate test for each resource with the order parameter passed.
@andrii-bodnar Didn't quite get your point- so let me elaborate: In line 106 of the class https://github.com/crowdin/crowdin-api-client-java/blob/master/src/test/java/com/crowdin/client/projectsgroups/ProjectsGroupsApiTest.java, we have the test for listProjects . Currently it is designed to accept only 4 arguments, so it gives a compilation error in the absence of the 5th param(orderBy) that is present in the actual method. Also , I see the that mock responses, make use of just one project, which isn't enough to simulate the orderBy behavior, because even if we were to pass null, the default ordering must happen based on 'id'. Are you saying that I leave the current test untouched and instead create a new test with multiple projects, so as to simulate ordering behavior in case of a list of projects?
@halfwind22 the listProjects should not fail if the orderBy is not present in the request since it is an optional parameter. Also, there is no need to simulate the resource order in the actual response. It would be sufficient to check that the method accepts the new parameter correctly and adds it to the request query.
A bit weird ask: I am using the Eclipse IDE and it comes with a built in code style formatter. So after making changes, I accidentally formatted the file, and that means some of the formatting that was there originally is now changed. By formatting I mean tabs, new line ,etc. Could you please send me the checkstyle.xml of whatever formatter you are using? Otherwise the PR might highlight a lot of non-code changes too, leaving you confused.
@halfwind22 I don't have a checkstyle.xml file with the formatting rules. Please disable automatic formatting for the project and don't include these changes in the PR.
I use the Intellij IDEA IDE
Hi @Sprokof , I was done with the dev part long ago. However while writing the unit tests, I started facing some weird JUnit and Mockito issues with my IDE, and haven't got much time to look into this thereafter.Would you be interested in working on my fork? Need to add unit test cases.
@Sprokof Cool. Please confirm if you can access this branch, you should be able to see the changes.
https://github.com/halfwind22/crowdin-api-client-java/tree/feature/add-orderby-parameter