|
Description
Tomás Cohen Arazi (tcohen)
2020-11-11 18:43:58 UTC
Created attachment 113548 [details] [review] Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset Can we have this for 20.11 please? Created attachment 113610 [details] [review] Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset Created attachment 113611 [details] [review] Bug 27002: Update callers for changed return 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. 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. Created attachment 113628 [details] [review] Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset Created attachment 113629 [details] [review] Bug 27002: Update callers for changed return 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 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. [U+1F600] Setting NSO so we get a tester. Created attachment 113642 [details] [review] Bug 27002: Update Koha::Biblio->pickup_locations to return a resultset Signed-off-by: David Nind <david@davidnind.com> Created attachment 113643 [details] [review] Bug 27002: Update callers for changed return Signed-off-by: David Nind <david@davidnind.com> Signed off based on running the tests in Comment #9 before and after without any fails is sufficient as a test plan. 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> 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> Pushed to master for 20.11, thanks to everybody involved! Enhancement, not backporting to 20.05 Pushed to 20.05.x for 20.05.07. Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |