[Bug 33080] New: Add helpers that return result_set for further processing
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Bug ID: 33080 Summary: Add helpers that return result_set for further processing Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: agustinmoyano@theke.io CC: tomascohen@gmail.com Add "search_rs" and "find_rs" helpers to REST API, which retrieves a result set with all search queries and attributes applied. -- 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=33080 --- Comment #1 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 147469 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147469&action=edit Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t -- 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=33080 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Assignee|koha-bugs@lists.koha-commun |agustinmoyano@theke.io |ity.org | Status|NEW |Needs Signoff -- 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=33080 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32734 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32734 [Bug 32734] REST API: Add a list endpoint for Biblios -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32735 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32735 [Bug 32735] REST API: Add a list endpoint for Authorities -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147469|0 |1 is obsolete| | --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147662&action=edit Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147663 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147663&action=edit Bug 33080: (follow-up) Minor cleanups Whilst reviewing I spotted a few calls to the stash that were no longer required and errors in the POD for the new/updated methods This patch adds fixes for those minor issues. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Comment on attachment 147662 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147662 Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api Review of attachment 147662: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=33080&attachment=147662) ----------------------------------------------------------------- ::: Koha/REST/Plugin/Objects.pm @@ +243,3 @@
# Perform search my $objects = $result_set->search( $filtered_params, $attributes );
I think we need to rework the pagination handling a little for this change. We can't really keep it inside this method now that it returns a resultset that we're expecting developers to then further filter and refine. I think we should update the add_pagination_headers helper to accept a resultset.. it can then build the total and base_total fields itself by executing the count and looking at the pager in the resultset passed. We could then call this automatically in the non-rs search header above and document that it should be called by the developer in their controller after they've applied their own additional filtering etc in the controller if they're using the _rs helper. For this.. I'm failing QA.. But I do really like where this is headed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've reviewed the follow-up on the branch Tomas sent me to.. more than happy to signoff on the set now :) Great work -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |https://gitlab.com/thekesol | |utions/Koha/-/tree/bug_3308 | |0_search_rs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #6 from Agustín Moyano <agustinmoyano@theke.io> --- (In reply to Martin Renvoize from comment #4)
Comment on attachment 147662 [details] [review] Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api
Review of attachment 147662 [details] [review]: -----------------------------------------------------------------
::: Koha/REST/Plugin/Objects.pm @@ +243,3 @@
# Perform search my $objects = $result_set->search( $filtered_params, $attributes );
I think we need to rework the pagination handling a little for this change. We can't really keep it inside this method now that it returns a resultset that we're expecting developers to then further filter and refine.
I think we should update the add_pagination_headers helper to accept a resultset.. it can then build the total and base_total fields itself by executing the count and looking at the pager in the resultset passed.
We could then call this automatically in the non-rs search header above and document that it should be called by the developer in their controller after they've applied their own additional filtering etc in the controller if they're using the _rs helper.
For this.. I'm failing QA.. But I do really like where this is headed.
Hi Martin, thanks for pointing this out... I do not believe we should change the add_pagination_headers helper, it really wouldn't change much of how we should call it. And it's actually called by some controllers, so we should change those also. Now we are calling it with base_total, total, and params... but if we change it to resultsets we should call it with filtered_resultset (to get total), unfiltered_resultset (to get base_total), and params. What I'll do is remove the add_pagination_headers call from objects->search_rs as you mentioned, and put that in objects->search instead. Cheers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #7 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 147720 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147720&action=edit Bug 33080: (follow-up) Move pagination logic out of search_rs -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Status|Signed Off |Failed QA --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Global symbol "$args" requires explicit package name (did you forget to declare "my $args"?) at /kohadevbox/koha/Koha/REST/Plugin/Objects.pm line 128. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #9 from Agustín Moyano <agustinmoyano@theke.io> --- Sorry, made it in a hurry... And then Tomas told me he was actually working on this with a different approach. So I'll just leave it to him to delete my patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Agustín Moyano from comment #9)
Sorry, made it in a hurry... And then Tomas told me he was actually working on this with a different approach. So I'll just leave it to him to delete my patch.
Yeah, I've worked on this on the referenced repo/branch. See URL. But right now I'm struggling with the ERM endpoints, which do funky use of the pagination headers and I'm not finding unit tests to make sure I don't introduce regressions. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #11 from Agustín Moyano <agustinmoyano@theke.io> --- Hm... maybe you should leave add_pagination_headers as is, and create a new helper called something like 'calc_pagination_numbers' that creates total numbers and calls add_pagination_headers somehow -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Agustín Moyano from comment #11)
Hm... maybe you should leave add_pagination_headers as is, and create a new helper called something like 'calc_pagination_numbers' that creates total numbers and calls add_pagination_headers somehow
I'm pondering an intermediate solution in which parameters can be passed as now, but there's a fallback to using the stashed values. To put it clear, the use case that is requiring this is: we need the base total available in objects.search to be passed to add_pagination_headers later. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147720|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147662|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147663|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 147734 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147734&action=edit Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 147735 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147735&action=edit Bug 33080: (follow-up) Minor cleanups Whilst reviewing I spotted a few calls to the stash that were no longer required and errors in the POD for the new/updated methods This patch adds fixes for those minor issues. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 147736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147736&action=edit Bug 33080: Add small warpper helper to call add_pagination_headers This patch introduces a small helper that uses the stashed values in `search_rs` to properly build the pagination headers based on the resultset. To test: 1. Run all the API tests and make sure nothing breaks 2. Run: $ ktd --shell $ t/db_dependent/Koha/REST/Plugin/Pagination.t => SUCCESS: Tests pass! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147734|0 |1 is obsolete| | --- Comment #16 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147788&action=edit Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147735|0 |1 is obsolete| | --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147789 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147789&action=edit Bug 33080: (follow-up) Minor cleanups Whilst reviewing I spotted a few calls to the stash that were no longer required and errors in the POD for the new/updated methods This patch adds fixes for those minor issues. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147736|0 |1 is obsolete| | --- Comment #18 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 147790 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147790&action=edit Bug 33080: Add small warpper helper to call add_pagination_headers This patch introduces a small helper that uses the stashed values in `search_rs` to properly build the pagination headers based on the resultset. To test: 1. Run all the API tests and make sure nothing breaks 2. Run: $ ktd --shell $ t/db_dependent/Koha/REST/Plugin/Pagination.t => SUCCESS: Tests pass! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmm.. well I liked where the branch was going.. but I wasn't aware of the additional calls we're already making to add_pagination_headers as yet so wasn't aware it was a bit of a faff. However, now I've just reviewed this final patch it feels like a few bits have been missed from the commit. The POD calls a helper `objects.total_from_resultset` but then defines it as `objects.generate_pagination_headers` and inside that new helper we call the non-existing `total_from_resultset`.. that feels like it should just be $result_set->count anyway.. but I'm also not entirely clear on the naming of `add_pagination_headers` and `generate_pagination_headers`.. that sort of feel like synonyms.. I could see it getting confusing down the line. Would you like me to step in and lend a hand here Tomas? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147788|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147789|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147790|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 147823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147823&action=edit Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 147824 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147824&action=edit Bug 33080: Allow pagination to be built from stashed values The way the old `objects.search` was build implied several totals were calculated there, and passed to the `$c->add_pagination_headers` helper. With the introduction of `objects.search_rs` and the ability of doing things to the resultset afterwards, it felt like out of place to have the pagination headers be implicitly set inside `objects.search_rs`. This patch makes the search_rs stash some required values (from the original request) and makes `add_pagination_headers` accept those values. This way other callers can still build their own pagination values, while allowing this simplified implementation. Full-stack tests still pass, and the helper tests are moved to the `db_dependent` section and are now more meaningful as well To test: 1. Apply this patch 2. Run: $ ktd --shell $ prove t/db_dependent/Koha/REST/Plugin/Pagination.t \ t/db_dependent/api/v1/ => SUCCESS: Tests pass! Nothing broken! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #22 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 147825 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=147825&action=edit Bug 33080: Adapt other use-cases This patch adapts the other controllers that rely on add_pagination_headers outside of `objects.search`. To test: 1. Apply this patchset 2. Run: $ ktd --shell $ prove t/db_dependent/api/v1/* \ t/db_dependent/Koha/REST/Plugin/Objects.t \ t/db_dependent/Koha/REST/Plugin/Pagination.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|agustinmoyano@theke.io |tomascohen@gmail.com CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- URL|https://gitlab.com/thekesol | |utions/Koha/-/tree/bug_3308 | |0_search_rs | Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147823|0 |1 is obsolete| | --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 148020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148020&action=edit Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147824|0 |1 is obsolete| | --- Comment #24 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 148021 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148021&action=edit Bug 33080: Allow pagination to be built from stashed values The way the old `objects.search` was build implied several totals were calculated there, and passed to the `$c->add_pagination_headers` helper. With the introduction of `objects.search_rs` and the ability of doing things to the resultset afterwards, it felt like out of place to have the pagination headers be implicitly set inside `objects.search_rs`. This patch makes the search_rs stash some required values (from the original request) and makes `add_pagination_headers` accept those values. This way other callers can still build their own pagination values, while allowing this simplified implementation. Full-stack tests still pass, and the helper tests are moved to the `db_dependent` section and are now more meaningful as well To test: 1. Apply this patch 2. Run: $ ktd --shell $ prove t/db_dependent/Koha/REST/Plugin/Pagination.t \ t/db_dependent/api/v1/ => SUCCESS: Tests pass! Nothing broken! 3. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #147825|0 |1 is obsolete| | --- Comment #25 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 148022 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148022&action=edit Bug 33080: Adapt other use-cases This patch adapts the other controllers that rely on add_pagination_headers outside of `objects.search`. To test: 1. Apply this patchset 2. Run: $ ktd --shell $ prove t/db_dependent/api/v1/* \ t/db_dependent/Koha/REST/Plugin/Objects.t \ t/db_dependent/Koha/REST/Plugin/Pagination.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #26 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All working fantastically and feels much cleaner. Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |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=33080 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=33080 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #148020|0 |1 is obsolete| | Attachment #148021|0 |1 is obsolete| | Attachment #148022|0 |1 is obsolete| | --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 148071 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148071&action=edit Bug 33080: Introduce objects.search_rs, objects.find_rs and objects.to_api This patch introduces the mentioned helpers, and reimplements objects.search and objects.find in terms of them. To test: 1. Apply patch 2. restart_all 3. Check that any API that uses objects.search helper is still running (GET /api/v1/items, GET /api/v1/holds, etc) 4. prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=33080 --- Comment #28 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 148072 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148072&action=edit Bug 33080: Allow pagination to be built from stashed values The way the old `objects.search` was build implied several totals were calculated there, and passed to the `$c->add_pagination_headers` helper. With the introduction of `objects.search_rs` and the ability of doing things to the resultset afterwards, it felt like out of place to have the pagination headers be implicitly set inside `objects.search_rs`. This patch makes the search_rs stash some required values (from the original request) and makes `add_pagination_headers` accept those values. This way other callers can still build their own pagination values, while allowing this simplified implementation. Full-stack tests still pass, and the helper tests are moved to the `db_dependent` section and are now more meaningful as well To test: 1. Apply this patch 2. Run: $ ktd --shell $ prove t/db_dependent/Koha/REST/Plugin/Pagination.t \ t/db_dependent/api/v1/ => SUCCESS: Tests pass! Nothing broken! 3. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=33080 --- Comment #29 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 148073 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=148073&action=edit Bug 33080: Adapt other use-cases This patch adapts the other controllers that rely on add_pagination_headers outside of `objects.search`. To test: 1. Apply this patchset 2. Run: $ ktd --shell $ prove t/db_dependent/api/v1/* \ t/db_dependent/Koha/REST/Plugin/Objects.t \ t/db_dependent/Koha/REST/Plugin/Pagination.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #30 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matt.blenkinsop@ptfs-europe | |.com Status|Pushed to master |Needs documenting --- Comment #31 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Enhancement - not backporting to 22.11.x Nice work everyone! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Required by bug 32734 which was backported already.. backporting for 22.11.x will be in 22.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|23.05.00 |23.05.00, 22.11.04 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|REST API |Architecture, internals, | |and plumbing -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED CC| |caroline.cyr-la-rose@inlibr | |o.com --- Comment #33 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- This seems like backend stuff only, nothing to document. Closing, but feel free to reopen with more details if I'm wrong. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33080 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |36483 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36483 [Bug 36483] Calling $object->to_api directly should be avoided -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org