From ca6a3a0218873e50f39c05d89452a187a28d1872 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 11 Nov 2015 15:12:45 +0000 Subject: [PATCH] Bug 13592: Add an option to charge for any hold placed Currently the fee is applied on if all items for the record are issued and at least one hold already exists on the record. This patch does not give a complete answer to the problem (see discussion on bug 13592 for the other user's expectations). It only adds the ability to charge for any hold placed regardless of the conditions. Test plan: 1) Execute the updatedb entry to insert the new pref 2) Confirm that the behavior is the same as before applying this patch 3) Change the HoldFeeMode pref to 'always' 4) Note that the fee is applied for any hold placed http://bugs.koha-community.org/show_bug.cgi?id=13952 --- C4/Reserves.pm | 3 ++- .../mysql/atomicupdate/bug_13592_add_HoldFeeMode_pref.sql | 1 + installer/data/mysql/sysprefs.sql | 1 + .../prog/en/modules/admin/preferences/circulation.pref | 6 ++++++ t/db_dependent/Reserves/GetReserveFee.t | 14 ++++++++++---- 5 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_13592_add_HoldFeeMode_pref.sql diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 4b10ff9..3e1dbb9 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -701,7 +701,8 @@ SELECT COUNT(*) FROM reserves WHERE biblionumber=? AND borrowernumber<>? my $dbh = C4::Context->dbh; my ( $fee ) = $dbh->selectrow_array( $borquery, undef, ($borrowernumber) ); - if( $fee && $fee > 0 ) { + my $hold_fee_mode = C4::Context->preference('HoldFeeMode') || 'not_always'; + if( $fee and $fee > 0 and $hold_fee_mode ne 'always' ) { # This is a reconstruction of the old code: # Compare number of items with items issued, and optionally check holds # If not all items are issued and there are no holds: charge no fee diff --git a/installer/data/mysql/atomicupdate/bug_13592_add_HoldFeeMode_pref.sql b/installer/data/mysql/atomicupdate/bug_13592_add_HoldFeeMode_pref.sql new file mode 100644 index 0000000..6fe043c --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_13592_add_HoldFeeMode_pref.sql @@ -0,0 +1 @@ +INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES ('HoldFeeMode','not_always','always|not_always','Set the hold fee mode','Choice'); diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql index 60b776a..60d7194 100644 --- a/installer/data/mysql/sysprefs.sql +++ b/installer/data/mysql/sysprefs.sql @@ -152,6 +152,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('hide_marc','0',NULL,'If ON, disables display of MARC fields, subfield codes & indicators (still shows data)','YesNo'), ('HighlightOwnItemsOnOPAC','0','','If on, and a patron is logged into the OPAC, items from his or her home library will be emphasized and shown first in search results and item details.','YesNo'), ('HighlightOwnItemsOnOPACWhich','PatronBranch','PatronBranch|OpacURLBranch','Decides which branch\'s items to emphasize. If PatronBranch, emphasize the logged in user\'s library\'s items. If OpacURLBranch, highlight the items of the Apache var BRANCHCODE defined in Koha\'s Apache configuration file.','Choice'), +('HoldFeeMode','not_always','always|not_always','Set the hold fee mode','Choice'), ('HoldsToPullStartDate','2',NULL,'Set the default start date for the Holds to pull list to this many days ago','Integer'), ('HomeOrHoldingBranch','holdingbranch','holdingbranch|homebranch','Used by Circulation to determine which branch of an item to check with independent branches on, and by search to determine which branch to choose for availability ','Choice'), ('HTML5MediaEnabled','not','not|opac|staff|both','Show a tab with a HTML5 media player for files catalogued in field 856','Choice'), 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 6a0954d..5ffde68 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 @@ -637,6 +637,12 @@ Circulation: yes: Charge no: "Don't Charge" - the replacement price when a patron loses an item. + - + - pref: ReserveFee + choices: + always: Always + not_always: "Don't always" + - Charge fee when a hold is placed. Self Checkout: - - "Include the following JavaScript on all pages in the web-based self checkout:" diff --git a/t/db_dependent/Reserves/GetReserveFee.t b/t/db_dependent/Reserves/GetReserveFee.t index 2a7a50c..3b477ea 100755 --- a/t/db_dependent/Reserves/GetReserveFee.t +++ b/t/db_dependent/Reserves/GetReserveFee.t @@ -21,9 +21,10 @@ use Modern::Perl; -use Test::More tests => 5; +use Test::More tests => 6; use Test::MockModule; use t::lib::TestBuilder; +use t::lib::Mocks; use C4::Circulation; use C4::Reserves qw|AddReserve|; @@ -48,7 +49,7 @@ $builder->build({ source => 'Category', value => { categorycode => 'XYZ1', - reservefee => 2.5, + reservefee => 2, }, }); my $patron1 = $builder->build({ @@ -96,6 +97,7 @@ is( acctlines( $patron1->{borrowernumber} ), $acc1, 'No fee charged for patron 1 # expect a charge for patron2. C4::Circulation::AddIssue( $patron1, $item1->{barcode}, '2015-12-31', 0, undef, 0, {} ); # the date does not really matter my $acc2 = acctlines( $patron2->{borrowernumber} ); +t::lib::Mocks::mock_preference('HoldFeeMode', 'not_always'); my $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); is( $fee > 0, 1, 'Patron 2 should be charged cf GetReserveFee' ); C4::Reserves::ChargeReserveFee( $patron2->{borrowernumber}, $fee, $biblio->{title} ); @@ -104,12 +106,16 @@ is( acctlines( $patron2->{borrowernumber} ), $acc2 + 1, 'Patron 2 has been charg # If we delete the reserve, there should be no charge $dbh->do( "DELETE FROM reserves WHERE reserve_id=?", undef, ( $res1 ) ); $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); -is( $fee, 0, 'Patron 2 will not be charged now' ); +is( $fee, 0, 'HoldFeeMode=not_always, Patron 2 should not be charged' ); + +t::lib::Mocks::mock_preference('HoldFeeMode', 'always'); +$fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); +is( int($fee), 2, 'HoldFeeMode=always, Patron 2 should be charged' ); # If we delete the second item, there should be a charge $dbh->do( "DELETE FROM items WHERE itemnumber=?", undef, ( $item2->{itemnumber} ) ); $fee = C4::Reserves::GetReserveFee( $patron2->{borrowernumber}, $biblio->{biblionumber} ); -is( $fee > 0, 1, 'Patron 2 should be charged again this time' ); +is( int($fee), 2, 'Patron 2 should be charged again this time' ); # End of tests sub acctlines { #calculate number of accountlines for a patron -- 2.1.0