Bugzilla – Attachment 159062 Details for
Bug 20844
Reset a hold when it is missing after allocation
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20844: Revert or cancel lost holds
Bug-20844-Revert-or-cancel-lost-holds.patch (text/plain), 20.68 KB, created by
Aleisha Amohia
on 2023-11-16 21:18:25 UTC
(
hide
)
Description:
Bug 20844: Revert or cancel lost holds
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2023-11-16 21:18:25 UTC
Size:
20.68 KB
patch
obsolete
>From 51576bf7e4bb40e395fdbe700384e84a11c87d33 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Thu, 26 Mar 2020 14:33:04 +0000 >Subject: [PATCH] Bug 20844: Revert or cancel lost holds > >This patch introduces the RevertLostBibLevelHolds syspref. When an item >is marked as lost, if there is a bib-level hold on it waiting, the hold >is reverted. If there is an item-level hold on it waiting, there is an >alert box asking if the library would like to revert the hold or cancel >it. > >Test: > >1) Update database and restart memcached/plack >2) Place a hold on Biblio A >3) Check in Item A from Biblio A and set the hold as waiting >4) Edit Item A and give it a lost status (952$1, you may need to edit > your MARC frameworks to have this visible in the Editor) >5) Look at your hold. Notice it is still waiting. >6) Go to Administration -> System preferences. Find the >RevertLostBibLevelHolds system preference and enable it. >7) Cancel your hold and remove Item A's lost status. Place another biblio-level hold on the same biblio >8) Check in Item A from Biblio A and set the hold as waiting >9) Go to the Items tab for Biblio A (catalogue/moredetail.pl) >10) Edit Item A and give it a lost status >11) Once the page reloads, go to view your hold. It should no longer be >waiting and have no item allocated. >12) Cancel your hold and remove Item A's lost status. Place an item-level hold on Item A >13) Check in Item A and set the hold as waiting >14) Go to the catalogue detail page for Biblio A (catalogue/detail.pl) and click >'Edit' for Item A (end up on cataloguing/additem.pl) >15) Give Item A a lost status and save changes >16) Once the page reloads, confirm there is an alert box asking you to >revert or cancel the hold. Click Revert >17) Confirm your hold is no longer waiting, but the item is still >allocated >18) Remove Item A's lost status >19) Check in Item A and set the hold as waiting >20) Go to the catalogue detail page for Biblio A (catalogue/detail.pl) and click >'Edit' for Item A (end up on cataloguing/additem.pl) >21) Give Item A a lost status and save changes >22) Once the page reloads, confirm there is an alert box asking you to >revert or cancel the hold. Click Cancel >23) Confirm the hold is now cancelled >24) Remove Item A's lost status >25) Place an item-level hold on Item A >26) Check in Item A and set the hold as waiting >27) Go to the Items tab for Biblio A (catalogue/moredetail.pl) >28) Give Item A a lost status and save changes >29) Once the page reloads, confirm there is an alert box asking you to >revert or cancel the hold. Click Revert >30) Confirm your hold is no longer waiting, but the item is still >allocated >31) Remove Item A's lost status >32) Check in Item A and set the hold as waiting >33) Go to the Items tab for Biblio A (catalogue/moredetail.pl) >34) Give Item A a lost status and save changes >35) Once the page reloads, confirm there is an alert box asking you to >revert or cancel the hold. Click Cancel >36) Confirm the hold is now cancelled >37) Remove Item A's lost status >38) Place an item-level hold on Item A. Check in Item A and set the hold >as waiting >39) Place an item-level hold on Item B (same biblio) for another borrower. Check in >Item B and set the hold as waiting. >40) Give both Items A and B lost statuses. >41) Confirm that both Items are included in the alert box on page when >editing items (cataloguing/additem.pl) >42) Confirm that both Items have individual alert boxes on the Items tab >(catalogue/moredetail.pl) >43) Confirm tests pass t/db_dependent/Reserves.t > >Sponsored-by: Catalyst IT >--- > C4/Circulation.pm | 19 +++ > catalogue/moredetail.pl | 1 + > catalogue/updateitem.pl | 12 ++ > cataloguing/additem.pl | 13 ++ > ...4-add_RevertLostBibLevelHolds_syspref.perl | 6 + > installer/data/mysql/mandatory/sysprefs.sql | 2 + > .../admin/preferences/circulation.pref | 6 + > .../prog/en/modules/catalogue/moredetail.tt | 12 ++ > .../prog/en/modules/cataloguing/additem.tt | 19 +++ > t/db_dependent/Reserves.t | 124 +++++++++++++++++- > 10 files changed, 213 insertions(+), 1 deletion(-) > create mode 100644 installer/data/mysql/atomicupdate/bug_20844-add_RevertLostBibLevelHolds_syspref.perl > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index ac28b2e74b5..7ec9f09cc11 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -4104,6 +4104,25 @@ sub LostItem{ > while (my $transfer = $transfers->next) { > $transfer->cancel({ reason => 'ItemLost', force => 1 }); > } >+ >+ # RevertLostBibLevelHolds >+ my $hold = Koha::Holds->find({ itemnumber => $itemnumber, found => { '!=' => undef } }); >+ if ( C4::Context->preference('RevertLostBibLevelHolds') && defined $hold ){ >+ # syspref is enabled and there is a waiting hold for this item >+ if ( $hold->item_level_hold && defined $params->{cancel_reserve} ){ >+ if ( $params->{cancel_reserve} ){ >+ # cancel item-level hold >+ $hold->cancel; >+ } else { # eq 'revert' >+ # revert item-level hold's waiting status >+ $hold = C4::Reserves::RevertWaitingStatus({ itemnumber => $itemnumber }); >+ } >+ } >+ if ( !$hold->item_level_hold ){ >+ # revert biblio-level hold's waiting status >+ $hold = C4::Reserves::RevertWaitingStatus({ itemnumber => $itemnumber }); >+ } >+ } > } > > sub GetOfflineOperations { >diff --git a/catalogue/moredetail.pl b/catalogue/moredetail.pl >index 5e122011efb..179fe669629 100755 >--- a/catalogue/moredetail.pl >+++ b/catalogue/moredetail.pl >@@ -248,6 +248,7 @@ foreach my $item (@items){ > ); > > $item_info->{nomod} = !$patron->can_edit_items_from( $item->homebranch ); >+ $item_info->{waiting_holds} = $item->{object}->holds({ found => 'W', item_level_hold => { '!=' => 0 } })->count; > > push @item_data, $item_info; > } >diff --git a/catalogue/updateitem.pl b/catalogue/updateitem.pl >index 6987cacbd05..2638d1f2b4d 100755 >--- a/catalogue/updateitem.pl >+++ b/catalogue/updateitem.pl >@@ -40,6 +40,18 @@ my $withdrawn=$cgi->param('withdrawn'); > my $damaged=$cgi->param('damaged'); > my $exclude_from_local_holds_priority = $cgi->param('exclude_from_local_holds_priority'); > my $bookable = $cgi->param('bookable'); >+my $cancelhold=$cgi->param('cancelhold'); >+my $reverthold=$cgi->param('reverthold'); >+my $redirecturl=$cgi->param('redirecturl'); >+if ( defined $cancelhold ){ >+ LostItem($itemnumber, 'moredetail', 0, { cancel_reserve => $cancelhold }); >+} elsif ( defined $reverthold ){ >+ $cancelhold = 0; >+ LostItem($itemnumber, 'moredetail', 0, { cancel_reserve => $cancelhold }); >+} >+if ( defined $redirecturl ){ >+ print $cgi->redirect( $redirecturl . $biblionumber ); >+} > > my $confirm=$cgi->param('confirm'); > my $dbh = C4::Context->dbh; >diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl >index bd7b2971098..2b9b62739a2 100755 >--- a/cataloguing/additem.pl >+++ b/cataloguing/additem.pl >@@ -758,6 +758,19 @@ if( my $default_location = C4::Context->preference('NewItemsDefaultLocation') ) > my @ig = Koha::Biblio::ItemGroups->search({ biblio_id => $biblionumber })->as_list(); > #sort by display order > my @sorted_ig = sort { $a->display_order <=> $b->display_order } @ig; >+ >+if ( C4::Context->preference('RevertLostBibLevelHolds') ){ >+ my @lostitems = Koha::Items->search({ biblionumber => $biblionumber, itemlost => { '!=' => 0 } }); >+ # find if any of these lost items have waiting holds >+ my @waitinglostitems; >+ foreach my $lostitem ( @lostitems ){ >+ if ( $lostitem->holds({ found => 'W', item_level_hold => 1 })->count > 0 ){ >+ push( @waitinglostitems, $lostitem ); >+ } >+ } >+ $template->param( waitinglostitems => \@waitinglostitems ); >+} >+ > # what's the next op ? it's what we are not in : an add if we're editing, otherwise, and edit. > $template->param( > biblio => $biblio, >diff --git a/installer/data/mysql/atomicupdate/bug_20844-add_RevertLostBibLevelHolds_syspref.perl b/installer/data/mysql/atomicupdate/bug_20844-add_RevertLostBibLevelHolds_syspref.perl >new file mode 100644 >index 00000000000..93b91b018f1 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_20844-add_RevertLostBibLevelHolds_syspref.perl >@@ -0,0 +1,6 @@ >+$DBversion = 'XXX'; >+if( CheckVersion( $DBversion ) ) { >+ $dbh->do(q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('RevertLostBibLevelHolds', '0', NULL, 'If an item is marked as lost after being allocated for a biblio-level hold, revert the hold from its waiting status', 'YesNo') }); >+ >+ NewVersion( $DBversion, 20844, "Add RevertLostBibLevelHolds system preference"); >+} >diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql >index b58114d802e..b4cb5abfa23 100644 >--- a/installer/data/mysql/mandatory/sysprefs.sql >+++ b/installer/data/mysql/mandatory/sysprefs.sql >@@ -649,6 +649,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('ReturnLog','1',NULL,'If ON, enables the circulation (returns) log','YesNo'), > ('ReturnpathDefault','',NULL,'Use this email address as return path or bounce address for undeliverable emails','Free'), > ('OPACComments','1','','If ON, enables patron reviews of bibliographic records in the OPAC','YesNo'), >+('RevertLostBibLevelHolds', '0', NULL, 'If an item is marked as lost after being allocated for a biblio-level hold, revert the hold from its waiting status', 'YesNo'), > ('RisExportAdditionalFields', '', NULL , 'Define additional RIS tags to export from MARC records in YAML format as an associative array with either a marc tag/subfield combination as the value, or a list of tag/subfield combinations.', 'textarea'), > ('RoundFinesAtPayment','0', NULL,'If enabled any fines with fractions of a cent will be rounded to the nearest cent when payments are coll ected. e.g. 1.004 will be paid off by a 1.00 payment','YesNo'), > ('RoutingListAddReserves','0','','If ON the patrons on routing lists are automatically added to holds on the issue.','YesNo'), >@@ -673,6 +674,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('SelfCheckReceiptPrompt','1','NULL','If ON, print receipt dialog pops up when self checkout is finished','YesNo'), > ('SelfCheckTimeout','120','','Define the number of seconds before the Web-based Self Checkout times out a patron','Integer'), > ('SendAllEmailsTo','',NULL,'All emails will be redirected to this email if it is not empty','free'), >+('SendLostHoldNotices', '0', NULL, 'Send a notice to a borrower if their reserved and waiting item is marked as lost', 'YesNo'), > ('SeparateHoldings','0',NULL,'Separate current branch holdings from other holdings','YesNo'), > ('SeparateHoldingsBranch','homebranch','homebranch|holdingbranch','Branch used to separate holdings','Choice'), > ('SerialsSearchResultsLimit', NULL, NULL, 'Serials search results limit', 'integer'), >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 40f2d65a81e..10a64594fcc 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 >@@ -682,6 +682,12 @@ Circulation: > 0: "Don't store" > - 'the last patron to return an item. This setting is independent of the <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=opacreadinghistory">opacreadinghistory</a> and <a href="/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=AnonymousPatron">AnonymousPatron</a> system preferences.' > Holds policy: >+ - >+ - pref: RevertLostBibLevelHolds >+ choices: >+ 1: Revert >+ 0: "Don't revert" >+ - waiting status of a hold if the allocated item is marked as Lost. > - > - In the staff interface, split the holds queue into separate tables by > - pref: HoldsSplitQueue >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >index c24aa47c8ae..7f3c11b755a 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt >@@ -119,6 +119,18 @@ > Barcode [% ITEM_DAT.barcode | html %] [% IF not_for_loan %][% not_for_loan_description | html %] [% END %] > </h3> > >+ [% IF ITEM_DAT.itemlost and ITEM_DAT.waiting_holds > 0 %] >+ <div class="dialog alert"> >+ This item has a waiting item-level hold on it, and has been marked as lost.<br> >+ <form action="updateitem.pl" method="post" class="inline"> >+ <input type="hidden" name="itemnumber" value="[% ITEM_DAT.itemnumber %]"> >+ <input type="hidden" name="biblionumber" value="[% ITEM_DAT.biblionumber %]"> >+ <button type="submit" name="reverthold" value="1" class="btn btn-default"><i class="fa fa-undo"></i> Revert waiting status</button> >+ <button type="submit" name="cancelhold" value="1" class="btn btn-default"><i class="fa fa-times"></i> Cancel hold</button> >+ </form> >+ </div> >+ [% END %] >+ > <h4> > Item information > [% UNLESS ( ITEM_DAT.nomod ) %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >index 6fe0ec6cdc3..8c6b6a64a6d 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt >@@ -86,6 +86,25 @@ > [% IF last_item_for_hold %]<div class="dialog alert"><strong>Cannot delete</strong>: Last item for bibliographic record with biblio-level hold on it.</div>[% END %] > [% IF item_not_found %]<div class="dialog alert"><strong>Cannot delete</strong>: Item not found.</div>[% END %] > >+[% IF waitinglostitems %] >+ <div class="alert"> >+ The following item(s) have a waiting item-level hold and have been marked as lost. >+ <ul> >+ [% FOREACH i IN waitinglostitems %] >+ <br> >+ <li>Item (<i>[% i.barcode %]</i>) >+ <form action="../catalogue/updateitem.pl" method="post" class="inline"> >+ <input type="hidden" name="itemnumber" value="[% i.itemnumber %]"> >+ <input type="hidden" name="biblionumber" value="[% i.biblionumber %]"> >+ <input type="hidden" name="redirecturl" value="../cataloguing/additem.pl?biblionumber="> >+ <button type="submit" name="reverthold" value="1" class="btn btn-default"><i class="fa fa-undo"></i> Revert waiting status</button> >+ <button type="submit" name="cancelhold" value="1" class="btn btn-default"><i class="fa fa-times"></i> Cancel hold</button></li> >+ </form> >+ [% END %] >+ </ul> >+ </div> >+[% END %] >+ > <div id="cataloguing_additem_itemlist"> > [% IF items %] > [% SET date_fields = [ 'dateaccessioned', 'onloan', 'datelastseen', 'datelastborrowed', 'replacementpricedate' ] %] >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 0076dc2e15e..176f19ca48b 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 77; >+use Test::More tests => 78; > use Test::MockModule; > use Test::Warn; > >@@ -1405,6 +1405,128 @@ subtest 'ModReserveAffect logging' => sub { > like( $log->info, qr{$expected}, 'Timestamp logged is the current one' ); > }; > >+subtest 'Waiting item is marked as lost' => sub { >+ >+ plan tests => 15; >+ >+ # Set up data >+ my $biblio = $builder->build_sample_biblio(); >+ my $item = $builder->build_sample_item({ biblionumber => $biblio->biblionumber }); >+ my $patron = $builder->build_object({ class => 'Koha::Patrons' }); >+ >+ ## TEST 1: BIBLIO-LEVEL HOLD >+ >+ # place biblio-level hold >+ my $reserve_id = AddReserve({ >+ branchcode => $item->homebranch, >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $biblio->biblionumber, >+ }); >+ my $hold = Koha::Holds->find( $reserve_id ); >+ >+ is( $hold->item_level_hold, 0, 'Biblio-level hold placed' ); >+ >+ # set hold as waiting and assign item >+ $hold->set_waiting; >+ $hold->set({ itemnumber => $item->itemnumber, priority => 0 })->store; >+ >+ # mark item in biblio as lost >+ $item->itemlost( 1 )->store; >+ is( $item->itemlost, 1, 'Item assigned to waiting hold has been marked lost'); >+ >+ # do test >+ t::lib::Mocks::mock_preference( 'RevertLostBibLevelHolds', 0 ); >+ C4::Circulation::LostItem( $item->itemnumber, 'test', 0, { cancel_reserve => 0 }); >+ is( $hold->found, 'W', 'Hold is still waiting even though the item is lost because RevertLostBibLevelHolds is disabled' ); >+ >+ # enable preference >+ t::lib::Mocks::mock_preference( 'RevertLostBibLevelHolds', 1 ); >+ >+ # try again >+ is( $item->itemlost, 1, 'Item assigned to waiting hold has been marked lost'); >+ C4::Circulation::LostItem( $item->itemnumber, 'test', 0, { cancel_reserve => 0 }); >+ $hold = Koha::Holds->find( $reserve_id ); >+ is( $hold->found, undef, 'Hold waiting status has been reverted because RevertLostBibLevelHolds is enabled' ); >+ >+ # clean up >+ $hold->cancel; >+ $item->itemlost( 0 ); >+ >+ ## TEST 2: ITEM-LEVEL HOLD, REVERT >+ >+ # place item-level hold >+ $reserve_id = AddReserve({ >+ branchcode => $item->homebranch, >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $biblio->biblionumber, >+ itemnumber => $item->itemnumber >+ }); >+ $hold = Koha::Holds->find( $reserve_id ); >+ >+ is( $hold->item_level_hold, 1, 'Item-level hold placed' ); >+ >+ # set hold as waiting and assign item >+ $hold->set_waiting; >+ $hold->set({ itemnumber => $item->itemnumber, priority => 0 })->store; >+ >+ # mark item in biblio as lost >+ $item->itemlost( 1 ); >+ is( $item->itemlost, 1, 'Item assigned to waiting hold has been marked lost'); >+ >+ # do test >+ t::lib::Mocks::mock_preference( 'RevertLostBibLevelHolds', 0 ); >+ C4::Circulation::LostItem( $item->itemnumber, 'test', 0, { cancel_reserve => 0 }); >+ is( $hold->found, 'W', 'Hold is still waiting even though the item is lost because RevertLostBibLevelHolds is disabled' ); >+ >+ # enable preference >+ t::lib::Mocks::mock_preference( 'RevertLostBibLevelHolds', 1 ); >+ >+ # try again >+ is( $item->itemlost, 1, 'Item assigned to waiting hold has been marked lost'); >+ C4::Circulation::LostItem( $item->itemnumber, 'test', 0, { cancel_reserve => 0 }); >+ $hold = Koha::Holds->find( $reserve_id ); >+ is( $hold->found, undef, 'Hold waiting status has been reverted because RevertLostBibLevelHolds is enabled, and we chose not to cancel' ); >+ >+ # clean up >+ $hold->cancel; >+ $item->itemlost( 0 ); >+ >+ # TEST 3: ITEM-LEVEL HOLD, CANCEL >+ >+ # place item-level hold >+ $reserve_id = AddReserve({ >+ branchcode => $item->homebranch, >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $biblio->biblionumber, >+ itemnumber => $item->itemnumber >+ }); >+ $hold = Koha::Holds->find( $reserve_id ); >+ >+ is( $hold->item_level_hold, 1, 'Item-level hold placed' ); >+ >+ # set hold as waiting and assign item >+ $hold->set_waiting; >+ $hold->set({ itemnumber => $item->itemnumber, priority => 0 })->store; >+ >+ # mark item in biblio as lost >+ $item->itemlost( 1 ); >+ is( $item->itemlost, 1, 'Item assigned to waiting hold has been marked lost'); >+ >+ # do test >+ t::lib::Mocks::mock_preference( 'RevertLostBibLevelHolds', 0 ); >+ C4::Circulation::LostItem( $item->itemnumber, 'test', 0, { cancel_reserve => 1 }); >+ is( $hold->found, 'W', 'Hold is still waiting even though the item is lost because RevertLostBibLevelHolds is disabled' ); >+ >+ # enable preference >+ t::lib::Mocks::mock_preference( 'RevertLostBibLevelHolds', 1 ); >+ >+ # try again >+ is( $item->itemlost, 1, 'Item assigned to waiting hold has been marked lost'); >+ C4::Circulation::LostItem( $item->itemnumber, 'test', 0, { cancel_reserve => 1 }); >+ $hold = Koha::Holds->find( $reserve_id ); >+ is( Koha::Holds->find( $reserve_id ), undef, 'Hold has been cancelled with RevertLostBibLevelHolds enabled' ); >+}; >+ > sub count_hold_print_messages { > my $message_count = $dbh->selectall_arrayref(q{ > SELECT COUNT(*) >-- >2.30.2
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 20844
:
79267
|
80760
|
80761
|
80890
|
80891
|
81045
|
81531
|
81623
|
81624
|
81640
|
81697
|
81698
|
81710
|
81711
|
81712
|
102334
|
102335
|
102336
|
113208
|
113209
|
113210
|
118264
|
118265
|
118266
|
125437
|
125438
|
125439
|
140214
|
140215
|
140216
|
150348
|
150349
|
150350
|
150480
|
159062
|
159063
|
159064
|
159065
|
159111
|
170263
|
170264