Bug 27002 - Make Koha::Biblio->pickup_locations return a Koha::Libraries resultset
Summary: Make Koha::Biblio->pickup_locations return a Koha::Libraries resultset
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 26963 27021
Blocks: 27015 27062
  Show dependency treegraph
 
Reported: 2020-11-11 18:43 UTC by Tomás Cohen Arazi
Modified: 2021-12-13 21:09 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.07


Attachments
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset (1.93 KB, patch)
2020-11-12 11:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset (1.93 KB, patch)
2020-11-13 16:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27002: Update callers for changed return (3.02 KB, patch)
2020-11-13 16:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27002: (follow-up) Fix empty Koha::Item->pickup_locations (1.85 KB, patch)
2020-11-13 16:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset (1.93 KB, patch)
2020-11-13 19:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27002: Update callers for changed return (3.03 KB, patch)
2020-11-13 19:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset (1.98 KB, patch)
2020-11-14 17:12 UTC, David Nind
Details | Diff | Splinter Review
Bug 27002: Update callers for changed return (3.07 KB, patch)
2020-11-14 17:12 UTC, David Nind
Details | Diff | Splinter Review
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset (2.04 KB, patch)
2020-11-16 11:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 27002: Update callers for changed return (3.13 KB, patch)
2020-11-16 11:23 UTC, Tomás Cohen Arazi
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 2020-11-11 18:43:58 UTC
For consistency and better handling, it should follow the bug 26963 implementation.
Comment 1 Martin Renvoize 2020-11-12 11:29:56 UTC
Created attachment 113548 [details] [review]
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset
Comment 2 Jonathan Druart 2020-11-13 13:21:17 UTC
Can we have this for 20.11 please?
Comment 3 Martin Renvoize 2020-11-13 16:11:04 UTC
Created attachment 113610 [details] [review]
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset
Comment 4 Martin Renvoize 2020-11-13 16:11:07 UTC
Created attachment 113611 [details] [review]
Bug 27002: Update callers for changed return
Comment 5 Martin Renvoize 2020-11-13 16:11:10 UTC
Created attachment 113612 [details] [review]
Bug 27002: (follow-up) Fix empty Koha::Item->pickup_locations

In Koha::Item pickup_locations we were using the Koha::Objects->empty to
return an effectively empty Koha::Libraries object. However, this only
sets the result cache to an empty arrayref and as such only affect
iterators and not subsequent chained queries (like the get_column I call
to get back a DBIx::Class:ResultSetColumn in this patchset). As such,
this patch updates the Koha::Item->pickup_locations method to use an
always empty search ->search(\'0 = 1') to fulfil the same purpose as the
prior call to empty and return an empty set that is available for both
iterating and query chaining.
Comment 6 Martin Renvoize 2020-11-13 16:17:53 UTC
Gotta love unit tests.. that last one was hard to pin down.

Our ->empty method in Koha::Objects is interesting.. as the commit comment says it Only resets the query result cache (which affects the iterating methods) but does NOT set anything for the query itself.. as such it's like having a brand new object when chaining as as such will return the opposite of what you want.. ALL rows rather than NONE when used in a query chain.
Comment 7 Tomás Cohen Arazi 2020-11-13 19:10:50 UTC
Created attachment 113628 [details] [review]
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset
Comment 8 Tomás Cohen Arazi 2020-11-13 19:10:55 UTC
Created attachment 113629 [details] [review]
Bug 27002: Update callers for changed return
Comment 9 Tomás Cohen Arazi 2020-11-13 19:12:29 UTC
I removed the last patch, as I made this dependent on bug 27021, which is the proper solution to the issue.

No QA complaints, ran:

   $ prove t/db_dependent/Koha/Biblio.t \ 
           t/db_dependent/Template/Plugin/Branches.t \
           t/db_dependent/Koha/Item.t \
           t/db_dependent/api/v1/holds.t
Comment 10 Martin Renvoize 2020-11-13 19:26:54 UTC
Great work, thanks Tomas.  Nice to have this fix further up the stack.. I wasn't confident enough in its current callers to do that myself, but trust your judgement there. 😀
Comment 11 Tomás Cohen Arazi 2020-11-14 13:58:47 UTC
Setting NSO so we get a tester.
Comment 12 David Nind 2020-11-14 17:12:50 UTC
Created attachment 113642 [details] [review]
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2020-11-14 17:12:54 UTC
Created attachment 113643 [details] [review]
Bug 27002: Update callers for changed return

Signed-off-by: David Nind <david@davidnind.com>
Comment 14 David Nind 2020-11-14 17:14:43 UTC
Signed off based on running the tests in Comment #9 before and after without any fails is sufficient as a test plan.
Comment 15 Tomás Cohen Arazi 2020-11-16 11:23:04 UTC
Created attachment 113658 [details] [review]
Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2020-11-16 11:23:09 UTC
Created attachment 113659 [details] [review]
Bug 27002: Update callers for changed return

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Jonathan Druart 2020-11-17 11:26:28 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 18 Andrew Fuerste-Henry 2020-12-09 20:15:37 UTC
Enhancement, not backporting to 20.05
Comment 19 Andrew Fuerste-Henry 2020-12-14 14:49:18 UTC
Pushed to 20.05.x for 20.05.07.
Comment 20 Victor Grousset/tuxayo 2020-12-16 14:40:08 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.