feat: add sort option commands#175
Conversation
Mudaafi
left a comment
Mudaafi
left a comment
There was a problem hiding this comment.
This is looking good. I had some implementation suggestions, but looking at the timeline, I might just push those changes up directly
| private String sortBy; | ||
|
|
||
| @SerializedName("sort_order") | ||
| private String sortOrder; |
There was a problem hiding this comment.
Since sort_order can only be one of two values "ascending" | "descending", would it be better to type it as an enum?
| * @return returns the sort options response | ||
| * @throws ConstructorException if the request is invalid | ||
| */ | ||
| public SortOptionsResponse retrieveSortOptions(String section, String sortBy) |
There was a problem hiding this comment.
Rather than having this overload, we would probably prefer having an overload that takes in a RetrieveSortOptionsRequest so that we cover all the different request parameters.
| * @return returns the deleted sort options as JSON string | ||
| * @throws ConstructorException if the request is invalid | ||
| */ | ||
| public String deleteSortOptions(String sortBy, String sortOrder, String section) |
There was a problem hiding this comment.
This method only deletes one sort option. We can keep this method for ease of use and rename it as deleteSortOption, but we should have another method that takes in an array of sortOptions to allow for the deletion of multiple options at once
evanyan13
left a comment
evanyan13
left a comment
There was a problem hiding this comment.
LGTM!
Left some lint-related comments for consideration please
| } | ||
|
|
||
| /** | ||
| * Retrieves all sort options with default section "Products" |
There was a problem hiding this comment.
I think the comment here is meant to be:
Retrieves all sort options with provided sortBy value
| public void sortOptionDeserialization() throws Exception { | ||
| String json = | ||
| "{" | ||
| + "\"display_name\": \"Preis\"," |
There was a problem hiding this comment.
Is this meant to be "Price"?
Or is this left as a weird value by intention?
There was a problem hiding this comment.
I think it's German. I'll change it to be Price
| + "\"total_count\": 2," | ||
| + "\"sort_options\": [" | ||
| + " {" | ||
| + " \"display_name\": \"Preis\"," |
4 participants
Adds the possibility to manage (CRUD) the sort options through the java client.
I used the docs regarding the sort option management to create the operations and tested it to be working. Also created an integration test to hopefully prove that it's working as intended.