Bug 35535

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();

We can just not pass NULL and use a blank/empty value.

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.

(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) :)

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

(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.

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.

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>

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

> Edit: amended the test plan to make it clearer

Thanks, Tomas!

Pushed for 24.05!

Well done everyone, thank you!

Pushed to 23.11.x for 23.11.03

*** Bug 36050 has been marked as a duplicate of this bug. ***

Hello all, can this fix please be backported to 23.05.x?