From da8bab0d71d67f859decaae99a86ddafdf6e6bbe Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 21 Apr 2020 12:52:38 -0400 Subject: [PATCH] Bug 25184: Add syspref It's entirely possible that some libraries are relying on the current before for part of their workflow. Do to this possibility, it seems like a good idea to control this behavior via a system preference. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Set TrapHoldsOnOrder to "don't trap" 4) Set an item's notforloan value to -1 5) Place a hold on that item 6) Check in the item 7) Note the item is not trapped for hold 9) Set TrapHoldsOnOrder to "trap" 10) Check in the item 11) Koha should now ask if you'd like to trap the item for the hold! --- C4/Reserves.pm | 5 ++++- installer/data/mysql/atomicupdate/bug_25184.perl | 11 +++++++++++ installer/data/mysql/sysprefs.sql | 1 + .../en/modules/admin/preferences/circulation.pref | 6 ++++++ t/db_dependent/Holds.t | 9 ++++++++- 5 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_25184.perl diff --git a/C4/Reserves.pm b/C4/Reserves.pm index d320d9d880..e591ebe65f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -793,7 +793,9 @@ sub CheckReserves { return unless $itemnumber; # bail if we got nothing. # if item is not for loan it cannot be reserved either..... # except where items.notforloan < 0 : This indicates the item is holdable. - return if $notforloan_per_item or $notforloan_per_itemtype; + + my $dont_trap = C4::Context->preference('TrapHoldsOnOrder') ? ($notforloan_per_item > 0) : ($notforloan_per_item && 1 ); + return if $dont_trap or $notforloan_per_itemtype; # Find this item in the reserves my @reserves = _Findgroupreserve( $bibitem, $biblio, $itemnumber, $lookahead_days, $ignore_borrowers); @@ -803,6 +805,7 @@ sub CheckReserves { # the more important the item.) # $highest is the most important item we've seen so far. my $highest; + if (scalar @reserves) { my $LocalHoldsPriority = C4::Context->preference('LocalHoldsPriority'); my $LocalHoldsPriorityPatronControl = C4::Context->preference('LocalHoldsPriorityPatronControl'); diff --git a/installer/data/mysql/atomicupdate/bug_25184.perl b/installer/data/mysql/atomicupdate/bug_25184.perl new file mode 100644 index 0000000000..af823f47f3 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_25184.perl @@ -0,0 +1,11 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + # $dbh->do( "ALTER TABLE biblio ADD COLUMN badtaste int" ); + $dbh->do(q{ + INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES + ('TrapHoldsOnOrder','1',NULL,'If enabled, Koha will trap holds for on order items ( notforloan < 0 )','YesNo't c) + }); + + # Always end with this (adjust the bug info) + NewVersion( $DBversion, 25184, "Items with a negative notforloan status should not be captured for holds"); +} diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index e39352448c..728b85f4d3 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -652,6 +652,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('TransfersMaxDaysWarning','3',NULL,'Define the days before a transfer is suspected of having a problem','Integer'), ('TransferWhenCancelAllWaitingHolds','0',NULL,'Transfer items when cancelling all waiting holds','YesNo'), ('TranslateNotices','0',NULL, 'Allow notices to be translated','YesNo'), +('TrapHoldsOnOrder','1',NULL,'If enabled, Koha will trap holds for on order items ( notforloan < 0 )','YesNo'), ('UNIMARCAuthorityField100','afrey50 ba0',NULL,'Define the contents of UNIMARC authority control field 100 position 08-35','Textarea'), ('UNIMARCAuthorsFacetsSeparator',', ',NULL,'UNIMARC authors facets separator','short'), ('UNIMARCField100Language','fre',NULL,'UNIMARC field 100 default language','short'), diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref index dbea85baab..dc2e7dae19 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -514,6 +514,12 @@ Circulation: - "
itype: [NEWBK,\"\"]" - "
Note: the word 'NULL' can be used to block renewal on undefined fields, while an empty string \"\" will block on an empty (but defined) field." Checkin policy: + - + - pref: TrapHoldsOnOrder + choices: + yes: Trap + no: "Don't trap" + - items that are not for loan but holdable ( notforloan < 0 ) to fill holds. - - pref: HoldsAutoFill choices: diff --git a/t/db_dependent/Holds.t b/t/db_dependent/Holds.t index 3cc2dc7e0f..688b12f2dd 100755 --- a/t/db_dependent/Holds.t +++ b/t/db_dependent/Holds.t @@ -7,7 +7,7 @@ use t::lib::TestBuilder; use C4::Context; -use Test::More tests => 63; +use Test::More tests => 64; use MARC::Record; use C4::Biblio; @@ -345,6 +345,7 @@ ok( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status} eq 'damaged', ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for damaged item with AllowHoldsOnDamagedItems disabled" ); # Items that are not for loan, but holdable should not be trapped until they are available for loan +t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 0 ); Koha::Items->find($itemnumber)->damaged(0)->notforloan(-1)->store; Koha::Holds->search({ biblionumber => $biblio->id })->delete(); is( CanItemBeReserved( $borrowernumbers[0], $itemnumber)->{status}, 'OK', "Patron can place hold on item that is not for loan but holdable ( notforloan < 0 )" ); @@ -353,9 +354,15 @@ $hold = Koha::Hold->new( borrowernumber => $borrowernumbers[0], itemnumber => $itemnumber, biblionumber => $biblio->biblionumber, + found => undef, + priority => 1, + reservedate => dt_from_string, + branchcode => $branch_1, } )->store(); ok( !defined( ( CheckReserves($itemnumber) )[1] ), "Hold cannot be trapped for item that is not for loan but holdable ( notforloan < 0 )" ); +t::lib::Mocks::mock_preference( 'TrapHoldsOnOrder', 1 ); +ok( defined( ( CheckReserves($itemnumber) )[1] ), "Hold is trapped for item that is not for loan but holdable ( notforloan < 0 )" ); $hold->delete(); # Regression test for bug 9532 -- 2.24.2 (Apple Git-127)