Bug 20844

Summary: Reset a hold when it is missing after allocation
Product: Koha Reporter: Hugh Rundle <hughr>
Component: Hold requestsAssignee: Aleisha Amohia <aleisha>
Status: Failed QA --- QA Contact: Martin Renvoize <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: aleisha, alexbuckley, andrewfh, black23, dcook, emily.lamancusa, felicity.brown, gmcharlt, katrin.fischer, kebliss, klas.blomberg, koha, martin.renvoize, Niamh.WalkerHeadon, nick, oleonard, sally.healey
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19287
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15679
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21780
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22221
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement gives librarians easier control over waiting holds when items are marked as lost. When an item with a waiting item-level hold on it is marked as lost the librarian is given the option to retain the hold or to cancel it. When a lost item has an allocated waiting bib-level hold on it then if the 'LostBibLevelHoldsRevert' syspref is enabled then the allocated hold is reverted to an unallocated bib-level hold.
Version(s) released in:
Bug Depends on: 9834, 21732, 21754    
Bug Blocks:    
Attachments: Bug 20844 - Revert hold when setting an item to lost
Bug 20844: Revert hold when setting an item to lost
Bug 20844: Fixed error when cancelling hold from cataloguing/additem.pl
Bug 20844: Fixed error when cancelling hold from cataloguing/additem.pl
Bug 20844: Added tests of the RevertWaitingStatus and RevertItemLevelHoldToBibLevelHold subroutines in C4/Reserves.pm
Bug 20844: Revert hold when setting an item to lost
Bug 20844: Revert hold when setting an item to lost
Bug 20844: Revert hold when setting an item to lost
Bug 20844: (follow-up) Fix style of dialogs
Bug 20844: (follow-up) Fix style of dialogs
Bug 20844: Revert hold when setting an item to lost
Bug 20844: (follow-up) Fix style of dialogs
Bug 20844: Revert hold when setting an item to lost
Bug 20844: (follow-up) Fix style of dialogs
Bug 20884: (QA follow-up) Remove Data::Dumper
Bug 20844: Revert or cancel lost holds
Bug 20844: (follow-up) SendLostHoldNotices
Bug 20844: (follow-up) Get working with transfers
Bug 20844: Revert or cancel lost holds
Bug 20844: (follow-up) SendLostHoldNotices
Bug 20844: (follow-up) Get working with transfers
Bug 20844: Revert or cancel lost holds
Bug 20844: (follow-up) SendLostHoldNotices
Bug 20844: (follow-up) Get working with transfers
Bug 20844: Revert or cancel lost holds
Bug 20844: (follow-up) SendLostHoldNotices
Bug 20844: (follow-up) Get working with transfers
Bug 20844: Revert or cancel lost holds
Bug 20844: (follow-up) SendLostHoldNotices
Bug 20844: (follow-up) Get working with transfers
Bug 20844: Revert or cancel lost holds
Bug 20844: (follow-up) SendLostHoldNotices
Bug 20844: (follow-up) Get working with transfers
Bug 20844: (follow-up) Fixes for lists, objects, templates, tests
Bug 20844: Revert or cancel lost holds
Bug 20844: (follow-up) SendLostHoldNotices
Bug 20844: (follow-up) Get working with transfers
Bug 20844: (follow-up) Fixes for lists, objects, templates, tests
Bug 20844: (follow-up) Fix alert boxes for item-level holds

Description Hugh Rundle 2018-05-31 07:07:48 UTC
There currently is no simple way to 'deallocate' a hold if it has gone missing after it is allocated to a patron but before they collect it.

This enhancement would work as follows:

1. item is allocated
2. item cannot be located when patron tries to collect it
3. staff member marks item as Lost
4. if hold was originally a bib-level hold, hold reverts from being specific to the missing item, back to a bib-level hold with priority 1.
5. if hold was originally item-level, staff member gets an alert asking whether to cancel or retain the hold
6. (optionally?) a 'lost hold' notice is queued to be sent to the patron

