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)
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?