Bugzilla – Attachment 110149 Details for
Bug 14866
Make high holds work with different item types and number of open days
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14866: Make high holds work with different item types
Bug-14866-Make-high-holds-work-with-different-item.patch (text/plain), 12.37 KB, created by
Aleisha Amohia
on 2020-09-16 03:51:02 UTC
(
hide
)
Description:
Bug 14866: Make high holds work with different item types
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2020-09-16 03:51:02 UTC
Size:
12.37 KB
patch
obsolete
>From 820403b7e095c5f706d2c85293145617e2b62618 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >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 >--- > 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 @@ > <th>OPAC item level holds</th> > <th>Article requests</th> > <th>Rental discount (%)</th> >+ <th>Decrease loan holds (day)</th> > <th>Actions</th> > </tr> > </thead> >@@ -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 %] > <tr row_countd="row_[% row_count | html %]"> >@@ -320,6 +322,7 @@ > [% END %] > </td> > <td>[% rentaldiscount | html %]</td> >+ <td>[% decreaseloanholds | html %]</td> > <td class="actions"> > <a href="#" class="editrule btn btn-default btn-xs"><i class="fa fa-pencil"></i> Edit</a> > <a class="btn btn-default btn-xs delete" href="/cgi-bin/koha/admin/smart-rules.pl?op=delete&itemtype=[% i || '*' | uri %]&categorycode=[% c || '*' | uri %]&branch=[% current_branch | uri %]"><i class="fa fa-trash"></i> Delete</a> >@@ -445,6 +448,7 @@ > </select> > </td> > <td><input type="text" name="rentaldiscount" id="rentaldiscount" size="2" /></td> >+ <td><input type="text" name="decreaseloanholds" id="decreaseloanholds" size="2" /></td> > <td class="actions"> > <input type="hidden" name="branch" value="[% current_branch | html %]"/> > <button type="submit" class="btn btn-default btn-xs"><i class="fa fa-save"></i> Save</button> >@@ -487,6 +491,7 @@ > <th>OPAC item level holds</th> > <th>Article requests</th> > <th>Rental discount (%)</th> >+ <th>Decrease loan holds (day)</th> > <th> </th> > </tr> > </tfoot> >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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=110149">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 14866
:
42751
|
58324
|
75861
|
75862
|
75863
|
103323
|
103427
|
110149
|
110224
|
110441
|
111937
|
111938
|
112225
|
112226
|
112936