Bug 36103 - Remove the "Cancel hold" link for item level holds
Summary: Remove the "Cancel hold" link for item level holds
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords: release-notes-needed
Depends on:
Blocks:
 
Reported: 2024-02-15 12:14 UTC by lasse.pouru
Modified: 2024-03-19 21:17 UTC (History)
8 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.04,23.05.10


Attachments
Bug 36103: Remove cancel-hold class (2.35 KB, patch)
2024-02-27 11:03 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36103: Remove the "Cancel hold" link for item level holds (1.56 KB, patch)
2024-02-27 13:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36103: Remove the "Cancel hold" link for item level holds (1.61 KB, patch)
2024-02-27 15:16 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 36103: Remove the "Cancel hold" link for item level holds (1.67 KB, patch)
2024-02-27 15:38 UTC, Pedro Amorim
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description lasse.pouru 2024-02-15 12:14:46 UTC
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.
Comment 1 Pedro Amorim 2024-02-27 10:37:23 UTC Comment hidden (obsolete)
Comment 2 Pedro Amorim 2024-02-27 11:03:04 UTC
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.
Comment 3 Pedro Amorim 2024-02-27 11:07:17 UTC
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?
Comment 4 Owen Leonard 2024-02-27 12:16:15 UTC
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.
Comment 5 Pedro Amorim 2024-02-27 12:52:23 UTC
(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.
Comment 6 Kyle M Hall 2024-02-27 13:36:58 UTC
(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!
Comment 7 Kyle M Hall 2024-02-27 13:43:34 UTC
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.
Comment 8 Owen Leonard 2024-02-27 15:16:18 UTC
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>
Comment 9 Pedro Amorim 2024-02-27 15:38:51 UTC
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>
Comment 10 Katrin Fischer 2024-03-08 17:46:41 UTC
Can we add the conclusion of the discussion to the release notes please? :)
Comment 11 Katrin Fischer 2024-03-08 17:59:59 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 12 Fridolin Somers 2024-03-11 14:29:54 UTC
Pushed to 23.11.x for 23.11.04
Comment 13 Lucas Gass 2024-03-19 21:17:15 UTC
Backported to 23.05.x for upcoming 23.05.10