From bb3893720d55100380bf431cf6aea8a044a9bd04 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! Signed-off-by: Martin Renvoize Signed-off-by: Joy Nelson --- 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 56459656b4..75235ee670 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -756,7 +756,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); @@ -766,6 +768,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 b80c0cc14a..c110c7300f 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -638,6 +638,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 31340db9aa..20ddc54570 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 @@ -500,6 +500,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 ffc1ffafbf..563c1a6ac8 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 => 60; +use Test::More tests => 61; use MARC::Record; use C4::Biblio; @@ -322,6 +322,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 )" ); @@ -330,9 +331,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.39.5