1. Open the "Holds" tab of a record with an item level hold. 2. Select a patron from the patron search. 3. Click on the "Cancel hold" link. (Link appears as it should, e.g. "modrequest.pl?CancelBiblioNumber=58&CancelBorrowerNumber=5&CancelItemnumber=125"). 4. Popup appears. Click "Confirm cancellation". 5. Page loads. Instead of the correct data for the hold to cancel, all of the values in the URL are null, i.e. "request.pl?action=cancel&borrowernumber=undefined&biblionumber=undefined&reserve_id=undefined". The hold is not cancelled.
I can't reproduce this. The "Cancel hold" link for me appears as: request.pl?action=cancel&borrowernumber=51&biblionumber=76&reserve_id=56&date=2024-02-27 Pop up appears, I click "Confirm cancellation" and the hold is cancelled as expected.
Created attachment 162492 [details] [review] Bug 36103: Remove cancel-hold class 1. Open the "Holds" tab of a record with an item level hold. 2. Select a patron from the patron search. 3. Click on the "Cancel hold" link. (Link appears as it should, e.g. "modrequest.pl?CancelBiblioNumber=58&CancelBorrowerNumber=5&CancelItemnumber=125"). 4. Popup appears. Click "Confirm cancellation". 5. Page loads. Instead of the correct data for the hold to cancel, all of the values in the URL are null, i.e. "request.pl?action=cancel&borrowernumber=undefined&biblionumber=undefined&reserve_id=undefined". The hold is not cancelled. 6. Apply patch. Repeat. Notice it now cancels the hold as expected.
I'm not convinced by my patch but I'm not sure what the intend here is so I'm adding Owen and Kyle in the hopes for additional input. It seems the JS using .cancel-hold in reserve/request.tt is meant to be working on the .cancel-hold link coming from holds_table.inc, i.e. this table: http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=76 But its also affecting the other table mentioned in the test plan, this table: http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=76&borrowernumber=51 (To see the problematic "Cancel hold" link here you need to have a hold previously placed) So, do we remove this CSS class from this link and keep its behaviour as before? Or do we move this problematic link to also make use of the cancellation reason modal and request.pl?action=cancel or the older modRequest.pl as currently is?
In my opinion we should remove the link altogether. I don't think it makes sense to offer a cancel link when a new hold is in the process of being placed.
(In reply to Owen Leonard from comment #4) > In my opinion we should remove the link altogether. I don't think it makes > sense to offer a cancel link when a new hold is in the process of being > placed. Makes sense. Updating this to "In Discussion" and adding others to the discussion.
(In reply to Owen Leonard from comment #4) > In my opinion we should remove the link altogether. I don't think it makes > sense to offer a cancel link when a new hold is in the process of being > placed. Nick and I both happened to discuss this very link last week and we came to the same conclusion!
Created attachment 162502 [details] [review] Bug 36103: Remove the "Cancel hold" link for item level holds This link is broken and doesn't make sense from a UI/UX perspective and thus should be removed.
Created attachment 162504 [details] [review] Bug 36103: Remove the "Cancel hold" link for item level holds This link is broken and doesn't make sense from a UI/UX perspective and thus should be removed. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 162513 [details] [review] Bug 36103: Remove the "Cancel hold" link for item level holds This link is broken and doesn't make sense from a UI/UX perspective and thus should be removed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Can we add the conclusion of the discussion to the release notes please? :)
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.04
Backported to 23.05.x for upcoming 23.05.10