Bugzilla – Attachment 80126 Details for
Bug 14364
Allow automatically canceled expired waiting holds to fill the next hold
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold
Bug-14364---Allow-automatically-canceled-expired-w.patch (text/plain), 17.25 KB, created by
Kyle M Hall (khall)
on 2018-10-05 14:54:36 UTC
(
hide
)
Description:
Bug 14364 - Allow automatically canceled expired waiting holds to fill the next hold
Filename:
MIME Type:
Creator:
Kyle M Hall (khall)
Created:
2018-10-05 14:54:36 UTC
Size:
17.25 KB
patch
obsolete
>From 13d79c2497824c59e4d055d4eba28d70c8a5ae16 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatetsolutions.com> >Date: Fri, 2 Feb 2018 08:36:02 -0500 >Subject: [PATCH] Bug 14364 - Allow automatically canceled expired waiting > holds to fill the next hold > >Right now, if a library automatically cancels expired waiting holds, a >librarian must still re-checkin an item to trap the next available hold >for that item. It would be better if the next hold was automatically >trapped and the librarians receive an email notification so they can >make any changes to the item if need be ( hold area, hold slip in item, >etc ). > >Test Plan: > 1) Apply this patch > 2) Run updatedatabase.pl > 3) Create a record with one item > 4) Place two holds on that record > 5) Check in the item and set it to waiting for the first patron > 6) Set ReservesMaxPickUpDelay to 1 > 7) Enable ExpireReservesMaxPickUpDelay > 8) Enable ExpireReservesAutoFill > 9) Set an email address in ExpireReservesAutoFillEmail >10) Modify the holds waitingdate to be in the past >11) Run misc/cronjobs/holds/cancel_expired_holds.pl >12) Note the hold is now waiting for the next patron >12) Note a waiting hold notification email was sent to that patron >13) Note a hold changed notification email was sent to the library >--- > C4/Items.pm | 2 +- > C4/Reserves.pm | 43 +++++- > Koha/Hold.pm | 15 ++ > .../data/mysql/atomicupdate/bug_14364.sql | 13 ++ > .../mysql/en/mandatory/sample_notices.sql | 10 ++ > installer/data/mysql/sysprefs.sql | 2 + > .../admin/preferences/circulation.pref | 10 ++ > t/db_dependent/Holds/ExpireReservesAutoFill.t | 137 ++++++++++++++++++ > 8 files changed, 228 insertions(+), 4 deletions(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_14364.sql > create mode 100755 t/db_dependent/Holds/ExpireReservesAutoFill.t > >diff --git a/C4/Items.pm b/C4/Items.pm >index 3f683750d9..97a8ee6383 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -295,7 +295,7 @@ sub AddItem { > > $item->{'itemnumber'} = $itemnumber; > >- ModZebra( $item->{biblionumber}, "specialUpdate", "biblioserver" ); >+ C4::Biblio::ModZebra( $item->{biblionumber}, "specialUpdate", "biblioserver" ); > > logaction( "CATALOGUING", "ADD", $itemnumber, "item" ) > if C4::Context->preference("CataloguingLog"); >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 9bac1e9190..6d761b1b9c 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -826,6 +826,7 @@ sub CancelExpiredReserves { > if ( $hold->found eq 'W' ) { > $cancel_params->{charge_cancel_fee} = 1; > } >+ $cancel_params->{autofill} = C4::Context->preference('ExpireReservesAutoFill'); > $hold->cancel( $cancel_params ); > } > } >@@ -1015,7 +1016,10 @@ sub ModReserveStatus { > > =head2 ModReserveAffect > >- &ModReserveAffect($itemnumber,$borrowernumber,$diffBranchSend,$reserve_id); >+&ModReserveAffect( >+ $itemnumber, $borrowernumber, $diffBranchSend, >+ $reserve_id, $notify_library >+); > > This function affect an item and a status for a given reserve, either fetched directly > by record_id, or by borrowernumber and itemnumber or biblionumber. If only biblionumber >@@ -1029,7 +1033,7 @@ take care of the waiting status > =cut > > sub ModReserveAffect { >- my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id ) = @_; >+ my ( $itemnumber, $borrowernumber, $transferToDo, $reserve_id, $notify_library ) = @_; > my $dbh = C4::Context->dbh; > > # we want to attach $itemnumber to $borrowernumber, find the biblionumber >@@ -1056,7 +1060,7 @@ sub ModReserveAffect { > $hold->itemnumber($itemnumber); > $hold->set_waiting($transferToDo); > >- _koha_notify_reserve( $hold->reserve_id ) >+ _koha_notify_reserve( $hold->reserve_id, $notify_library ) > if ( !$transferToDo && !$already_on_shelf ); > > _FixPriority( { biblionumber => $biblionumber } ); >@@ -1628,6 +1632,8 @@ The following tables are availalbe witin the notice: > > sub _koha_notify_reserve { > my $reserve_id = shift; >+ my $notify_library = shift; >+ > my $hold = Koha::Holds->find($reserve_id); > my $borrowernumber = $hold->borrowernumber; > >@@ -1694,6 +1700,37 @@ sub _koha_notify_reserve { > &$send_notification('print', 'HOLD'); > } > >+ if ($notify_library) { >+ my $letter = C4::Letters::GetPreparedLetter( >+ module => 'reserves', >+ letter_code => 'HOLD_CHANGED', >+ branchcode => $hold->branchcode, >+ substitute => { today => output_pref( dt_from_string ) }, >+ tables => { >+ 'branches' => $library, >+ 'borrowers' => $patron->unblessed, >+ 'biblio' => $hold->biblionumber, >+ 'biblioitems' => $hold->biblionumber, >+ 'reserves' => $hold->unblessed, >+ 'items' => $hold->itemnumber, >+ }, >+ ); >+ >+ my $email = >+ C4::Context->preference('ExpireReservesAutoFillEmail') >+ || $library->{branchemail} >+ || C4::Context->preference('KohaAdminEmailAddress'); >+ >+ C4::Letters::EnqueueLetter( >+ { >+ letter => $letter, >+ borrowernumber => $borrowernumber, >+ message_transport_type => 'email', >+ from_address => $email, >+ to_address => $email, >+ } >+ ); >+ } > } > > =head2 _ShiftPriorityByDateAndPriority >diff --git a/Koha/Hold.pm b/Koha/Hold.pm >index 23d5d447a5..2854249092 100644 >--- a/Koha/Hold.pm >+++ b/Koha/Hold.pm >@@ -25,6 +25,7 @@ use Data::Dumper qw(Dumper); > > use C4::Context qw(preference); > use C4::Log; >+use C4::Reserves; > > use Koha::DateUtils qw(dt_from_string output_pref); > use Koha::Patrons; >@@ -338,6 +339,9 @@ Cancel a hold: > > sub cancel { > my ( $self, $params ) = @_; >+ >+ my $autofill_next = $params->{autofill} && $self->itemnumber && $self->found && $self->found eq 'W'; >+ > $self->_result->result_source->schema->txn_do( > sub { > $self->cancellationdate(dt_from_string); >@@ -358,6 +362,17 @@ sub cancel { > if C4::Context->preference('HoldsLog'); > } > ); >+ >+ if ($autofill_next) { >+ my ( undef, $next_hold ) = C4::Reserves::CheckReserves( $self->itemnumber ); >+ if ($next_hold) { >+ my $is_transfer = $self->branchcode ne $next_hold->{branchcode}; >+ >+ ModReserveAffect( $self->itemnumber, $self->borrowernumber, $is_transfer, $next_hold->{reserve_id}, $autofill_next ); >+ ModItemTransfer( $self->itemnumber, $self->branchcode, $next_hold->{branchcode} ) if $is_transfer; >+ } >+ } >+ > return $self; > } > >diff --git a/installer/data/mysql/atomicupdate/bug_14364.sql b/installer/data/mysql/atomicupdate/bug_14364.sql >new file mode 100644 >index 0000000000..4f3e21ef9f >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_14364.sql >@@ -0,0 +1,13 @@ >+INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES >+('ExpireReservesAutoFill','0',NULL,'Automatically fill the next hold with a automatically canceled expired waiting hold.','YesNo'), >+('ExpireReservesAutoFillEmail','', NULL,'If ExpireReservesAutoFill and an email is defined here, the email notification for the change in the hold will be sent to this address.','Free'); >+ >+INSERT INTO letter(module,code,branchcode,name,is_html,title,content,message_transport_type) >+VALUES ( 'reserves', 'HOLD_CHANGED', '', 'Canceled Hold Available for Different Patron', '0', 'Canceled Hold Available for Different Patron', 'The patron picking up <<biblio.title>> (<<items.barcode>>) has changed to <<borrowers.firstname>> <<borrowers.surname>> (<<borrowers.cardnumber>>). >+ >+Please update the hold information for this item. >+ >+Title: <<biblio.title>> >+Author: <<biblio.author>> >+Copy: <<items.copynumber>> >+Pickup location: <<branches.branchname>>', 'email'); >diff --git a/installer/data/mysql/en/mandatory/sample_notices.sql b/installer/data/mysql/en/mandatory/sample_notices.sql >index c36458ec26..3ce3a519d1 100644 >--- a/installer/data/mysql/en/mandatory/sample_notices.sql >+++ b/installer/data/mysql/en/mandatory/sample_notices.sql >@@ -181,3 +181,13 @@ INSERT INTO `letter` (`module`, `code`, `branchcode`, `name`, `is_html`, `title` > VALUES > ('circulation', 'ACCOUNT_PAYMENT', '', 'Account payment', 0, 'Account payment', '[%- USE Price -%]\r\nA payment of [% credit.amount * -1 | $Price %] has been applied to your account.\r\n\r\nThis payment affected the following fees:\r\n[%- FOREACH o IN offsets %]\r\nDescription: [% o.debit.description %]\r\nAmount paid: [% o.amount * -1 | $Price %]\r\nAmount remaining: [% o.debit.amountoutstanding | $Price %]\r\n[% END %]', 'email', 'default'), > ('circulation', 'ACCOUNT_WRITEOFF', '', 'Account writeoff', 0, 'Account writeoff', '[%- USE Price -%]\r\nAn account writeoff of [% credit.amount * -1 | $Price %] has been applied to your account.\r\n\r\nThis writeoff affected the following fees:\r\n[%- FOREACH o IN offsets %]\r\nDescription: [% o.debit.description %]\r\nAmount paid: [% o.amount * -1 | $Price %]\r\nAmount remaining: [% o.debit.amountoutstanding | $Price %]\r\n[% END %]', 'email', 'default'); >+ >+INSERT INTO letter(module,code,branchcode,name,is_html,title,content,message_transport_type) >+ VALUES ( 'reserves', 'HOLD_CHANGED', '', 'Canceled Hold Available for Different Patron', '0', 'Canceled Hold Available for Different Patron', 'The patron picking up <<biblio.title>> (<<items.barcode>>) has changed to <<borrowers.firstname>> <<borrowers.surname>> (<<borrowers.cardnumber>>). >+ >+Please update the hold information for this item. >+ >+Title: <<biblio.title>> >+Author: <<biblio.author>> >+Copy: <<items.copynumber>> >+Pickup location: <<branches.branchname>>', 'email'); >diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql >index bc7af05dd8..5c5717d8e1 100644 >--- a/installer/data/mysql/sysprefs.sql >+++ b/installer/data/mysql/sysprefs.sql >@@ -159,6 +159,8 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('EnhancedMessagingPreferences','1','','If ON, allows patrons to select to receive additional messages about items due or nearly due.','YesNo'), > ('EnhancedMessagingPreferencesOPAC', '1', NULL, 'If ON, show patrons messaging setting on the OPAC.', 'YesNo'), > ('expandedSearchOption','0',NULL,'If ON, set advanced search to be expanded by default','YesNo'), >+('ExpireReservesAutoFill','0',NULL,'Automatically fill the next hold with a automatically canceled expired waiting hold.','YesNo'), >+('ExpireReservesAutoFillEmail','', NULL,'If ExpireReservesAutoFill and an email is defined here, the email notification for the change in the hold will be sent to this address.','Free'), > ('ExpireReservesMaxPickUpDelay','0','','Enabling this allows holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay','YesNo'), > ('ExpireReservesMaxPickUpDelayCharge','0',NULL,'If ExpireReservesMaxPickUpDelay is enabled, and this field has a non-zero value, than a borrower whose waiting hold has expired will be charged this amount.','free'), > ('ExpireReservesOnHolidays', '1', NULL, 'If false, reserves at a library will not be canceled on days the library is not open.', 'YesNo'), >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 952eb81405..83b4cecd74 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 >@@ -601,6 +601,16 @@ Circulation: > yes: Allow > no: "Don't allow" > - "holds to expire automatically if they have not been picked by within the time period specified in ReservesMaxPickUpDelay" >+ - >+ - pref: ExpireReservesAutoFill >+ choices: >+ yes: "Do" >+ no: "Don't" >+ - automatically fill the next hold using the item. If enabled, an email notification will be sent to either the email address defined in ExpireReservesAutoFillEmail, the item's holding library's email address, or the email address defined in KohaAdminEmailAddress in that order. >+ - >+ - Send email notification of the new hold filled with a canceled item to >+ - pref: ExpireReservesAutoFillEmail >+ - . If no address is defined here, the email will be sent to either the item's holding library or the email address defined in KohaAdminEmailAddress in that order. > - > - If using ExpireReservesMaxPickUpDelay, charge a borrower who allows his or her waiting hold to expire a fee of > - pref: ExpireReservesMaxPickUpDelayCharge >diff --git a/t/db_dependent/Holds/ExpireReservesAutoFill.t b/t/db_dependent/Holds/ExpireReservesAutoFill.t >new file mode 100755 >index 0000000000..545802033f >--- /dev/null >+++ b/t/db_dependent/Holds/ExpireReservesAutoFill.t >@@ -0,0 +1,137 @@ >+#!/usr/bin/perl >+ >+use Modern::Perl; >+ >+use Test::More tests => 9; >+ >+use t::lib::Mocks; >+use t::lib::TestBuilder; >+ >+use MARC::Record; >+ >+use C4::Context; >+use C4::Biblio; >+use C4::Items; >+use Koha::Database; >+use Koha::Holds; >+ >+BEGIN { >+ use FindBin; >+ use lib $FindBin::Bin; >+ use_ok('C4::Reserves'); >+} >+ >+my $schema = Koha::Database->new->schema; >+$schema->storage->txn_begin; >+ >+my $builder = t::lib::TestBuilder->new(); >+my $dbh = C4::Context->dbh; >+ >+# Create two random branches >+my $library = $builder->build( { source => 'Branch' } ); >+my $branchcode = $library->{branchcode}; >+ >+$dbh->do('DELETE FROM reserves'); >+$dbh->do('DELETE FROM message_queue'); >+ >+# Create a biblio instance for testing >+my ($biblionumber) = create_helper_biblio('DUMMY'); >+ >+# Create item instance for testing. >+my ( $item_bibnum, $item_bibitemnum, $itemnumber ) = >+ AddItem( { homebranch => $branchcode, holdingbranch => $branchcode }, >+ $biblionumber ); >+ >+my $patron_1 = $builder->build( { source => 'Borrower' } ); >+my $patron_2 = $builder->build( { source => 'Borrower' } ); >+my $patron_3 = $builder->build( { source => 'Borrower' } ); >+ >+# Add a hold on the item for each of our patrons >+my $hold_1 = Koha::Hold->new( >+ { >+ priority => 0, >+ borrowernumber => $patron_1->{borrowernumber}, >+ branchcode => $library->{branchcode}, >+ biblionumber => $biblionumber, >+ itemnumber => $itemnumber, >+ found => 'W', >+ reservedate => '1900-01-01', >+ waitingdate => '1900-01-01', >+ expirationdate => '1900-01-01', >+ lowestPriority => 0, >+ suspend => 0, >+ } >+)->store(); >+my $hold_2 = Koha::Hold->new( >+ { >+ priority => 1, >+ borrowernumber => $patron_2->{borrowernumber}, >+ branchcode => $library->{branchcode}, >+ biblionumber => $biblionumber, >+ itemnumber => $itemnumber, >+ reservedate => '1900-01-01', >+ expirationdate => '9999-01-01', >+ lowestPriority => 0, >+ suspend => 0, >+ } >+)->store(); >+my $hold_3 = Koha::Hold->new( >+ { >+ priority => 2, >+ borrowernumber => $patron_2->{borrowernumber}, >+ branchcode => $library->{branchcode}, >+ biblionumber => $biblionumber, >+ itemnumber => $itemnumber, >+ reservedate => '1900-01-01', >+ expirationdate => '9999-01-01', >+ lowestPriority => 0, >+ suspend => 0, >+ } >+)->store(); >+ >+# Test CancelExpiredReserves >+t::lib::Mocks::mock_preference( 'ExpireReservesMaxPickUpDelay', 1 ); >+t::lib::Mocks::mock_preference( 'ReservesMaxPickUpDelay', 1 ); >+t::lib::Mocks::mock_preference( 'ExpireReservesOnHolidays', 1 ); >+t::lib::Mocks::mock_preference( 'ExpireReservesAutoFill', 1 ); >+t::lib::Mocks::mock_preference( 'ExpireReservesAutoFillEmail', >+ 'kyle@example.com' ); >+ >+CancelExpiredReserves(); >+ >+my @holds = Koha::Holds->search( {}, { order_by => 'priority' } ); >+$hold_2 = $holds[0]; >+$hold_3 = $holds[1]; >+ >+is( @holds, 2, 'Found 2 holds' ); >+is( $hold_2->priority, 0, 'Next hold in line now has priority of 0' ); >+is( $hold_2->found, 'W', 'Next hold in line is now set to waiting' ); >+ >+my @messages = $schema->resultset('MessageQueue') >+ ->search( { letter_code => 'HOLD_CHANGED' } ); >+is( @messages, 1, 'Found 1 message in the message queue' ); >+is( $messages[0]->to_address, 'kyle@example.com', 'Message sent to correct email address' ); >+ >+$hold_2->expirationdate('1900-01-01')->store(); >+ >+CancelExpiredReserves(); >+ >+@holds = Koha::Holds->search( {}, { order_by => 'priority' } ); >+$hold_3 = $holds[0]; >+ >+is( @holds, 1, 'Found 1 hold' ); >+is( $hold_3->priority, 0, 'Next hold in line now has priority of 0' ); >+is( $hold_3->found, 'W', 'Next hold in line is now set to waiting' ); >+ >+# Helper method to set up a Biblio. >+sub create_helper_biblio { >+ my $itemtype = shift; >+ my $bib = MARC::Record->new(); >+ my $title = 'Silence in the library'; >+ $bib->append_fields( >+ MARC::Field->new( '100', ' ', ' ', a => 'Moffat, Steven' ), >+ MARC::Field->new( '245', ' ', ' ', a => $title ), >+ MARC::Field->new( '942', ' ', ' ', c => $itemtype ), >+ ); >+ return my ( $b, $t, $bi ) = AddBiblio( $bib, '' ); >+} >-- >2.17.1 (Apple Git-112)
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 14364
:
39998
|
41236
|
45149
|
49699
|
49700
|
49822
|
52310
|
52311
|
52312
|
71148
|
80126
|
103297
|
105015
|
115427
|
115466
|
120494
|
120941
|
120942
|
128757
|
128758
|
128759
|
132019
|
132020
|
132021
|
133812
|
133923
|
133924
|
133925
|
133926
|
133927
|
133928
|
137927
|
137928
|
137929
|
137930
|
137931
|
137932