[Bug 25421] New: Inconsistent return value in Koha::Libraries->pickup_locations
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Bug ID: 25421 Summary: Inconsistent return value in Koha::Libraries->pickup_locations Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: critical Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Depends on: 22284 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 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22284 [Bug 22284] Groups of pickup locations for holds -- 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=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_20_05_candidate -- 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=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24368 -- 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=25421 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |tomascohen@gmail.com -- 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=25421 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 104601 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104601&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 104602 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104602&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24368 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24368 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24368 [Bug 24368] Koha::Libraries->pickup_locations needs refactoring/ratifying -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Inconsistent return value |Make Koha::Item and |in |Koha::Biblio |Koha::Libraries->pickup_loc |->pickup_locations return |ations |an arrayref -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart@bugs.koha-c |y.org |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 didier <didier.gautheron@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104601|0 |1 is obsolete| | --- Comment #3 from didier <didier.gautheron@biblibre.com> --- Created attachment 104687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104687&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 didier <didier.gautheron@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104602|0 |1 is obsolete| | --- Comment #4 from didier <didier.gautheron@biblibre.com> --- Created attachment 104688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104688&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 didier <didier.gautheron@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |didier.gautheron@biblibre.c | |om -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|critical |normal --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Since bug 8132. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104687|0 |1 is obsolete| | Attachment #104688|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104690 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104690&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104691 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104691&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 104692 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104692&action=edit Bug 25421: Update POD Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master CC| |martin.renvoize@ptfs-europe | |.com --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed, | |rel_20_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #13 from didier <didier.gautheron@biblibre.com> --- 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? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25421 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #15 from Joy Nelson <joy@bywatersolutions.com> --- missing dependencies. not backported to 19.11.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org