Bug 35535 - Cancel hold -button does not work in pop-up (Hold found, item is already waiting)
Summary: Cancel hold -button does not work in pop-up (Hold found, item is already wait...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Lucas Gass
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords: rel_23_05_candidate
: 36050 (view as bug list)
Depends on:
Blocks:
 
Reported: 2023-12-11 09:30 UTC by Minna Kivinen
Modified: 2024-08-19 20:36 UTC (History)
15 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.03
Circulation function:


Attachments
Bug 35535: Do not pass value of NULL on returns.tt when cancelling hold (1.78 KB, patch)
2024-01-16 18:13 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35535: Allow for cancellation of holds with or without a reason (2.53 KB, patch)
2024-01-17 15:50 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35535: Allow for cancellation of holds with or without a reason (2.58 KB, patch)
2024-01-17 16:01 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35535: Allow for cancellation of holds with or without a reason (2.92 KB, patch)
2024-01-25 16:57 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Minna Kivinen 2023-12-11 09:30:58 UTC
When you check in an item having awaiting hold there comes a pop-up window. There is a button saying Cancel hold.

This button does not do anything.

(you are able to cancel the hold from other hold windows)
Comment 1 Andreas Jonsson 2023-12-14 09:17:06 UTC
This will happen if there are no authorised values in the category HOLD_CANCELLATION, because no select input element will be rendered in this case so the javascript code references an undefined element.

This could be made more robust by adding a default reason if the element is missing:

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
index 2abde7f9c5..92a375531e 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt
@@ -1562,7 +1562,8 @@
                 this.form.submit();
             });
             $('.cancel-hold').on("click",function(e){
-                let cancel_reason = document.getElementById('cancellation-reason').value;
+                const r = document.getElementById('cancellation-reason');
+                let cancel_reason = r ? r.value : 'Manually cancelled';
                 this.form.cancel_reserve.value = 1;
                 this.form.cancel_reason.value = cancel_reason;
                 this.form.submit();
Comment 2 Lucas Gass 2024-01-16 18:10:33 UTC
We can just not pass NULL and use a blank/empty value.
Comment 3 Lucas Gass 2024-01-16 18:13:27 UTC
Created attachment 161072 [details] [review]
Bug 35535: Do not pass value of NULL on returns.tt when cancelling hold

To test:
1. APPLY PATCH
2. Check in an item having awaiting hold there comes a pop-up window. There is a button saying Cancel hold.
3. Cancel the hold without giving a reason.
4. Check that the hold has been properly canceled.
Comment 4 Liz Rea 2024-01-16 18:20:47 UTC
(In reply to Lucas Gass from comment #3)
> Created attachment 161072 [details] [review] [review]
> Bug 35535: Do not pass value of NULL on returns.tt when cancelling hold
> 
> To test:
> 1. APPLY PATCH
> 2. Check in an item having awaiting hold there comes a pop-up window. There
> is a button saying Cancel hold.
> 3. Cancel the hold without giving a reason.
> 4. Check that the hold has been properly canceled.

Noting that the case in which this manifests is when there are no authorized values defined for the category of HOLD_CANCELLATION - so delete those before you test this (if you have some defined) :)
Comment 5 David Nind 2024-01-17 12:47:29 UTC
With the patch applied and no values in HOLD_CANCELLATION authorized value, when I go to cancel the hold:
- There is no cancellation reason dropdown list before the cancel hold button
- I can't escape out of the pop-up modal window
Comment 6 Lucas Gass 2024-01-17 15:28:09 UTC
(In reply to David Nind from comment #5)
> With the patch applied and no values in HOLD_CANCELLATION authorized value,
> when I go to cancel the hold:
> - There is no cancellation reason dropdown list before the cancel hold button
> - I can't escape out of the pop-up modal window

Thanks David, it looks like that happens with or without the patch but let's fix it all here. New patch incoming.
Comment 7 Lucas Gass 2024-01-17 15:50:01 UTC
Created attachment 161112 [details] [review]
Bug 35535: Allow for cancellation of holds with or without a reason

TO TEST:
1. APPLY PATCH
2. Have at least 1 HOLD_CANCELLATION auth value.
3. Check in a waiting hold and make sure you can cancel a hold when selecting the reason.
4. Also try a cancelling this hold without giving a reason.
5. Delete all HOLD_CANCELLATION auth values.
6. Try checking in another hold, there should be no dropdown for choosing a reason.
7. The hold should cancel.
Comment 8 Owen Leonard 2024-01-17 16:01:21 UTC
Created attachment 161113 [details] [review]
Bug 35535: Allow for cancellation of holds with or without a reason

TO TEST:
1. APPLY PATCH
2. Have at least 1 HOLD_CANCELLATION auth value.
3. Check in a waiting hold and make sure you can cancel a hold when
   selecting the reason.
4. Also try a cancelling this hold without giving a reason.
5. Delete all HOLD_CANCELLATION auth values.
6. Try checking in another hold, there should be no dropdown for
   choosing a reason.
7. The hold should cancel.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Tomás Cohen Arazi (tcohen) 2024-01-25 16:57:38 UTC
Created attachment 161449 [details] [review]
Bug 35535: Allow for cancellation of holds with or without a reason

TO TEST:
1. Have a waiting hold on a particular barcode
2. Have at least 1 HOLD_CANCELLATION auth value
3. Check in the barcode and try to cancel the hold on the modal while
   selecting a reason.
=> SUCCESS: You can cancel a hold when selecting a reason.
4. Repeat 1-3 but not selecting any reason.
=> SUCCESS: Hold cancelled correctly.
5. Delete all HOLD_CANCELLATION auth values.
6. Repeat Try 1-3
=> SUCCESS: No reason displayed
=> FAIL: Cancelling does nothing. There's an error in the browser
   inspector
7. Apply this patch
8. Repeat 1-3
=> SUCCESS: No reason displayed
=> SUCCESS: Cancelling works
9. Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Edit: amended the test plan to make it clearer
Comment 10 Lucas Gass 2024-01-25 17:32:55 UTC
> Edit: amended the test plan to make it clearer

Thanks, Tomas!
Comment 11 Katrin Fischer 2024-01-29 11:28:52 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 12 Fridolin Somers 2024-02-01 15:21:26 UTC
Pushed to 23.11.x for 23.11.03
Comment 13 Emily Lamancusa 2024-02-08 16:08:37 UTC
*** Bug 36050 has been marked as a duplicate of this bug. ***
Comment 14 Aleisha Amohia 2024-04-14 23:48:13 UTC
Hello all, can this fix please be backported to 23.05.x?