[Bug 19234] New: Add query parameters handling helpers
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Bug ID: 19234 Summary: Add query parameters handling helpers Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org In order to ease endpoint writing, we need a common (and fully tested) way of dealing with two things: - Extract pagination controlling params from the query params and create the right pagination setup for Koha::Objects - Generate the right DBIC query for the passed params -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |kyle@bywatersolutions.com, | |lari.taskula@jns.fi -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 66752 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66752&action=edit Bug 19234: Unit tests for query parameters handling helpers This patch adds unit tests for the new query parameters handling Mojo plugin. Sponsored-by: Camden County Sponsored-by: ByWater Solutions -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 66753 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66753&action=edit Bug 19234: Add query parameters handling helpers This patch introduces a Mojolicious plugin to be used on the REST api. It adds a helper method: generate_dbic_query =================== When used, it generates what's needed to perform a search on DBIC/Koha::Objects like this: my $params = $c->validation->output; my ($filter, $attributes) = $c->generate_dbic_query($params); my $patrons = Koha::Patrons->search( $filter, $attributes ); It introduces a reserved word _exact_match that controls the generated $filter: - Default behaviour: right truncation on the query params - _exact_match=1: no truncation added. All the plugin's behaviour is tested. To test: - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater solutions Sponsored-by: Camden County -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 66754 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66754&action=edit Bug 19234: (followup) Make Query plugin available to endpoints -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |18731 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18731 [Bug 18731] REST api: Add an endpoint for acquisition orders -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |18137, 19196 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18137 [Bug 18137] REST API: Migrate from Mojolicious::Plugin::Swagger2 to Mojolicious::Plugin::OpenAPI https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19196 [Bug 19196] Add pagination helpers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|18137 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18137 [Bug 18137] REST API: Migrate from Mojolicious::Plugin::Swagger2 to Mojolicious::Plugin::OpenAPI -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #66752|0 |1 is obsolete| | Attachment #66753|0 |1 is obsolete| | Attachment #66754|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 66991 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66991&action=edit Bug 19234: Unit tests for query parameters handling helpers This patch adds unit tests for the new query parameters handling Mojo plugin. Sponsored-by: Camden County Sponsored-by: ByWater Solutions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 66992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66992&action=edit Bug 19234: Add query parameters handling helpers This patch introduces a Mojolicious plugin to be used on the REST api. It adds a helper method: generate_dbic_query =================== When used, it generates what's needed to perform a search on DBIC/Koha::Objects like this: my $params = $c->validation->output; my ($filter, $attributes) = $c->generate_dbic_query($params); my $patrons = Koha::Patrons->search( $filter, $attributes ); It introduces a reserved word _exact_match that controls the generated $filter: - Default behaviour: right truncation on the query params - _exact_match=1: no truncation added. All the plugin's behaviour is tested. To test: - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater solutions Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 66993 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66993&action=edit Bug 19234: (followup) Make Query plugin available to endpoints Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Architecture, internals, |REST api |and plumbing | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 66991 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66991 Bug 19234: Unit tests for query parameters handling helpers Review of attachment 66991: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=19234&attachment=66991) ----------------------------------------------------------------- ::: t/Koha/REST/Plugin/Query.t @@ +68,5 @@
+ + $t->get_ok('/query') + ->status_is( 200 ) + ->json_is( '/attributes' => { page => 2, rows => 3 } ) + ->json_is( '/filter' => { firstname => { like => 'Manuel%' }, surname => { like => 'Cohen Arazi%' } } );
Is it by design that this 'like' is actually a 'starts with' rather than a 'contains' search? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This somehow feels like a step too far in trying to generalise a mapping between api query parameters and database fields.. perhaps I'm wrong though and we're aiming to try to keep that link strongly bound.. perhaps some examples of how you're expecting to use this outside of just the pagination merging. On the pagination side.. I think I'd probably add that slice of code to ::Plugin::Pagination (to keep it with the pagination stuff) and name it something along the lines of 'dbic_merge_pagination_attributes'? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OR.. perhaps you need some sort of object (be that Koha:: or Schema::Result) introspection to match up fields to query parameters in there? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 66992 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=66992 Bug 19234: Add query parameters handling helpers Review of attachment 66992: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=19234&attachment=66992) ----------------------------------------------------------------- ::: Koha/REST/Plugin/Query.pm @@ +68,5 @@
+ my ( $page, $per_page ) = @_; + + my $attributes = { + rows => $per_page, + page => $page
As a general rule when doing paging one should also always explicitly set an order_by parameter to ensure the resultset is always sorted consistently between paged queries. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- order_by is missing. This needs to be fixed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #7)
Comment on attachment 66991 [details] [review] Bug 19234: Unit tests for query parameters handling helpers
Review of attachment 66991 [details] [review]: -----------------------------------------------------------------
::: t/Koha/REST/Plugin/Query.t @@ +68,5 @@
+ + $t->get_ok('/query') + ->status_is( 200 ) + ->json_is( '/attributes' => { page => 2, rows => 3 } ) + ->json_is( '/filter' => { firstname => { like => 'Manuel%' }, surname => { like => 'Cohen Arazi%' } } );
Is it by design that this 'like' is actually a 'starts with' rather than a 'contains' search?
I was just keeping the current behaviour. If we added more options (contains, left truncation, etc) we should still keep right truncation as default to avoid unneeded behaviour changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #8)
This somehow feels like a step too far in trying to generalise a mapping between api query parameters and database fields.. perhaps I'm wrong though and we're aiming to try to keep that link strongly bound.. perhaps some examples of how you're expecting to use this outside of just the pagination merging.
This helper is only intended to extract reserved params from the query (so a single point for having this written, and testing it, thus becoming a standard on the codebase too). It is up to the controller methods to pass the resulting $filter as-is or chew it a bit more. But I agree the naming of the method, and the sample usage is misleading.
On the pagination side.. I think I'd probably add that slice of code to ::Plugin::Pagination (to keep it with the pagination stuff) and name it something along the lines of 'dbic_merge_pagination_attributes'?
Adding to my response above, I think this should just extract the reserved params, and return the two hashrefs it returns now, without mentioning attributes and filters. And yes, building the DBIC 'pagination filter' should be left to the Pagination helper. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #66991|0 |1 is obsolete| | --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 67358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67358&action=edit Bug 19234: Unit tests for query parameters handling helpers This patch adds unit tests for the new query parameters handling Mojo plugin. Sponsored-by: Camden County Sponsored-by: ByWater Solutions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #66992|0 |1 is obsolete| | --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 67359 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67359&action=edit Bug 19234: Add query parameters handling helpers This patch introduces a Mojolicious plugin to be used on the REST api. It adds a helper method: generate_dbic_query =================== When used, it generates what's needed to perform a search on DBIC/Koha::Objects like this: my $params = $c->validation->output; my ($filtered_params, $reserved_params) = $c->extract_reserved_params($params); my $filter = do_smth($filtered_params, $reserved_params); my $attributes = do_smth_reserved($reserved_params); my $patrons = Koha::Patrons->search( $filter, $attributes ); It introduces reserved param names: - _match - _order_by - _page - _per_page They are reserved for later usage (pagination, matching algorithm on building DB queries) All the plugin's behaviour is tested. To test: - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater solutions Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #66993|0 |1 is obsolete| | --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 67360 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67360&action=edit Bug 19234: (followup) Make Query plugin available to endpoints Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Failed QA |Needs Signoff CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Kyle, Martin, I have simplified this plugin, making it only extract reserved params from the query, and providing a 'cleaned up' attributes hash. The commit message should be clear on how this is supposed to be used. New helpers could be added in order to: - Merge pagination params into $filter (for DBIC's ->search) - Generate DBI'c search query based on _match and the received params. - Handle order_by, this is important because we could add some reflection to Koha::Objects (or DBIC in order to have them say what columns can be used for sorting) Please give this a try ASAP so I move on on top of this. Thanks in advance! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67358|0 |1 is obsolete| | --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 67363 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67363&action=edit Bug 19234: Unit tests for query parameters handling helpers This patch adds unit tests for the new query parameters handling Mojo plugin. Sponsored-by: Camden County Sponsored-by: ByWater Solutions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67359|0 |1 is obsolete| | --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 67364 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67364&action=edit Bug 19234: Add query parameters handling helpers This patch introduces a Mojolicious plugin to be used on the REST api. It adds a helper method: generate_dbic_query =================== When used, it generates what's needed to perform a search on DBIC/Koha::Objects like this: my $params = $c->validation->output; my ($filtered_params, $reserved_params) = $c->extract_reserved_params($params); my $filter = do_smth($filtered_params, $reserved_params); my $attributes = do_smth_reserved($reserved_params); my $patrons = Koha::Patrons->search( $filter, $attributes ); It introduces reserved param names: - _match - _order_by - _page - _per_page They are reserved for later usage (pagination, matching algorithm on building DB queries) All the plugin's behaviour is tested. To test: - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater solutions Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67360|0 |1 is obsolete| | --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 67365 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67365&action=edit Bug 19234: (followup) Make Query plugin available to endpoints Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19369 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19369 [Bug 19369] Add a helper function for translating pagination params into SQL::Abstract -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19370 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19370 [Bug 19370] Add a helper function for translating order_by params into SQL::Abstract -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67363|0 |1 is obsolete| | Attachment #67364|0 |1 is obsolete| | Attachment #67365|0 |1 is obsolete| | --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 67374 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67374&action=edit Bug 19234: Unit tests for query parameters handling helpers This patch adds unit tests for the new query parameters handling Mojo plugin. Sponsored-by: Camden County Sponsored-by: ByWater Solutions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 67375 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67375&action=edit Bug 19234: Add query parameters handling helpers This patch introduces a Mojolicious plugin to be used on the REST api. It adds a helper method: generate_dbic_query =================== When used, it generates what's needed to perform a search on DBIC/Koha::Objects like this: my $params = $c->validation->output; my ($filtered_params, $reserved_params) = $c->extract_reserved_params($params); my $filter = do_smth($filtered_params, $reserved_params); my $attributes = do_smth_reserved($reserved_params); my $patrons = Koha::Patrons->search( $filter, $attributes ); It introduces reserved param names: - _match - _order_by - _page - _per_page They are reserved for later usage (pagination, matching algorithm on building DB queries) All the plugin's behaviour is tested. To test: - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater solutions Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 67377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67377&action=edit Bug 19234: (followup) Make Query plugin available to endpoints Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19410 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19410 [Bug 19410] Koha::Objects needs a ->search_for_api method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67374|0 |1 is obsolete| | --- Comment #24 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 67785 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67785&action=edit Bug 19234: Unit tests for query parameters handling helpers This patch adds unit tests for the new query parameters handling Mojo plugin. Sponsored-by: Camden County Sponsored-by: ByWater Solutions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67375|0 |1 is obsolete| | --- Comment #25 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 67786 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67786&action=edit Bug 19234: Add query parameters handling helpers This patch introduces a Mojolicious plugin to be used on the REST api. It adds a helper method: generate_dbic_query =================== When used, it generates what's needed to perform a search on DBIC/Koha::Objects like this: my $params = $c->validation->output; my ($filtered_params, $reserved_params) = $c->extract_reserved_params($params); my $filter = do_smth($filtered_params, $reserved_params); my $attributes = do_smth_reserved($reserved_params); my $patrons = Koha::Patrons->search( $filter, $attributes ); It introduces reserved param names: - _match - _order_by - _page - _per_page They are reserved for later usage (pagination, matching algorithm on building DB queries) All the plugin's behaviour is tested. To test: - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater solutions Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67377|0 |1 is obsolete| | --- Comment #26 from Lari Taskula <lari.taskula@jns.fi> --- Created attachment 67787 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67787&action=edit Bug 19234: (followup) Make Query plugin available to endpoints Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_17_11_candidate CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #27 from Julian Maurice <julian.maurice@biblibre.com> --- Maybe I'm missing the big picture here, but I don't understand why this plugin is needed. All it does is separate query parameters so one can pass 'reserved' parameters to others mojo helpers (at least that's how it's used in bug 19410), but helpers already have access to those parameters through their first parameter ($c), right ? And not all helpers will have a use for all reserved parameters, so passing all those parameters to all helpers is useless, they can retrieve what they need on their own. Please explain :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #28 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Julian Maurice from comment #27)
Maybe I'm missing the big picture here, but I don't understand why this plugin is needed. All it does is separate query parameters so one can pass 'reserved' parameters to others mojo helpers (at least that's how it's used in bug 19410), but helpers already have access to those parameters through their first parameter ($c), right ? And not all helpers will have a use for all reserved parameters, so passing all those parameters to all helpers is useless, they can retrieve what they need on their own.
Please explain :)
The idea behind this was: - To make sure some things are reserved and make it explicit. Have the list in a single place, make it grow if needed. - Make parameters extraction algorithm testable, so no need to add thousands of tests for each endpoint... and ... - Code less: we don't need to write useless repeating loops extracting parameters and separating the reserved ones on each controller method. Koha::Objects->search_for_api is a clear example of how this would improve productivity when writing endpoints. If you look at how a controller looks like using it when it is just a wrapper or Koha::Objects-derived classes, it becomes clear: sub list { my $c = ... my $objects = Koha::Blah->search_for_api($c); return $c->render( openapi => $objects, status => 200 ); } In this tiny example, pagination, sorting and query building choice are solved already using this helpers. And those who require more flexibility (business endpoints, that are not just wrappers to Koha::Object(s) derived classes) can just skip using them and do more interesting stuff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_17_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67785|0 |1 is obsolete| | Attachment #67786|0 |1 is obsolete| | Attachment #67787|0 |1 is obsolete| | --- Comment #29 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 69360 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69360&action=edit Bug 19234: Unit tests for query parameters handling helpers This patch adds unit tests for the new query parameters handling Mojo plugin. Sponsored-by: Camden County Sponsored-by: ByWater Solutions Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #30 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 69361 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69361&action=edit Bug 19234: Add query parameters handling helpers This patch introduces a Mojolicious plugin to be used on the REST api. It adds a helper method: generate_dbic_query =================== When used, it generates what's needed to perform a search on DBIC/Koha::Objects like this: my $params = $c->validation->output; my ($filtered_params, $reserved_params) = $c->extract_reserved_params($params); my $filter = do_smth($filtered_params, $reserved_params); my $attributes = do_smth_reserved($reserved_params); my $patrons = Koha::Patrons->search( $filter, $attributes ); It introduces reserved param names: - _match - _order_by - _page - _per_page They are reserved for later usage (pagination, matching algorithm on building DB queries) All the plugin's behaviour is tested. To test: - Run: $ sudo koha-shell kohadev k$ cd kohaclone k$ prove t/Koha/REST/Plugin/Query.t => SUCCESS: Tests pass! - Sign off :-D Sponsored-by: ByWater solutions Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 --- Comment #31 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 69362 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69362&action=edit Bug 19234: (followup) Make Query plugin available to endpoints Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Lari Taskula <lari.taskula@jns.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #32 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 18.05, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED CC| |nick@bywatersolutions.com --- Comment #33 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Enhancement, skipping for 17.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19234 Bug 19234 depends on bug 19196, which changed state. Bug 19196 Summary: Add pagination helpers https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19196 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org