Bugzilla – Attachment 103425 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
Bug-8367-Add-holdspickupperiod-circulation-rule.patch (text/plain), 13.84 KB, created by
Aleisha Amohia
on 2020-04-21 22:40:18 UTC
(
hide
)
Description:
Bug 8367: Add holds_pickup_period circulation rule
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2020-04-21 22:40:18 UTC
Size:
13.84 KB
patch
obsolete
>From 3da454727223fe1d283bf675e941f030194d9a7b 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 >4) Place a hold on a biblio >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 >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 > >I've also added this to the TalkingTech_itiva_outbound.pl cronjob, I'm >not sure how to test this. Please test properly if you can, otherwise >maybe put some warns in and confirm the correct expiration date is >calculated. > >Sponsored-by: Catalyst IT >--- > Koha/CirculationRules.pm | 3 + > Koha/Hold.pm | 12 +++ > admin/smart-rules.pl | 2 + > .../bug_8367-add_holds_pickup_period_circrule.perl | 6 ++ > installer/onboarding.pl | 1 + > .../prog/en/modules/admin/smart-rules.tt | 7 +- > .../thirdparty/TalkingTech_itiva_outbound.pl | 12 +++ > t/db_dependent/Holds/WaitingReserves.t | 90 +++++++++++++++++++++- > 8 files changed, 131 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 5042e6f55d3..cb84554b012 100644 >--- a/Koha/CirculationRules.pm >+++ b/Koha/CirculationRules.pm >@@ -157,6 +157,9 @@ our $RULE_KINDS = { > note => { # This is not really a rule. Maybe we will want to separate this later. > 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 6896f5e94d1..bf1489842da 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -181,6 +181,18 @@ sub set_waiting { > my $calendar = Koha::Calendar->new( branchcode => $self->branchcode ); > > 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 6bea920dd90..ccc6e6fc4e7 100755 >--- a/admin/smart-rules.pl >+++ b/admin/smart-rules.pl >@@ -288,6 +288,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 $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 = { >@@ -320,6 +321,7 @@ elsif ($op eq 'add') { > cap_fine_to_replacement_price => $cap_fine_to_replacement_price, > article_requests => $article_requests, > note => $note, >+ 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 00000000000..2f62aa436d6 >--- /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 547df29e0da..d98b265a577 100755 >--- a/installer/onboarding.pl >+++ b/installer/onboarding.pl >@@ -283,6 +283,7 @@ if ( $step == 5 ) { > rentaldiscount => 0, > reservesallowed => $reservesallowed, > suspension_chargeperiod => 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 5ae3bb94e1a..8de81cc7f6b 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 >@@ -115,6 +115,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>Actions</th> >@@ -153,10 +154,11 @@ > [% SET holds_per_record = all_rules.$c.$i.holds_per_record %] > [% SET onshelfholds = all_rules.$c.$i.onshelfholds %] > [% SET opacitemholds = all_rules.$c.$i.opacitemholds %] >+ [% SET holds_pickup_period = all_rules.$c.$i.holds_pickup_period %] > [% SET article_requests = all_rules.$c.$i.article_requests %] > [% SET rentaldiscount = all_rules.$c.$i.rentaldiscount %] > >- [% SET show_rule = maxissueqty || maxonsiteissueqty || issuelength || lengthunit || hardduedate || hardduedatebefore || hardduedateexact || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalsallowed || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || article_requests %] >+ [% SET show_rule = maxissueqty || maxonsiteissueqty || issuelength || lengthunit || hardduedate || hardduedatebefore || hardduedateexact || fine || chargeperiod || chargeperiod_charge_at || firstremind || overduefinescap || cap_fine_to_replacement_price || finedays || maxsuspensiondays || suspension_chargeperiod || renewalsallowed || renewalsallowed || norenewalbefore || auto_renew || no_auto_renewal_after || no_auto_renewal_after_hard_limit || reservesallowed || holds_per_day || holds_per_record || onshelfholds || opacitemholds || article_requests || article_requests || holds_pickup_period %] > [% IF show_rule %] > [% SET row_count = row_count + 1 %] > <tr row_countd="row_[% row_count | html %]"> >@@ -289,6 +291,7 @@ > <span>Don't allow</span> > [% END %] > </td> >+ <td>[% holds_pickup_period | html %]</td> > <td> > [% IF article_requests == 'no' %] > <span>No</span> >@@ -395,6 +398,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> >@@ -441,6 +445,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/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl b/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl >index 1131da61e29..f10caaf9dc5 100755 >--- a/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl >+++ b/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl >@@ -335,6 +335,18 @@ sub GetWaitingHolds { > my $calendar = Koha::Calendar->new( branchcode => $issue->{'site'} ); > > my $waiting_date = dt_from_string( $issue->{waitingdate}, 'sql' ); >+ my $item = Koha::Items->find({ barcode => $issue->{barcode} }); >+ 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/t/db_dependent/Holds/WaitingReserves.t b/t/db_dependent/Holds/WaitingReserves.t >index 292061def3b..03be7c7131f 100644 >--- 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'); > >@@ -83,6 +83,18 @@ my $biblio4 = $builder->build({ > title => 'Title 4', }, > }); > >+my $biblio5 = $builder->build({ >+ source => 'Biblio', >+ value => { >+ title => 'Title 5', }, >+}); >+ >+my $biblio6 = $builder->build({ >+ source => 'Biblio', >+ value => { >+ title => 'Title 6', }, >+}); >+ > my $item1 = $builder->build({ > source => 'Item', > value => { >@@ -111,6 +123,29 @@ my $item4 = $builder->build({ > }, > }); > >+my $item5 = $builder->build({ >+ source => 'Item', >+ value => { >+ biblionumber => $biblio5->{biblionumber}, >+ }, >+}); >+ >+my $item6 = $builder->build({ >+ source => 'Item', >+ value => { >+ biblionumber => $biblio6->{biblionumber}, >+ }, >+}); >+ >+Koha::CirculationRules->set_rules({ >+ categorycode => undef, >+ itemtype => undef, >+ branchcode => undef, >+ rules => { >+ holds_pickup_period => undef, >+ } >+}); >+ > my $today = dt_from_string(); > > my $reserve1_reservedate = $today->clone; >@@ -241,4 +276,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.11.0
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