[Bug 43448] New: Unreachable code in reserve/request.pl
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 Bug ID: 43448 Summary: Unreachable code in reserve/request.pl Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, lisette@bywatersolutions.com, tomascohen@gmail.com Target Milestone: --- This stems from a fix on bug 35573 - when AllowHoldPolicyOverride is enabled, we never want to fully prevent a hold because there are no pickup locations available. The code current checks AllowHoldPolicyOverride then gates on a coutn of pickup locations or the value of AllowHoldPolicyOverride Since we have already entered a conditional on AllowHoldPolicyOverride it will always be true at the second check, so the 'else' is meaningless I thought at first that the second condition should be removed, but it is there to prevent us from ever hitting the 'else' so we should remove the else and the condition 600 } elsif ( C4::Context->preference('AllowHoldPolicyOverride') ) { 601 602 # If AllowHoldPolicyOverride is set, it should override EVERY restriction, not just branch item rules 603 # with the exception of itemAlreadyOnHold because, you know, the item is already on hold 604 if ( $can_item_be_reserved ne 'itemAlreadyOnHold' ) { 605 606 # Send the pickup locations count to the UI, the pickup locations will be pulled using the API 607 my @pickup_locations = $item_object->pickup_locations( { patron => $patron } )->as_list; 608 $item->{pickup_locations_count} = scalar @pickup_locations; 609 610 if ( @pickup_locations || C4::Context->preference('AllowHoldPolicyOverride') ) { 611 $num_items_available++; 612 $item->{override} = 1; 613 $num_override++; 614 615 my $default_pickup_location; 616 617 ($default_pickup_location) = 618 grep { $_->branchcode eq $default_pickup_branch } @pickup_locations; 619 620 $item->{default_pickup_location} = $default_pickup_location; 621 } else { 622 $item->{available} = 0; 623 $item->{not_holdable} = "no_valid_pickup_location"; 624 } -- 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=43448 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | Patch complexity|--- |Trivial patch 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=43448 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 204732 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204732&action=edit Bug 43448: Remove meaningless conditional We are in a conditional based on AllowHoldPolicyOverride being enabled, a second conditional with an OR on the value will always be true To test: 1 - Do not apply patch 2 - Disable AllowHoldPolicyOverride 3 - Create a biblio B with an item I at library A. 4 - Configure this library A to not be a pickup location 5 - Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6 - Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7 - Enable AllowHoldPolicyOverride 8 - Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9 - Verify you can place a title-level hold and an item-level hold 10 - Apply patch, repeat test plan, verify nothing has changed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=42999, | |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35573 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #204732|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=43448 --- Comment #2 from David Nind <david@davidnind.com> --- Created attachment 204788 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204788&action=edit Bug 43448: Remove meaningless conditional We are in a conditional based on AllowHoldPolicyOverride being enabled, a second conditional with an OR on the value will always be true To test: 1 - Do not apply patch 2 - Disable AllowHoldPolicyOverride 3 - Create a biblio B with an item I at library A. 4 - Configure this library A to not be a pickup location 5 - Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6 - Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7 - Enable AllowHoldPolicyOverride 8 - Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9 - Verify you can place a title-level hold and an item-level hold 10 - Apply patch, repeat test plan, verify nothing has changed Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #3 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Step 3 - Record B: - Programming Perl (262) - Library: Centerville - Item type: Book 2. Step 4: - Library A Centerville - make it not a pickup library: . Administration > Basic parameters > Libraries . Edit Centerville . Pickup location: set to "No" 3. Step 5: - Go to Administration > Patrons and circulation - Search the page for "Default holds and bookings policies by item type" (at the bottom of the page) - Item type: Books - Hold and booking pickup library match: "item's home library" - Click "Add" 4. Step 8: exclamation mark is after the library name to the right (with the text "This pickup location is not allowed according to circulation rules") -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |matt.blenkinsop@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #204788|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=43448 --- Comment #4 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 204841 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=204841&action=edit Bug 43448: Remove meaningless conditional We are in a conditional based on AllowHoldPolicyOverride being enabled, a second conditional with an OR on the value will always be true To test: 1 - Do not apply patch 2 - Disable AllowHoldPolicyOverride 3 - Create a biblio B with an item I at library A. 4 - Configure this library A to not be a pickup location 5 - Add a "Default holds policy by item type" for item I type where "Hold pickup library match" is "item's home library" 6 - Try to place a hold on biblio B You should not be able to place a hold because there is no valid pickup locations 7 - Enable AllowHoldPolicyOverride 8 - Try to place a hold on biblio B You should now see all valid pickup locations in a dropdown list (with an exclamation mark in front of each option) with none selected by default 9 - Verify you can place a title-level hold and an item-level hold 10 - Apply patch, repeat test plan, verify nothing has changed Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43448 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |26.11.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=43448 --- Comment #5 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org