It would be handy to have a helper function in Koha::REST::Plugin::Query generating the 'order_by' attribute out of the query params, to be used when listing objects on the REST api endpoints.
Created attachment 67379 [details] [review] Bug 19370: Unit tests
Created attachment 67380 [details] [review] Bug 19370: Add helper function for order_by attribute generation This patch introduces a helper function called 'dbic_merge_sorting' to Koha::REST::Plugin::Query. This simple function adds SQL::Abstract order_by attribute to the passed $filter hashref, as explained in the POD. It introduces a syntax for passing sorting params on the request to the REST api. The proposed syntax has been found in the wild, and is pretty trivial to parse/work with: GET /api/v1/<endpoint>?order_by=+column_1|-column_2|column_3 As explained on the POD, + stands for 'asc' and - for 'desc'. If ommited, it defaults to the DB engine default (usually asc). To test: - Apply this patches - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! And they make sense! :-P - Sign off :-D Sponsored-by: Camden County
Created attachment 67402 [details] [review] Bug 19370: Unit tests
Created attachment 67403 [details] [review] Bug 19370: Add helper function for order_by attribute generation This patch introduces a helper function called 'dbic_merge_sorting' to Koha::REST::Plugin::Query. This simple function adds SQL::Abstract order_by attribute to the passed $filter hashref, as explained in the POD. It introduces a syntax for passing sorting params on the request to the REST api. The proposed syntax has been found in the wild, and is pretty trivial to parse/work with: GET /api/v1/<endpoint>?order_by=+column_1|-column_2|column_3 As explained on the POD, + stands for 'asc' and - for 'desc'. If ommited, it defaults to the DB engine default (usually asc). To test: - Apply this patches - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! And they make sense! :-P - Sign off :-D Edit: renamed params to match DBIC terminology. My bad :-D (tcohen) Sponsored-by: Camden County
Created attachment 67525 [details] [review] Bug 19369: (followup) Rename params for consitency with 19370
Is the patch "Bug 19369: (followup) Rename params for consitency with 19370" accidentally attached into this Bug?
(In reply to Lari Taskula from comment #6) > Is the patch "Bug 19369: (followup) Rename params for consitency with 19370" > accidentally attached into this Bug? Indeed :-D
Created attachment 67576 [details] [review] Bug 19370: Remove undefined variable warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 67782 [details] [review] Bug 19370: Unit tests Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Created attachment 67783 [details] [review] Bug 19370: Add helper function for order_by attribute generation This patch introduces a helper function called 'dbic_merge_sorting' to Koha::REST::Plugin::Query. This simple function adds SQL::Abstract order_by attribute to the passed $filter hashref, as explained in the POD. It introduces a syntax for passing sorting params on the request to the REST api. The proposed syntax has been found in the wild, and is pretty trivial to parse/work with: GET /api/v1/<endpoint>?order_by=+column_1|-column_2|column_3 As explained on the POD, + stands for 'asc' and - for 'desc'. If ommited, it defaults to the DB engine default (usually asc). To test: - Apply this patches - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! And they make sense! :-P - Sign off :-D Edit: renamed params to match DBIC terminology. My bad :-D (tcohen) Sponsored-by: Camden County Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Created attachment 67784 [details] [review] Bug 19370: Remove undefined variable warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Created attachment 69304 [details] [review] Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values This patch changes the _order_by definition so it's values are handled correctly by the Mojolicious::Plugin::OpenAPI plugin. Code is adjusted to expect a list instead of a (to-be-splitted) string. Tests are adjusted too. Note: In the process I noticed + on the URL represents a space, so the helper function is updated to handle both + and %2B as ascending. To test: - Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 69305 [details] [review] Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values This patch makes the helper handling _order_by params expect a list of values instead of a (to-be-splitted) string. The idea is that the OpenAPI plugin will take care of splitting pipe-delimited values if the spec is correctly defined. Note: In the process I noticed + on the URL represents a space, so the helper function is updated to handle both + and %2B as ascending. To test: - Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 69307 [details] [review] Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values This patch makes the helper handling _order_by params expect a list of values instead of a (to-be-splitted) string. The idea is that the OpenAPI plugin will take care of splitting pipe-delimited values if the spec is correctly defined. Note: In the process I noticed + on the URL represents a space, so the helper function is updated to handle both + and %2B as ascending. To test: - Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 69383 [details] [review] Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values This patch makes the helper handling _order_by params expect a list of values instead of a (to-be-splitted) string. The idea is that the OpenAPI plugin will take care of splitting pipe-delimited values if the spec is correctly defined. Note: In the process I noticed + on the URL represents a space, so the helper function is updated to handle both + and %2B as ascending. To test: - Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Edit: Removed rebasing leftover making the tests fail. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 69384 [details] [review] Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values This patch makes the helper handling _order_by params expect a list of values instead of a (to-be-splitted) string. The idea is that the OpenAPI plugin will take care of splitting pipe-delimited values if the spec is correctly defined. Note: In the process I noticed + on the URL represents a space, so the helper function is updated to handle both + and %2B as ascending. To test: - Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Edit: Removed rebasing leftover making the tests fail. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 69391 [details] [review] Bug 19370: Unit tests Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 69392 [details] [review] Bug 19370: Add helper function for order_by attribute generation This patch introduces a helper function called 'dbic_merge_sorting' to Koha::REST::Plugin::Query. This simple function adds SQL::Abstract order_by attribute to the passed $filter hashref, as explained in the POD. It introduces a syntax for passing sorting params on the request to the REST api. The proposed syntax has been found in the wild, and is pretty trivial to parse/work with: GET /api/v1/<endpoint>?order_by=+column_1|-column_2|column_3 As explained on the POD, + stands for 'asc' and - for 'desc'. If ommited, it defaults to the DB engine default (usually asc). To test: - Apply this patches - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! And they make sense! :-P - Sign off :-D Edit: renamed params to match DBIC terminology. My bad :-D (tcohen) Sponsored-by: Camden County Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 69393 [details] [review] Bug 19370: Remove undefined variable warning Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 69394 [details] [review] Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values This patch makes the helper handling _order_by params expect a list of values instead of a (to-be-splitted) string. The idea is that the OpenAPI plugin will take care of splitting pipe-delimited values if the spec is correctly defined. Note: In the process I noticed + on the URL represents a space, so the helper function is updated to handle both + and %2B as ascending. To test: - Run: $ kshell k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Edit: Removed rebasing leftover making the tests fail. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Pushed to master for 18.05, thanks to everybody involved!
Awesome work all! Enhancement, won't backport for for 17.11.01