This bug is related to the original description of https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19287#c32 and also to https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15679

To do it properly, it may require a couple of new settings, something like:

LostBibLevelHoldsRevert
SendLostHoldNotices
Comment 1 Katrin Fischer 2018-05-31 15:23:41 UTC
Hi Hugh, 
so this woudl be to 'reset' a hold that has been waiting already? Does this happen often? It sounds like someone would be stealing from the waiting holds shelf.
Comment 2 Hugh Rundle 2018-05-31 22:50:58 UTC
(In reply to Katrin Fischer from comment #1)
> Hi Hugh, 
> so this woudl be to 'reset' a hold that has been waiting already? Does this
> happen often? It sounds like someone would be stealing from the waiting
> holds shelf.

Hi Katrin

Yes, this is specifically for holds that are waiting already. It could be missing because someone has stolen it (rare, but certainly happens sometimes in libraries that have open shelves for waiting holds), or it may simply have been mislaid by staff between returning it and putting it on a waiting holds shelf (possible in a busy library). It's certainly more common that may initially seem likely.

The other thing I've just realised would need to be either part of this enhancement, or paired with it perhaps as a different bug, is holds that go missing when they're in transit. i.e. the hold 'allocates' to a specific item when returned, but the pickup is at a different library. The notification won't be sent until it is returned at the pickup point, but it might go missing whilst in transit: in that case (which is more common) much the same process is needed, in that there needs to be a simple way to reverse the allocation since that item has gone missing, but there's probably no need to send a notification saying it got lost since there never would have been one saying it was ready to collect.
Comment 3 Alex Buckley 2018-09-23 21:25:34 UTC Comment hidden (obsolete)
Comment 4 Alex Buckley 2018-10-16 00:11:18 UTC
Any chance we can move this one along with testing?
Comment 5 Owen Leonard 2018-10-16 14:53:53 UTC
This seems to be working perfectly except for one thing: I can't cancel a hold from the add item screen. I get an error, Can't call method "cancel" on an undefined value at /home/vagrant/kohaclone/cataloguing/additem.pl line 741.

The patch does need to be fixed up according to QA tool warnings.
Comment 6 Alex Buckley 2018-10-17 21:30:34 UTC Comment hidden (obsolete)
Comment 7 Alex Buckley 2018-10-17 21:31:32 UTC Comment hidden (obsolete)
Comment 8 Alex Buckley 2018-10-17 21:33:44 UTC
Hi Owen

Thanks for testing. I have corrected the commit message in the first patch, and have just attached a second path which corrects the issue you saw trying to cancel a hold on the cataloguing/additem.pl interface. 

This second patch also corrects the qa warning on C4/Circulation.pm

Can you please test again?

Cheers,
Alex
Comment 9 Alex Buckley 2018-10-19 00:39:30 UTC Comment hidden (obsolete)
Comment 10 Alex Buckley 2018-10-19 00:40:00 UTC Comment hidden (obsolete)
Comment 11 Alex Buckley 2018-10-19 00:41:57 UTC
Hi all

I've added unit tests for the two new subroutines I added to C4/Reserves.pm which were RevertWaitingStatus() and RevertItemLevelHoldToBibLevelHold().

Also all three patches pass the QA test tool now.
Comment 12 Michal Denar 2018-10-19 06:25:10 UTC
Hi,

on kohadevbox I get:

kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove t/db_dependent/Holds/RevertWaitingStatus.t
t/db_dependent/Holds/RevertWaitingStatus.t .. DBD::mysql::st execute failed: Unknown column 'originalholdtype' in 'field list' [for Statement "INSERT INTO `reserves` ( `biblionumber`, `borrowernumber`, `branchcode`, `expirationdate`, `found`, `itemnumber`, `itemtype`, `originalholdtype`, `priority`, `reservedate`, `reservenotes`, `waitingdate`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='439', 1='53', 2='YbGPVdI', 3=undef, 4=undef, 5=undef, 6=undef, 7='B', 8=undef, 9='2018-10-19', 10='', 11=undef] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
Can't locate object method "rethrow" via package "DBD::mysql::st execute failed: Unknown column 'originalholdtype' in 'field list' [for Statement "INSERT INTO `reserves` ( `biblionumber`, `borrowernumber`, `branchcode`, `expirationdate`, `found`, `itemnumber`, `itemtype`, `originalholdtype`, `priority`, `reservedate`, `reservenotes`, `waitingdate`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='439', 1='53', 2='YbGPVdI', 3=undef, 4=undef, 5=undef, 6=undef, 7='B', 8=undef, 9='2018-10-19', 10='', 11=undef] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
" (perhaps you forgot to load "DBD::mysql::st execute failed: Unknown column 'originalholdtype' in 'field list' [for Statement "INSERT INTO `reserves` ( `biblionumber`, `borrowernumber`, `branchcode`, `expirationdate`, `found`, `itemnumber`, `itemtype`, `originalholdtype`, `priority`, `reservedate`, `reservenotes`, `waitingdate`) VALUES ( ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ? )" with ParamValues: 0='439', 1='53', 2='YbGPVdI', 3=undef, 4=undef, 5=undef, 6=undef, 7='B', 8=undef, 9='2018-10-19', 10='', 11=undef] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
"?) at /home/vagrant/kohaclone/Koha/Object.pm line 148.
# Looks like your test exited with 11 before it could output anything.
t/db_dependent/Holds/RevertWaitingStatus.t .. Dubious, test returned 11 (wstat 2816, 0xb00)
Failed 12/12 subtests
Comment 13 Alex Buckley 2018-10-24 01:57:13 UTC Comment hidden (obsolete)
Comment 14 Alex Buckley 2018-10-24 02:16:53 UTC
Hi Michael

Thanks for testing. Can you please try testing now using the test plan in the patch I have just attached.
Comment 15 Alex Buckley 2018-10-24 02:17:52 UTC
Apologies that should be 'Hi Michal'
Comment 16 Alex Buckley 2018-10-28 21:54:46 UTC
With the feature freeze coming up very soon are we able to get this moving along with testing?
Comment 17 Alex Buckley 2018-10-29 19:37:28 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2018-10-30 14:38:36 UTC Comment hidden (obsolete)
Comment 19 Owen Leonard 2018-10-30 14:38:39 UTC Comment hidden (obsolete)
Comment 20 Alex Buckley 2018-10-30 21:29:40 UTC Comment hidden (obsolete)
Comment 21 Alex Buckley 2018-10-30 21:38:23 UTC
Thanks Owen for testing my patch I have tested your follow-up and the dialogs look correct so I have signed off on it.
Comment 22 Martin Renvoize 2018-10-31 10:56:14 UTC Comment hidden (obsolete)
Comment 23 Martin Renvoize 2018-10-31 10:56:18 UTC Comment hidden (obsolete)
Comment 24 Martin Renvoize 2018-10-31 10:57:21 UTC
I've split this into two distinct bugs.. one for the enhancement details here and another for the included LostItem change which cleans up transfers as I count that as it's own bug.
Comment 25 Martin Renvoize 2018-10-31 12:15:51 UTC Comment hidden (obsolete)
Comment 26 Martin Renvoize 2018-10-31 12:15:55 UTC Comment hidden (obsolete)
Comment 27 Martin Renvoize 2018-10-31 12:15:59 UTC Comment hidden (obsolete)
Comment 28 Martin Renvoize 2018-10-31 12:17:38 UTC
I'm afraid I'm not confident enough thought has been given to this regarding all the different ways an item can be marked as lost.

/tools/batchMod.pl, circ/pendingreserves.pl and mist/cronjobs/longoverdue.pl all call LostItem and would need some level of handling for the new workflow which require user input.
Comment 29 Alex Buckley 2018-11-01 11:54:16 UTC
Hi Martin

Thanks for your feedback.

I have tested and looked through the code calls in /circ/pendingreserves.pl, /tools/batchMod.pl and misc/cronjobs/longoverdue.pl

I have noticed they are not consistent in their current handling of the reserves on lost items. 

* /circ/pendingreserves.pl - This cancels non-waiting item level reserves when item is marked as lost. Doesn't show allocated (waiting) holds.

* /tools/batchMod.pl - Doesn’t cancel item-level or allocated bib-level reserves.

* /misc/cronjob/longoverdue.pl - Doesn't cancel non-waiting (un-allocated) reserves

It would be good to have a consistent approach throughout Koha for dealing with reserves on lost items. 

A question do you think it would be best to split this patch up so it is only dealing with allocated (waiting) holds. This would alter the behavior of the patches on the bug report which check for reserve type of 'waiting' or 'reserved'. 

Then we would only need to add reserve handling to /tools/batchMod.pl as longoverdue.pl and pendingreserves.pl don't touch waiting reserves. 

Amending longoverdue.pl and pendingreserves.pl could go in another bug report building on this to handle un-allocated holds, just to prevent too much scope creep on this bug report?

Cheers,
Alex
Comment 30 Aleisha Amohia 2019-02-20 23:17:42 UTC
(In reply to Alex Buckley from comment #29)
> 
> It would be good to have a consistent approach throughout Koha for dealing
> with reserves on lost items. 
> 
> A question do you think it would be best to split this patch up so it is
> only dealing with allocated (waiting) holds. This would alter the behavior
> of the patches on the bug report which check for reserve type of 'waiting'
> or 'reserved'. 
> 
> Then we would only need to add reserve handling to /tools/batchMod.pl as
> longoverdue.pl and pendingreserves.pl don't touch waiting reserves. 
> 
> Amending longoverdue.pl and pendingreserves.pl could go in another bug
> report building on this to handle un-allocated holds, just to prevent too
> much scope creep on this bug report?
> 

It would be great to have some feedback on this so we know what the next steps are.
Comment 31 Nick Clemens 2019-06-14 17:53:29 UTC
9834 allows for manual reversion of hold to take original state of hold (item level vs next available) 

This should be adapted to use that code, or maybe that fix satisfies the requirements here?
Comment 32 David Cook 2019-11-08 02:07:47 UTC
Comment on attachment 81710 [details] [review]
Bug 20844: Revert hold when setting an item to lost

Review of attachment 81710 [details] [review]:
-----------------------------------------------------------------

::: C4/Circulation.pm
@@ +3688,5 @@
> +    my $LostItemLevelHold;
> +    if ( $restype eq "Waiting" || $restype eq "Reserved" ) {
> +        if ( $res->{originalholdtype} eq "B" ) {
> +            #originalholdtype equalling "B" means this is a bib level hold
> +            C4::Reserves::RevertWaitingStatus({itemnumber => $itemnumber});

This line appears to be buggy. I haven't fully investigated, but if the hold type is B, the itemnumber can't be used to look up the reserve, and eventually causes a fatal error in _FixPriority (due to a lack of error handling in RevertWaitingStatus, _FixPriority, etc.)
Comment 33 David Cook 2019-11-08 02:11:32 UTC
Ran into this problem as it was causing /usr/share/koha/bin/cronjobs/longoverdue.pl to seemingly randomly die when this patch is applied.
Comment 34 Aleisha Amohia 2020-04-01 04:04:16 UTC
The code has moved on a bit since these patches were originally written. Will see if I can revive.
Comment 35 Aleisha Amohia 2020-04-03 01:47:49 UTC Comment hidden (obsolete)
Comment 36 Aleisha Amohia 2020-04-03 03:49:09 UTC Comment hidden (obsolete)
Comment 37 Aleisha Amohia 2020-04-03 04:03:53 UTC Comment hidden (obsolete)
Comment 38 Sally 2020-06-23 15:23:44 UTC
Patch doesn't apply.
Comment 39 Aleisha Amohia 2020-11-05 23:16:19 UTC
Created attachment 113208 [details] [review]
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
Comment 40 Aleisha Amohia 2020-11-05 23:16:24 UTC Comment hidden (obsolete)
Comment 41 Aleisha Amohia 2020-11-05 23:16:28 UTC Comment hidden (obsolete)
Comment 42 Aleisha Amohia 2021-03-16 01:49:58 UTC
Created attachment 118264 [details] [review]
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
Comment 43 Aleisha Amohia 2021-03-16 01:50:04 UTC
Created attachment 118265 [details] [review]
Bug 20844: (follow-up) SendLostHoldNotices

This patch adds a new syspref SendLostHoldNotices

Test:
1) Update database and restart memcached
2) Enable the SendLostHoldNotices system preference
3) Place a hold on an item
4) Check in the item and set the hold to waiting
5) Go to edit the item and set an item lost status
6) Check the borrower's notices and confirm the notice has been enqueued
7) Confirm the notice is not sent twice when reverting or cancelling the
hold
8) Confirm notice enqueues as expected for a bib-level hold

