Created attachment 104601 [details] [review] Bug 25421: Make ->pickup_locations only return an arrayref This patch makes Koha::Item->pickup_locations and Koha::Biblio->pickup_locations always return an arrayref. Test are adjusted to reflect this: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t => SUCCESS: Tests pass! 2. Apply this patch and repeat 1 => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 104602 [details] [review] Bug 25421: Remove use of Koha::Libraries->pickup_locations The current implementation uses Koha::Libraries->pickup_locations which is problematic and due to be removed by bug 24368. This patch makes the trivial change of just searching for libraries that are marked with pickup_location => 1. Calls to Koha::Item->pickup_locations and Koha::Biblio->pickup_locations are as well adapted to the new arrayref return value. To test: 1. Pick a record with only one item 2. Place a biblio-level hold on it 3. Edit the items: remove the item 4. Go to the Holds tab => FAIL: It explodes 5. Apply this patch and restart: $ sudo koha-plack --restart kohadev 6. Go back and go to the holds tab again => SUCCESS: No failure! 7. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 104687 [details] [review] Bug 25421: Make ->pickup_locations only return an arrayref This patch makes Koha::Item->pickup_locations and Koha::Biblio->pickup_locations always return an arrayref. Test are adjusted to reflect this: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t => SUCCESS: Tests pass! 2. Apply this patch and repeat 1 => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> Created attachment 104688 [details] [review] Bug 25421: Remove use of Koha::Libraries->pickup_locations The current implementation uses Koha::Libraries->pickup_locations which is problematic and due to be removed by bug 24368. This patch makes the trivial change of just searching for libraries that are marked with pickup_location => 1. Calls to Koha::Item->pickup_locations and Koha::Biblio->pickup_locations are as well adapted to the new arrayref return value. To test: 1. Pick a record with only one item 2. Place a biblio-level hold on it 3. Edit the items: remove the item 4. Go to the Holds tab => FAIL: It explodes 5. Apply this patch and restart: $ sudo koha-plack --restart kohadev 6. Go back and go to the holds tab again => SUCCESS: No failure! 7. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> No longer critical as it's no longer possible to delete the last item of a bibliographic record that have a biblio-level hold on it. Created attachment 104690 [details] [review] Bug 25421: Make ->pickup_locations only return an arrayref This patch makes Koha::Item->pickup_locations and Koha::Biblio->pickup_locations always return an arrayref. Test are adjusted to reflect this: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Biblio.t t/db_dependent/Koha/Item.t => SUCCESS: Tests pass! 2. Apply this patch and repeat 1 => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 104691 [details] [review] Bug 25421: Remove use of Koha::Libraries->pickup_locations The current implementation uses Koha::Libraries->pickup_locations which is problematic and due to be removed by bug 24368. This patch makes the trivial change of just searching for libraries that are marked with pickup_location => 1. Calls to Koha::Item->pickup_locations and Koha::Biblio->pickup_locations are as well adapted to the new arrayref return value. To test: 1. Pick a record with only one item 2. Place a biblio-level hold on it 3. Edit the items: remove the item 4. Go to the Holds tab => FAIL: It explodes 5. Apply this patch and restart: $ sudo koha-plack --restart kohadev 6. Go back and go to the holds tab again => SUCCESS: No failure! 7. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 104692 [details] [review] Bug 25421: Update POD Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Nice work everyone! Pushed to master for 20.05 This was wrong:
- @libraries = Koha::Libraries->pickup_locations() unless @libraries;
+ @libraries = Koha::Libraries->search( { pickup_location => 1 },
+ { order_by => ['branchname'] } )->as_list
+ unless @libraries;
->pickup_locations does much more than search for the pickup_location flag.
Tests from t/db_dependent/Template/Plugin/Branches.t are failing massively.
(In reply to Jonathan Druart from comment #11) > This was wrong: > > - @libraries = Koha::Libraries->pickup_locations() unless @libraries; > + @libraries = Koha::Libraries->search( { pickup_location => 1 }, > + { order_by => ['branchname'] } )->as_list > + unless @libraries; > > ->pickup_locations does much more than search for the pickup_location flag. Did that, but not from the call in the Branches plugin. > Tests from t/db_dependent/Template/Plugin/Branches.t are failing massively. Weird, I took care of them. I'll re-check I thought I ran the Test plan and prove xt $(find t -name "*.t") Likely I screw it. BTW Is there any way to find or add the master commit we test against? (In reply to Tomás Cohen Arazi from comment #12) > (In reply to Jonathan Druart from comment #11) > > This was wrong: > > > > - @libraries = Koha::Libraries->pickup_locations() unless @libraries; > > + @libraries = Koha::Libraries->search( { pickup_location => 1 }, > > + { order_by => ['branchname'] } )->as_list > > + unless @libraries; > > > > ->pickup_locations does much more than search for the pickup_location flag. > > Did that, but not from the call in the Branches plugin. > > > Tests from t/db_dependent/Template/Plugin/Branches.t are failing massively. > > Weird, I took care of them. I'll re-check I'm sorry for the failing tests. This patch was supposed to be pushed along with bug 24368 which basically rewrites all the tests as well. missing dependencies. not backported to 19.11.x |
In list context we sometimes return an array, or an arrayref: 84 return $libraries->unblessed unless $item or $biblio; 110 return wantarray ? @pickup_locations : \@pickup_locations; It causes the following error: ARRAY(0x55cb36339ec0) at /usr/share/perl5/CGI/Compile.pm line 137. Template process failed: undef error - Not a HASH reference at /kohadevbox/koha/Koha/Template/Plugin/Branches.pm line 129. at /kohadevbox/koha/C4/Templates.pm line 122 C4::Templates::output('C4::Templates=HASH(0x55cb355b7a40)') called at /kohadevbox/koha/reserve/request.pl line 764 When doing: Have 1 bibliographic record with 1 item on it Place 1 biblio level hold Delete the item (from moredetail.pl) Go to the hold list