Bugzilla – Attachment 118021 Details for
Bug 8367
How long is a hold waiting for pickup at a more granular level
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 8367: Add holds_pickup_period circulation rule
0001-Bug-8367-Add-holds_pickup_period-circulation-rule.patch (text/plain), 16.77 KB, created by
Emmi Takkinen
on 2021-03-10 09:54:19 UTC
(
hide
)
Description:
Bug 8367: Add holds_pickup_period circulation rule
Filename:
MIME Type:
Creator:
Emmi Takkinen
Created:
2021-03-10 09:54:19 UTC
Size:
16.77 KB
patch
obsolete
>From cc94115f961a5beaa1702543f011e86a84a004fe Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Mon, 20 Apr 2020 16:13:35 +0000 >Subject: [PATCH] Bug 8367: Add holds_pickup_period circulation rule > >So that pickup delay can have a different value per patron category, > item type or branch. > >To test: > >1) Update database, restart services >2) Set ReservesMaxPickUpDelay syspref (if not already set) >3) Edit your circulation rules and set a value under 'Holds pickup >period (day) that is DIFFERENT from ReservesMaxPickUpDelay. Set a few >different numbers for different branches as well. >4) Place a hold on a biblio from the staff client. >5) Check in an item from that biblio and confirm the hold as waiting >6) Confirm the expiration date is calculated using the 'Holds pickup >period' value instead of the ReservesMaxPickUpDelay syspref >7) Revert the waiting status and delete the hold >8) Re-place the hold on the biblio on the OPAC. Notice that when you >change the pick up location, the number of days in the pickup message >below the dropdown changes based on the circ rules. >9) Create a holiday with a date that will overlap with the 'Holds pickup >period' >10) Check in an item from that biblio and confirm the hold as waiting >11) Confirm the expiration date is calculated using the 'Holds pickup >period' value AND considers the special holiday >12) Confirm tests pass t/db_dependent/Holds/WaitingReserves.t >13) Test Talking Tech: >13a) Enable TalkingTechItivaPhoneNotification >13b) Go to Tools -> Notices & slips. Add content to the HOLD phone >(itiva) notice. >13c) In your terminal, run perl >/path/to/koha/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl -o >~/itiva.tmp -w 0 --type=RESERVE > >Sponsored-by: Catalyst IT > >Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi> >--- > Koha/CirculationRules.pm | 3 + > Koha/Hold.pm | 12 ++++ > admin/smart-rules.pl | 2 + > ...8367-add_holds_pickup_period_circrule.perl | 6 ++ > installer/onboarding.pl | 1 + > .../prog/en/modules/admin/smart-rules.tt | 7 +- > .../bootstrap/en/modules/opac-reserve.tt | 19 ++++++ > .../thirdparty/TalkingTech_itiva_outbound.pl | 11 +++ > opac/opac-reserve.pl | 15 ++++ > t/db_dependent/Holds/WaitingReserves.t | 68 ++++++++++++++++++- > 10 files changed, 142 insertions(+), 2 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_8367-add_holds_pickup_period_circrule.perl > >diff --git a/Koha/CirculationRules.pm b/Koha/CirculationRules.pm >index df1314613e..db8aeb39f5 100644 >--- a/Koha/CirculationRules.pm >+++ b/Koha/CirculationRules.pm >@@ -169,6 +169,9 @@ our $RULE_KINDS = { > decreaseloanholds => { > scope => [ 'branchcode', 'categorycode', 'itemtype' ], > }, >+ holds_pickup_period => { >+ scope => [ 'branchcode', 'categorycode', 'itemtype' ], >+ }, > # Not included (deprecated?): > # * accountsent > # * reservecharge >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index 892fb20cd5..00981ae13b 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -196,6 +196,18 @@ sub set_waiting { > my $cancel_on_holidays = C4::Context->preference('ExpireReservesOnHolidays'); > > my $expirationdate = $today->clone; >+ >+ my $rule = Koha::CirculationRules->get_effective_rule({ >+ categorycode => $self->borrower->categorycode, >+ itemtype => $self->item->effective_itemtype, >+ branchcode => $self->branchcode, >+ rule_name => 'holds_pickup_period', >+ }); >+ if ( defined($rule) and $rule->rule_value ne '' ){ >+ # circulation rule overrides ReservesMaxPickUpDelay >+ $max_pickup_delay = $rule->rule_value; >+ } >+ > $expirationdate->add(days => $max_pickup_delay); > > if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) { >diff --git a/admin/smart-rules.pl b/admin/smart-rules.pl >index b1fa57b4da..99bf41c13a 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -290,6 +290,7 @@ elsif ($op eq 'add') { > 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; >+ my $holds_pickup_period = strip_non_numeric($input->param('holds_pickup_period')); > $debug and warn "Adding $br, $bor, $itemtype, $fine, $maxissueqty, $maxonsiteissueqty, $cap_fine_to_replacement_price"; > > my $rules = { >@@ -325,6 +326,7 @@ elsif ($op eq 'add') { > article_requests => $article_requests, > note => $note, > decreaseloanholds => $decreaseloanholds, >+ holds_pickup_period => $holds_pickup_period, > }; > > Koha::CirculationRules->set_rules( >diff --git a/installer/data/mysql/atomicupdate/bug_8367-add_holds_pickup_period_circrule.perl b/installer/data/mysql/atomicupdate/bug_8367-add_holds_pickup_period_circrule.perl >new file mode 100644 >index 0000000000..2f62aa436d >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_8367-add_holds_pickup_period_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, 'holds_pickup_period', NULL) }); >+ >+ NewVersion( $DBversion, 8367, "Add holds_pickup_period circulation rule" ); >+} >diff --git a/installer/onboarding.pl b/installer/onboarding.pl >index 6680cd94d9..48c48e6b51 100755 >--- a/installer/onboarding.pl >+++ b/installer/onboarding.pl >@@ -288,6 +288,7 @@ if ( $step == 5 ) { > reservesallowed => $reservesallowed, > suspension_chargeperiod => undef, > decreaseloanholds => undef, >+ holds_pickup_period => 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 d41e9ecc50..32c6c9e962 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 >@@ -126,6 +126,7 @@ > <th>Holds per record (count)</th> > <th>On shelf holds allowed</th> > <th>OPAC item level holds</th> >+ <th>Holds pickup period (day)</th> > <th>Article requests</th> > <th>Rental discount (%)</th> > <th class="noExport">Actions</th> >@@ -169,8 +170,9 @@ > [% SET article_requests = all_rules.$c.$i.article_requests %] > [% SET rentaldiscount = all_rules.$c.$i.rentaldiscount %] > [% SET decreaseloanholds = all_rules.$c.$i.decreaseloanholds %] >+ [% SET holds_pickup_period = all_rules.$c.$i.holds_pickup_period %] > >- [% 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 || unseenrenewalsallowed || 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 %] >+ [% 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 || unseenrenewalsallowed || 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 || holds_pickup_period %] > [% IF show_rule %] > [% SET row_count = row_count + 1 %] > <tr row_countd="row_[% row_count | html %]"> >@@ -318,6 +320,7 @@ > <span>Don't allow</span> > [% END %] > </td> >+ <td>[% holds_pickup_period | html %]</td> > <td> > [% IF article_requests == 'no' %] > <span>No</span> >@@ -450,6 +453,7 @@ > <option value="F">Force</option> > </select> > </td> >+ <td><input type="text" name="holds_pickup_period" id="holds_pickup_period" size="2" /></td> > <td> > <select id="article_requests" name="article_requests"> > <option value="no">No</option> >@@ -503,6 +507,7 @@ > <th>Holds per record (count)</th> > <th>On shelf holds allowed</th> > <th>OPAC item level holds</th> >+ <th>Holds pickup period (day)</th> > <th>Article requests</th> > <th>Rental discount (%)</th> > <th> </th> >diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >index ec4aaf5b03..4b9992a1b7 100644 >--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-reserve.tt >@@ -271,6 +271,10 @@ > [% END # / UNLESS singleBranchMode %] > </ul> > >+ [% IF pickup_delays %] >+ <p id="pickup_delay" style="display:none;">When your item is ready for pickup, you will have <span id="pickup_delay_days"></span> days to pick it up from this library.</p> >+ [% END %] >+ > <a class="toggle-hold-options" id="toggle-hold-options-[% bibitemloo.biblionumber | html %]" style="display:none;" href="#">Show more options</a> > > <div id="hold-options-[% bibitemloo.biblionumber | html %]" class="hold-options"> >@@ -482,7 +486,22 @@ > $(radios).first().attr("checked", "checked"); > } > >+ function calculate_delay_days(){ >+ var selected_branch = $("#branch_1").val(); >+ [% FOREACH branchcode IN pickup_delays.keys %] >+ var branchcode = "[% branchcode | html %]"; >+ if ( branchcode == selected_branch ) { >+ $("#pickup_delay_days").text([% pickup_delays.$branchcode | html %]); >+ } >+ [% END %] >+ } >+ > $(document).ready(function() { >+ calculate_delay_days(); >+ $("#pickup_delay").show(); >+ $("#branch_1").change(function(){ >+ calculate_delay_days(); >+ }); > $("#hold-request-form").preventDoubleFormSubmit(); > var copiesRowId = null; > var wasSpecific = false; >diff --git a/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl b/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl >index ae6a4f4e00..84fc2e5195 100755 >--- a/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl >+++ b/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl >@@ -344,6 +344,17 @@ sub GetWaitingHolds { > my $calendar = Koha::Calendar->new( branchcode => $issue->{'site'}, days_mode => $daysmode ); > > my $waiting_date = dt_from_string( $issue->{waitingdate}, 'sql' ); >+ my $rule = Koha::CirculationRules->get_effective_rule({ >+ categorycode => $issue->{categorycode}, >+ itemtype => $item->effective_itemtype, >+ branchcode => $issue->{branchcode}, >+ rule_name => 'holds_pickup_period', >+ }); >+ if ( defined($rule) and $rule->rule_value ne '' ){ >+ # circulation rule overrides ReservesMaxPickUpDelay >+ $pickupdelay = $rule->rule_value; >+ } >+ > my $pickup_date = $waiting_date->clone->add( days => $pickupdelay ); > if ( $calendar->is_holiday($pickup_date) ) { > $pickup_date = $calendar->next_open_days( $pickup_date, 1 ); >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 9b7680759d..4d114911e3 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -682,4 +682,19 @@ if ( > ); > } > >+my @branches = Koha::Libraries->search(); >+my %hold_pickup_delay_by_branch = (); >+ >+foreach my $branch ( @branches ) { >+ my $rule = Koha::CirculationRules->get_effective_rule({ >+ categorycode => $patron->categorycode, >+ branchcode => $branch->branchcode, >+ itemtype => undef, >+ rule_name => 'holds_pickup_period', >+ }); >+ $hold_pickup_delay_by_branch{$branch->branchcode} = $rule->rule_value if ( $rule and $rule->rule_value ); >+} >+ >+$template->param( pickup_delays => \%hold_pickup_delay_by_branch ); >+ > output_html_with_http_headers $query, $cookie, $template->output, undef, { force_no_caching => 1 }; >diff --git a/t/db_dependent/Holds/WaitingReserves.t b/t/db_dependent/Holds/WaitingReserves.t >index 50b6ed8573..2c77e2d4a5 100755 >--- a/t/db_dependent/Holds/WaitingReserves.t >+++ b/t/db_dependent/Holds/WaitingReserves.t >@@ -8,7 +8,7 @@ use Koha::DateUtils; > use t::lib::Mocks; > use t::lib::TestBuilder; > >-use Test::More tests => 11; >+use Test::More tests => 13; > > use_ok('C4::Reserves'); > >@@ -63,11 +63,24 @@ my $biblio = $builder->build_sample_biblio; > my $biblio2 = $builder->build_sample_biblio; > my $biblio3 = $builder->build_sample_biblio; > my $biblio4 = $builder->build_sample_biblio; >+my $biblio5 = $builder->build_sample_biblio; >+my $biblio6 = $builder->build_sample_biblio; > > my $item1 = $builder->build_sample_item({biblionumber => $biblio->biblionumber}); > my $item2 = $builder->build_sample_item({biblionumber => $biblio2->biblionumber}); > my $item3 = $builder->build_sample_item({biblionumber => $biblio3->biblionumber}); > my $item4 = $builder->build_sample_item({biblionumber => $biblio4->biblionumber}); >+my $item5 = $builder->build_sample_item({biblionumber => $biblio5->biblionumber}); >+my $item6 = $builder->build_sample_item({biblionumber => $biblio6->biblionumber}); >+ >+Koha::CirculationRules->set_rules({ >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, >+ rules => { >+ holds_pickup_period => undef, >+ } >+}); > > my $today = dt_from_string(); > >@@ -199,4 +212,57 @@ ModReserveAffect( $item4->itemnumber, $patron2->{borrowernumber}, 0, $reserve4-> > my $r4 = Koha::Holds->find($reserve4->{reserve_id}); > is($r4->expirationdate, $requested_expiredate->ymd, 'Requested expiration date should be kept' ); > >+Koha::CirculationRules->set_rules({ >+ categorycode => $patron1->{categorycode}, >+ itemtype => undef, >+ branchcode => undef, >+ rules => { >+ holds_pickup_period => '3', >+ } >+}); >+t::lib::Mocks::mock_preference('ReservesMaxPickUpDelay', 7); >+ >+my $reserve5_reservedate = $today->clone; >+my $reserve5_expirationdate = $reserve5_reservedate->add(days => 3); >+ >+my $reserve5 = $builder->build({ >+ source => 'Reserve', >+ value => { >+ borrowernumber => $patron1->{borrowernumber}, >+ reservedate => $reserve5_reservedate->ymd, >+ expirationdate => undef, >+ biblionumber => $biblio5->biblionumber, >+ branchcode => 'LIB2', >+ priority => 1, >+ found => '', >+ }, >+}); >+ >+ModReserveAffect( $item5->itemnumber, $patron1->{borrowernumber}); >+my $r5 = Koha::Holds->find($reserve5->{reserve_id}); >+ >+is($r5->expirationdate, $reserve5_expirationdate->ymd, 'Expiration date should be set to today + 3 based on circulation rules' ); >+ >+my $reserve6_reservedate = $today->clone; >+# add 3 days of pickup + 1 day of holiday >+my $reserve6_expirationdate = $reserve6_reservedate->add(days => 5); >+ >+my $reserve6 = $builder->build({ >+ source => 'Reserve', >+ value => { >+ borrowernumber => $patron1->{borrowernumber}, >+ reservedate => $reserve6_reservedate->ymd, >+ expirationdate => undef, >+ biblionumber => $biblio6->biblionumber, >+ branchcode => 'LIB1', >+ priority => 1, >+ found => '', >+ }, >+}); >+ >+ModReserveAffect( $item6->itemnumber, $patron1->{borrowernumber}); >+my $r6 = Koha::Holds->find($reserve6->{reserve_id}); >+ >+is($r6->expirationdate, $reserve6_expirationdate->ymd, 'Expiration date should be set to today + 4 based on circulation rules and including a holiday' ); >+ > $schema->storage->txn_rollback; >-- >2.25.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8367
:
12130
|
13139
|
13602
|
13603
|
13604
|
13624
|
15334
|
15335
|
15369
|
15498
|
15613
|
15658
|
15659
|
19093
|
19094
|
19095
|
19096
|
19104
|
19105
|
19107
|
19108
|
19116
|
19180
|
19181
|
19182
|
19360
|
19361
|
19362
|
20300
|
20301
|
20302
|
20303
|
20304
|
32786
|
32826
|
32884
|
32885
|
32916
|
32930
|
42706
|
42754
|
79790
|
79791
|
79798
|
79799
|
79800
|
79853
|
79854
|
79855
|
79859
|
92412
|
92413
|
92414
|
92415
|
92416
|
92417
|
92418
|
92419
|
92420
|
92421
|
92422
|
92423
|
92424
|
92425
|
92426
|
92427
|
92428
|
92958
|
92959
|
92960
|
92961
|
103425
|
113211
|
117284
|
118017
|
118021
|
124568
|
125087
|
131299
|
131300
|
131301
|
131302
|
131646
|
131647
|
131718
|
131719
|
131720
|
131722
|
131723
|
131724
|
133089
|
133090
|
133091
|
133092
|
133093
|
133094
|
142897
|
142898
|
142899
|
142900
|
142901
|
142902
|
142903
|
144097
|
144098
|
144099
|
144100
|
144101
|
144102
|
144103
|
144146
|
145644
|
145657
|
147798
|
147799
|
147800
|
147801
|
147802
|
147803
|
147804
|
147805
|
147806
|
147807
|
148536
|
148537
|
148538
|
148539
|
148540
|
148541
|
148542
|
148543
|
148544
|
148545
|
150437
|
150438
|
150439
|
153235
|
153237
|
153238
|
153239
|
153240
|
153241
|
153242
|
153243
|
153244
|
153245
|
153246
|
157138
|
157139
|
157140
|
157141
|
157142
|
157143
|
157144
|
157145
|
157146
|
157147
|
157148
|
157149
|
157150
|
157298
|
157343
|
157344
|
157345
|
157346
|
157347
|
157348
|
157349
|
157350
|
157351
|
157352
|
157354
|
157355
|
157356
|
157697
|
157698
|
157699
|
157700
|
157701
|
157702
|
157703
|
157704
|
157705
|
157706
|
157707
|
157708
|
157709
|
157715
|
158023
|
158024
|
158025
|
158026
|
158027
|
158028
|
158029
|
158030
|
158031
|
158032
|
158033
|
158034
|
158035
|
158254
|
158255