Sponsored-by: Catalyst IT
Comment 44 Aleisha Amohia 2021-03-16 01:50:09 UTC
Created attachment 118266 [details] [review]
Bug 20844: (follow-up) Get working with transfers

Test:

1) Change branch to Branch B
2) Place a biblio-level hold at Branch B
3) Change branch to Branch A
4) Check in item at Branch A and set waiting and trigger transfer
5) Go to edit item and set item as lost
6) Check borrower's notices tab and confirm the lost_waiting_hold notice
was enqueued
7) Follow test plan again with item-level hold

Sponsored-by: Catalyst IT
Comment 45 Aleisha Amohia 2021-09-29 03:37:21 UTC
Created attachment 125437 [details] [review]
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
Comment 46 Aleisha Amohia 2021-09-29 03:37:27 UTC
Created attachment 125438 [details] [review]
Bug 20844: (follow-up) SendLostHoldNotices

This patch adds a new syspref SendLostHoldNotices

Test:
1) Update database and restart memcached
2) Enable the SendLostHoldNotices system preference
3) Place a hold on an item
4) Check in the item and set the hold to waiting
5) Go to edit the item and set an item lost status
6) Check the borrower's notices and confirm the notice has been enqueued
7) Confirm the notice is not sent twice when reverting or cancelling the
hold
8) Confirm notice enqueues as expected for a bib-level hold

