From 5510ac91e9a3b95398968d8d03c2be12d7b38253 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 20 Apr 2020 14:31:27 +0000 Subject: [PATCH] Bug 14866: Make high holds work with different item types To test: 1) Apply patch, update database, restart services 2) Enable decreaseLoanHighHolds sysprefs if not already active 3) Go to circulation rules and set a value under 'Decrease loan holds (day)' that is DIFFERENT from decreaseLoanHighHoldsDuration 4) Check out an item with holds and confirm that the value from the rule is used instead of the system preference 5) Confirm tests pass before and after the patch t/db_dependent/DecreaseLoanHighHolds.t Sponsored-by: Catalyst IT Signed-off-by: Lisette Scheer --- C4/Circulation.pm | 22 +++++++++++-- Koha/CirculationRules.pm | 3 ++ admin/smart-rules.pl | 2 ++ .../bug_14866-add_decreaseloanholds_circrule.perl | 6 ++++ installer/onboarding.pl | 1 + .../prog/en/modules/admin/smart-rules.tt | 7 +++- t/db_dependent/DecreaseLoanHighHolds.t | 37 ++++++++++++++++++++-- 7 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds_circrule.perl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 3571051e84..e5555a8006 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1307,15 +1307,31 @@ sub checkHighHolds { my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower ); my $decreaseLoanHighHoldsDuration = C4::Context->preference('decreaseLoanHighHoldsDuration'); - - my $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration ); + my $rule = Koha::CirculationRules->get_effective_rule( + { + categorycode => $borrower->{categorycode}, + itemtype => $item_object->effective_itemtype, + branchcode => $branchcode, + rule_name => 'decreaseloanholds', + } + ); + my $reduced_datedue; + my $duration; + if ( defined($rule) && $rule->rule_value ne '' ){ + # overrides decreaseLoanHighHoldsDuration syspref + $duration = $rule->rule_value; + $reduced_datedue = $calendar->addDate( $issuedate, $rule->rule_value ); + } else { + $duration = $decreaseLoanHighHoldsDuration; + $reduced_datedue = $calendar->addDate( $issuedate, $decreaseLoanHighHoldsDuration ); + } $reduced_datedue->set_hour($orig_due->hour); $reduced_datedue->set_minute($orig_due->minute); $reduced_datedue->truncate( to => 'minute' ); if ( DateTime->compare( $reduced_datedue, $orig_due ) == -1 ) { $return_data->{exceeded} = 1; - $return_data->{duration} = $decreaseLoanHighHoldsDuration; + $return_data->{duration} = $duration; $return_data->{due_date} = $reduced_datedue; } } diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm index 87062f541a..ac47278406 100644 --- a/Koha/CirculationRules.pm +++ b/Koha/CirculationRules.pm @@ -160,6 +160,9 @@ our $RULE_KINDS = { note => { # This is not really a rule. Maybe we will want to separate this later. scope => [ 'branchcode', 'categorycode', 'itemtype' ], }, + decreaseloanholds => { + scope => [ 'branchcode', 'categorycode', 'itemtype' ], + }, # Not included (deprecated?): # * accountsent # * reservecharge diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl index 4a4727e5c2..120ce9d7ee 100755 --- a/admin/smart-rules.pl +++ b/admin/smart-rules.pl @@ -287,6 +287,7 @@ elsif ($op eq 'add') { my $overduefinescap = $input->param('overduefinescap') || ''; my $cap_fine_to_replacement_price = ($input->param('cap_fine_to_replacement_price') || '') eq 'on'; my $note = $input->param('note'); + my $decreaseloanholds = $input->param('decreaseloanholds') || undef; $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; my $rules = { @@ -320,6 +321,7 @@ elsif ($op eq 'add') { cap_fine_to_replacement_price => $cap_fine_to_replacement_price, article_requests => $article_requests, note => $note, + decreaseloanholds => $decreaseloanholds, }; Koha::CirculationRules->set_rules( diff --git a/installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds_circrule.perl b/installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds_circrule.perl new file mode 100644 index 0000000000..246f125ba4 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_14866-add_decreaseloanholds_circrule.perl @@ -0,0 +1,6 @@ +$DBversion = 'XXX'; +if( CheckVersion( $DBversion ) ) { + $dbh->do(q{INSERT IGNORE INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value) VALUES (NULL, NULL, NULL, 'decreaseloanholds', NULL) }); + + NewVersion( $DBversion, 14866, "Add decreaseloanholds circulation rule" ); +} diff --git a/installer/onboarding.pl b/installer/onboarding.pl index f3be391f32..61a8fa3191 100755 --- a/installer/onboarding.pl +++ b/installer/onboarding.pl @@ -287,6 +287,7 @@ if ( $step == 5 ) { rentaldiscount => 0, reservesallowed => $reservesallowed, suspension_chargeperiod => undef, + decreaseloanholds => undef, } }; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index b34d02e2fa..3dcfd503cd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -124,6 +124,7 @@ OPAC item level holds Article requests Rental discount (%) + Decrease loan holds (day) Actions @@ -163,8 +164,9 @@ [% SET opacitemholds = all_rules.$c.$i.opacitemholds %] [% SET article_requests = all_rules.$c.$i.article_requests %] [% SET rentaldiscount = all_rules.$c.$i.rentaldiscount %] + [% SET decreaseloanholds = all_rules.$c.$i.decreaseloanholds %] - [% SET show_rule = note || maxissueqty || maxonsiteissueqty || issuelength || daysmode || lengthunit || hardduedate || hardduedatecompare || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalperiod || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || rentaldiscount %] + [% SET show_rule = note || maxissueqty || maxonsiteissueqty || issuelength || daysmode || lengthunit || hardduedate || hardduedatecompare || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalperiod || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || rentaldiscount || decreaseloanholds %] [% IF show_rule %] [% SET row_count = row_count + 1 %] @@ -320,6 +322,7 @@ [% END %] [% rentaldiscount | html %] + [% decreaseloanholds | html %] Edit Delete @@ -445,6 +448,7 @@ + @@ -487,6 +491,7 @@ OPAC item level holds Article requests Rental discount (%) + Decrease loan holds (day)   diff --git a/t/db_dependent/DecreaseLoanHighHolds.t b/t/db_dependent/DecreaseLoanHighHolds.t index 9228f32c93..9e25fafba9 100755 --- a/t/db_dependent/DecreaseLoanHighHolds.t +++ b/t/db_dependent/DecreaseLoanHighHolds.t @@ -30,7 +30,7 @@ use Koha::CirculationRules; use t::lib::TestBuilder; use t::lib::Mocks; -use Test::More tests => 19; +use Test::More tests => 21; my $dbh = C4::Context->dbh; my $schema = Koha::Database->new()->schema(); @@ -109,11 +109,11 @@ Koha::CirculationRules->set_rules( lengthunit => 'days', reservesallowed => '99', holds_per_record => '99', + decreaseloanholds => 0, } } ); - my $orig_due = C4::Circulation::CalcDateDue( dt_from_string(), $item->effective_itemtype, @@ -133,9 +133,27 @@ my $patron_hr = { borrowernumber => $patron->id, branchcode => $library->{branch my $data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); is( $data->{exceeded}, 1, "Static mode should exceed threshold" ); is( $data->{outstanding}, 6, "Should have 6 outstanding holds" ); -is( $data->{duration}, 1, "Should have duration of 1" ); +is( $data->{duration}, 0, "Should have duration of 0 because of specific circulation rules" ); is( ref( $data->{due_date} ), 'DateTime', "due_date should be a DateTime object" ); +Koha::CirculationRules->set_rules( + { + branchcode => undef, + categorycode => undef, + itemtype => $item->itype, + rules => { + issuelength => '14', + lengthunit => 'days', + reservesallowed => '99', + holds_per_record => '99', + decreaseloanholds => undef, + } + } +); + +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{duration}, 1, "Should have a duration of 1 because no specific circulation rules so defaults to system preference" ); + my $duedate = $data->{due_date}; is($duedate->hour, $orig_due->hour, 'New due hour is equal to original due hour.'); is($duedate->min, $orig_due->min, 'New due minute is equal to original due minute.'); @@ -215,4 +233,17 @@ ok( $needsconfirmation->{HIGHHOLDS}, "High holds checkout needs confirmation" ); ( undef, $needsconfirmation ) = CanBookBeIssued( $patron_object, $item->barcode, undef, undef, undef, { override_high_holds => 1 } ); ok( !$needsconfirmation->{HIGHHOLDS}, "High holds checkout does not need confirmation" ); +Koha::CirculationRules->set_rule( + { + branchcode => undef, + categorycode => undef, + itemtype => $item->itype, + rule_name => 'decreaseloanholds', + rule_value => 2, + } +); + +$data = C4::Circulation::checkHighHolds( $item_hr, $patron_hr ); +is( $data->{duration}, 2, "Circulation rules override system preferences" ); + $schema->storage->txn_rollback(); -- 2.11.0