Bug 19370 - Add a helper function for translating order_by params into SQL::Abstract
Summary: Add a helper function for translating order_by params into SQL::Abstract
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST api (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 19234
Blocks: 18731 19410
  Show dependency treegraph
 
Reported: 2017-09-26 18:08 UTC by Tomás Cohen Arazi
Modified: 2017-12-20 11:42 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19370: Unit tests (1.46 KB, patch)
2017-09-26 18:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: Add helper function for order_by attribute generation (2.84 KB, patch)
2017-09-26 18:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: Unit tests (1.49 KB, patch)
2017-09-27 16:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: Add helper function for order_by attribute generation (2.94 KB, patch)
2017-09-27 16:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19369: (followup) Rename params for consitency with 19370 (2.45 KB, patch)
2017-10-02 16:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: Remove undefined variable warning (1.21 KB, patch)
2017-10-03 20:05 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: Unit tests (1.53 KB, patch)
2017-10-07 12:43 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19370: Add helper function for order_by attribute generation (2.99 KB, patch)
2017-10-07 12:43 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19370: Remove undefined variable warning (1.25 KB, patch)
2017-10-07 12:43 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values (3.67 KB, patch)
2017-11-23 13:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values (3.18 KB, patch)
2017-11-23 14:03 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values (4.40 KB, patch)
2017-11-23 14:06 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values (2.46 KB, patch)
2017-11-27 14:38 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values (3.16 KB, patch)
2017-11-27 14:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19370: Unit tests (1.59 KB, patch)
2017-11-27 16:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19370: Add helper function for order_by attribute generation (3.05 KB, patch)
2017-11-27 16:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19370: Remove undefined variable warning (1.31 KB, patch)
2017-11-27 16:46 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19370: (QA followup) Use OpenAPI's handling of pipe separated values (3.20 KB, patch)
2017-11-27 16:46 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2017-09-26 18:08:30 UTC
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.
Comment 1 Tomás Cohen Arazi 2017-09-26 18:29:39 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2017-09-26 18:29:45 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2017-09-27 16:30:00 UTC
Created attachment 67402 [details] [review]
Bug 19370: Unit tests
Comment 4 Tomás Cohen Arazi 2017-09-27 16:30:06 UTC
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
Comment 5 Tomás Cohen Arazi 2017-10-02 16:22:27 UTC
Created attachment 67525 [details] [review]
Bug 19369: (followup) Rename params for consitency with 19370
Comment 6 Lari Taskula 2017-10-03 10:55:03 UTC
Is the patch "Bug 19369: (followup) Rename params for consitency with 19370" accidentally attached into this Bug?
Comment 7 Tomás Cohen Arazi 2017-10-03 11:49:37 UTC
(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
Comment 8 Tomás Cohen Arazi 2017-10-03 20:05:58 UTC
Created attachment 67576 [details] [review]
Bug 19370: Remove undefined variable warning

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Lari Taskula 2017-10-07 12:43:23 UTC
Created attachment 67782 [details] [review]
Bug 19370: Unit tests

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 10 Lari Taskula 2017-10-07 12:43:33 UTC
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>
Comment 11 Lari Taskula 2017-10-07 12:43:48 UTC
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>
Comment 12 Tomás Cohen Arazi 2017-11-23 13:55:08 UTC Comment hidden (obsolete)
Comment 13 Tomás Cohen Arazi 2017-11-23 14:03:14 UTC Comment hidden (obsolete)
Comment 14 Tomás Cohen Arazi 2017-11-23 14:06:18 UTC
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>
Comment 15 Tomás Cohen Arazi 2017-11-27 14:38:16 UTC
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>
Comment 16 Tomás Cohen Arazi 2017-11-27 14:40:35 UTC
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>
Comment 17 Kyle M Hall 2017-11-27 16:46:13 UTC
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>
Comment 18 Kyle M Hall 2017-11-27 16:46:19 UTC
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>
Comment 19 Kyle M Hall 2017-11-27 16:46:22 UTC
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>
Comment 20 Kyle M Hall 2017-11-27 16:46:25 UTC
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>
Comment 21 Jonathan Druart 2017-12-11 20:47:42 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 22 Nick Clemens 2017-12-20 11:42:35 UTC
Awesome work all! Enhancement, won't backport for for 17.11.01