Sponsored-by: Catalyst IT
Comment 47 Aleisha Amohia 2021-09-29 03:37:33 UTC
Created attachment 125439 [details] [review]
Bug 20844: (follow-up) Get working with transfers

Test:

1) Change branch to Branch B
2) Place a biblio-level hold at Branch B
3) Change branch to Branch A
4) Check in item at Branch A and set waiting and trigger transfer
5) Go to edit item and set item as lost
6) Check borrower's notices tab and confirm the lost_waiting_hold notice
was enqueued
7) Follow test plan again with item-level hold

Sponsored-by: Catalyst IT
Comment 48 Andrew Fuerste-Henry 2021-09-29 20:25:15 UTC
On initial testing, this technically works, but is severely hampered by bug 21729 -- when a hold goes to Waiting, the expiration date is set based on ReservesMaxPickUpDelay. When the hold is reverted, that expiration date remains. Without patching 21729, this feature will leave most libraries with a bunch of bib-level holds that expire within a week or so.
Comment 49 Aleisha Amohia 2021-09-29 21:12:40 UTC
Hi Andrew, it seems that bug exists regardless of this new work. I can work on Bug 21729 but waiting on that patch shouldn't hold up the signoff for this enhancement.

Thanks
Comment 50 Andrew Fuerste-Henry 2021-09-30 13:22:36 UTC
(In reply to Aleisha Amohia from comment #49)
> Hi Andrew, it seems that bug exists regardless of this new work. I can work
> on Bug 21729 but waiting on that patch shouldn't hold up the signoff for
> this enhancement.
> 
> Thanks

You're absolutely correct! I didn't add 21729 as a blocker here as it doesn't technically break this feature.
Comment 51 Felicity Brown 2022-08-30 17:25:14 UTC
Does this apply to setting a status to "missing" as well as lost?
Comment 52 Aleisha Amohia 2022-09-05 22:16:05 UTC
Created attachment 140214 [details] [review]
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
Comment 53 Aleisha Amohia 2022-09-05 22:16:11 UTC
Created attachment 140215 [details] [review]
Bug 20844: (follow-up) SendLostHoldNotices

This patch adds a new syspref SendLostHoldNotices

Test:
1) Update database and restart memcached
2) Enable the SendLostHoldNotices system preference
3) Place a hold on an item
4) Check in the item and set the hold to waiting
5) Go to edit the item and set an item lost status
6) Check the borrower's notices and confirm the notice has been enqueued
7) Confirm the notice is not sent twice when reverting or cancelling the
hold
8) Confirm notice enqueues as expected for a bib-level hold

Sponsored-by: Catalyst IT
Comment 54 Aleisha Amohia 2022-09-05 22:16:16 UTC
Created attachment 140216 [details] [review]
Bug 20844: (follow-up) Get working with transfers

Test:

1) Change branch to Branch B
2) Place a biblio-level hold at Branch B
3) Change branch to Branch A
4) Check in item at Branch A and set waiting and trigger transfer
5) Go to edit item and set item as lost
6) Check borrower's notices tab and confirm the lost_waiting_hold notice
was enqueued
7) Follow test plan again with item-level hold

Sponsored-by: Catalyst IT
Comment 55 Aleisha Amohia 2022-09-05 22:25:18 UTC
(In reply to Felicity Brown from comment #51)
> Does this apply to setting a status to "missing" as well as lost?

The short answer is Yes.

The long answer: this uses items.itemlost value in the database to determine if the item is lost. By default, this is usually linked to the LOST authorised value. So, any selected value of LOST (which might be Lost, or Missing, or Long Overdue) will flag items.itemlost in the database as true.
Comment 56 Emily Lamancusa 2023-03-24 13:38:12 UTC
Patch doesn't apply
Merge conflicts in:
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/moredetail.tt
koha-tmpl/intranet-tmpl/prog/en/modules/admin/prefrences/circulation.pref
catalogue/moredetail.pl
Comment 57 Aleisha Amohia 2023-04-28 03:29:55 UTC
Created attachment 150348 [details] [review]
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
Comment 58 Aleisha Amohia 2023-04-28 03:29:59 UTC
Created attachment 150349 [details] [review]
Bug 20844: (follow-up) SendLostHoldNotices

This patch adds a new syspref SendLostHoldNotices

Test:
1) Update database and restart memcached
2) Enable the SendLostHoldNotices system preference
3) Place a hold on an item
4) Check in the item and set the hold to waiting
5) Go to edit the item and set an item lost status
6) Check the borrower's notices and confirm the notice has been enqueued
7) Confirm the notice is not sent twice when reverting or cancelling the
hold
8) Confirm notice enqueues as expected for a bib-level hold

Sponsored-by: Catalyst IT
Comment 59 Aleisha Amohia 2023-04-28 03:30:03 UTC
Created attachment 150350 [details] [review]
Bug 20844: (follow-up) Get working with transfers

Test:

1) Change branch to Branch B
2) Place a biblio-level hold at Branch B
3) Change branch to Branch A
4) Check in item at Branch A and set waiting and trigger transfer
5) Go to edit item and set item as lost
6) Check borrower's notices tab and confirm the lost_waiting_hold notice
was enqueued
7) Follow test plan again with item-level hold

Sponsored-by: Catalyst IT
Comment 60 Emily Lamancusa 2023-05-01 20:34:25 UTC
Hi Aleisha,

I encountered a few errors while trying to test this patchset.

#1
After changing RevertLostBibLevelHolds to "Revert", I get the following error on cataloguing/additem.pl:
The method Koha::Items->holds is not covered by tests!

Trace begun at /kohadevbox/koha/Koha/Objects.pm line 572
Koha::Objects::AUTOLOAD('Koha::Items=HASH(0x564bf159f540)', 'HASH(0x564bf1743f18)') called at /kohadevbox/koha/cataloguing/additem.pl line 757
eval {...} at /kohadevbox/koha/cataloguing/additem.pl line 2

The error persists even if I change RevertLostBibLevelHolds to "Don't Revert" and restart all. (It does not occur before I change the syspref, and does not persist if I reset all.)

---------------------------------------------------------------------------------

#2
I get the following error on moredetail.pl:

Can't call method "holds" on an undefined value at /kohadevbox/koha/catalogue/moredetail.pl line 251

The line in question is:
$item_info->{waiting_holds} = $item->{object}->holds({ found => [ 'W', 'T' ], item_level_hold => { '!=' => 0 } })->count;

I tried replacing "$item->{object}->holds" with "$item->holds" and that seemed to fix it (and had no effect on error #1).

---------------------------------------------------------------------------------

#3
Tests don't pass:
t/db_dependent/Reserves.t .. "my" variable $item masks earlier declaration in same scope at /kohadevbox/koha/C4/Circulation.pm line 4029.
t/db_dependent/Reserves.t .. 24/78 No reserves HOLD_CANCELLATION letter transported by email at /kohadevbox/koha/C4/Letters.pm line 588.
t/db_dependent/Reserves.t .. 73/78     # Looks like you planned 17 tests but ran 13.

#   Failed test 'Waiting item is marked as lost'
#   at t/db_dependent/Reserves.t line 1563.
Can't call method "notice_email_address" on unblessed reference at /kohadevbox/koha/C4/Message.pm line 185.
# Looks like your test exited with 11 just after 73.
t/db_dependent/Reserves.t .. Dubious, test returned 11 (wstat 2816, 0xb00)
Failed 6/78 subtests
Comment 61 Aleisha Amohia 2023-05-01 22:47:25 UTC
Created attachment 150480 [details] [review]
Bug 20844: (follow-up) Fixes for lists, objects, templates, tests
Comment 62 Emily Lamancusa 2023-05-03 16:00:00 UTC
The majority of the test plan works - just a few more bugs to be worked out:

- If RevertLostBibLevelHolds is set to "Revert" and later switched back to "Don't Revert", the holds continue to automatically revert even after the syspref is disabled. And yes, I did double-check that I actually clicked "Save" and that the syspref was still set to "Don't Revert" if I navigated away from the System Preferences page and then back ;)

- When RevertLostBibLevelHolds is disabled, if a waiting item on an item-level hold is marked as lost, the alert asking whether to revert or cancel the hold still appears, but the "Revert" and "Cancel" buttons have no effect.
In combination with the previous bug, the effect is this:
    * If RevertLostBibLevelHolds has never been enabled, the effect just 
      described occurs.
    * If RevertLostBibLevelHolds was enabled and then disabled, the alert 
      appears and the buttons revert/cancel the hold, as would be expected if 
      the pref was still enabled

A thought: the preference is called RevertLostBibLevelHolds, but if I'm understanding correctly it affects the behavior of both bib level and item level holds. It seems to me that it could more accurately be called RevertLostHolds, and maybe add a note to the description explaining that item-level holds will ask whether they should be reverted or cancelled.

Finally, I just want to clarify something from the test plan:
> 41) Confirm that both Items are included in the alert box on page when
> editing items (cataloguing/additem.pl)
As far as I know, additem.pl can only edit one item at a time - is there a way to edit two at once from that page? (When I edited them one at a time they each got an alert as expected, so may not be an issue - just wanted to check.)
Comment 63 Emily Lamancusa 2023-08-11 21:12:17 UTC
I'd love to see this move forward soon! It needs a minor rebase and fixes for the two issues I noticed the last time I tested, but then I think I'll be able to sign off on it.

I was able to fix the first issue locally by making the following change in circulation.pref:

- 690    yes: Revert
- 691    no: "Don't revert"

+ 690    1: Revert
+ 691    0: "Don't revert"

The issue was that the code in most places checked the syspref as follows:
if ( C4::Context->preference('RevertLostBibLevelHolds') )
which evaluates to true for both "yes" and "no". (The default was put into the database as 0, so the odd behavior only occurs after the pref has been changed.)

--------

For the second issue - do moredetail.tt and additem.tt have access to the syspref value in any way? It's not clear to me whether either of the template files is taking the value of the syspref into account when determining whether to display the confirmation alert for handling a lost item-level hold. However, all of the .pl files in the patches perform the actual handling of lost item-level holds inside of a conditional that checks the syspref. I assume the templates should be checking that data somehow as well, so as to display the alert only if the syspref is set to "revert"?
Comment 64 Aleisha Amohia 2023-11-16 21:18:25 UTC
Created attachment 159062 [details] [review]
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
Comment 65 Aleisha Amohia 2023-11-16 21:18:29 UTC
Created attachment 159063 [details] [review]
Bug 20844: (follow-up) SendLostHoldNotices

This patch adds a new syspref SendLostHoldNotices

Test:
1) Update database and restart memcached
2) Enable the SendLostHoldNotices system preference
3) Place a hold on an item
4) Check in the item and set the hold to waiting
5) Go to edit the item and set an item lost status
6) Check the borrower's notices and confirm the notice has been enqueued
7) Confirm the notice is not sent twice when reverting or cancelling the
hold
8) Confirm notice enqueues as expected for a bib-level hold

Sponsored-by: Catalyst IT
Comment 66 Aleisha Amohia 2023-11-16 21:18:34 UTC
Created attachment 159064 [details] [review]
Bug 20844: (follow-up) Get working with transfers

Test:

1) Change branch to Branch B
2) Place a biblio-level hold at Branch B
3) Change branch to Branch A
4) Check in item at Branch A and set waiting and trigger transfer
5) Go to edit item and set item as lost
6) Check borrower's notices tab and confirm the lost_waiting_hold notice
was enqueued
7) Follow test plan again with item-level hold

Sponsored-by: Catalyst IT
Comment 67 Aleisha Amohia 2023-11-16 21:18:38 UTC
Created attachment 159065 [details] [review]
Bug 20844: (follow-up) Fixes for lists, objects, templates, tests
Comment 68 Emily Lamancusa 2023-11-17 19:42:57 UTC
Created attachment 159111 [details] [review]
Bug 20844: (follow-up) Fix alert boxes for item-level holds

Fix styling on the alert in additem.tt
Don't show alert in moredetail page if RevertBibLevelHolds is disabled
Comment 69 Emily Lamancusa 2023-11-17 19:43:27 UTC
Thanks for the fixes!

I added a follow-up with a couple of trivial fixes to the alerts. :)

The notice is sent twice when reverting or cancelling an item-level hold from the prompt (sorry for missing that the last time I tested). Is there a reason LostItem needs to be called a second time when the user responds to the prompt of whether to revert or cancel an item-level hold? Couldn't the prompt cancel/revert the hold directly at that point, and avoid re-doing all the lost item processing (including the second notice)?

Also, tests are failing:
# Subtest: Waiting item is marked as lost
    1..17
    ok 1 - Biblio-level hold placed
not ok 70 - Waiting item is marked as lost
    # Looks like you planned 17 tests but ran 1.

#   Failed test 'Waiting item is marked as lost'
#   at t/db_dependent/Reserves.t line 1546.
Can't call method "effective_itemtype" on an undefined value at /kohadevbox/koha/Koha/Hold.pm line 234.
# Looks like your test exited with 11 just after 70.
Dubious, test returned 11 (wstat 2816, 0xb00)
Failed 9/78 subtests
Comment 70 Emily Lamancusa 2023-11-29 18:41:33 UTC
*** Bug 30208 has been marked as a duplicate of this bug. ***