Bug 38853 - 'Cancel selected' on holds table does not work
Summary: 'Cancel selected' on holds table does not work
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Phil Ringnalda
QA Contact: Pedro Amorim
URL:
Keywords: regression
Depends on: 34478
Blocks:
  Show dependency treegraph
 
Reported: 2025-01-09 12:29 UTC by Pedro Amorim
Modified: 2025-03-22 11:38 UTC (History)
8 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes the "Cancel selected" button for a records' holds table in the staff interface - it now cancels the selected holds. Previously, after you confirmed the hold cancellation nothing happened (the background jobs didn't run and the holds were not cancelled). (This is related to the CSRF changes added in Koha 24.05 to improve form security.)
Version(s) released in:
25.05.00,24.11.03
Circulation function:


Attachments
Bug 38853: Make 'Cancel selected' on holds table work (1.94 KB, patch)
2025-01-09 23:36 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 38853: Make 'Cancel selected' on holds table work (1.98 KB, patch)
2025-01-10 13:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 38853: Make 'Cancel selected' on holds table work (2.04 KB, patch)
2025-01-10 13:36 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 Pedro Amorim 2025-01-09 12:29:03 UTC
To reproduce:
1) Place a hold on a record
2) On the "existing holds" table, check a hold.
3) Click 'Cancel selected". Nothing happens. Ever.
Comment 1 Phil Ringnalda 2025-01-09 16:33:03 UTC
No question, it should be buggy, and I see why (koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt#L1922 means to call the function .empty() but instead throws away the value of checking the property empty).

But something is missing from those steps to reproduce: when I do that, place one any-copy hold on a record with no holds and then check the one hold and cancel selected, I get a background job scheduled which does cancel the hold.
Comment 2 Phil Ringnalda 2025-01-09 23:08:58 UTC
Well, that was fun. Somehow my repo became convinced that origin/main was 23.11.x, so git checkout -b bug_38853 origin/main kept showing me that everything was working just fine.
Comment 3 Phil Ringnalda 2025-01-09 23:36:11 UTC
Created attachment 176306 [details] [review]
Bug 38853: Make 'Cancel selected' on holds table work

Because 'Cancel selected' on the table of existing holds uses
$("#cancel_modal_form #inputs").empty; rather than empty();, it fails to
clear out whatever things are in there before it puts in the things it wants.
That leaves things in a confused state, and makes it fail to actually cancel
holds.

Test plan:
1. Without the patch, add a hold to any bib record
2. Select the checkbox for the hold you placed, then click 'Cancel selected'
   and confirm that you want to cancel
3. Note that there's no message box about a background job having been
   enqueued, and no matter how long you wait, the hold doesn't go away
4. Apply patch, repeat step 2, note that there's now a message box, and
   when the background job finishes, that the hold has been deleted.

Sponsored-by: Chetco Community Public Library
Comment 4 David Nind 2025-01-10 13:29:41 UTC
Created attachment 176327 [details] [review]
Bug 38853: Make 'Cancel selected' on holds table work

Because 'Cancel selected' on the table of existing holds uses
$("#cancel_modal_form #inputs").empty; rather than empty();, it fails to
clear out whatever things are in there before it puts in the things it wants.
That leaves things in a confused state, and makes it fail to actually cancel
holds.

Test plan:
1. Without the patch, add a hold to any bib record
2. Select the checkbox for the hold you placed, then click 'Cancel selected'
   and confirm that you want to cancel
3. Note that there's no message box about a background job having been
   enqueued, and no matter how long you wait, the hold doesn't go away
4. Apply patch, repeat step 2, note that there's now a message box, and
   when the background job finishes, that the hold has been deleted.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Pedro Amorim 2025-01-10 13:36:00 UTC
Created attachment 176330 [details] [review]
Bug 38853: Make 'Cancel selected' on holds table work

Because 'Cancel selected' on the table of existing holds uses
$("#cancel_modal_form #inputs").empty; rather than empty();, it fails to
clear out whatever things are in there before it puts in the things it wants.
That leaves things in a confused state, and makes it fail to actually cancel
holds.

Test plan:
1. Without the patch, add a hold to any bib record
2. Select the checkbox for the hold you placed, then click 'Cancel selected'
   and confirm that you want to cancel
3. Note that there's no message box about a background job having been
   enqueued, and no matter how long you wait, the hold doesn't go away
4. Apply patch, repeat step 2, note that there's now a message box, and
   when the background job finishes, that the hold has been deleted.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 6 Pedro Amorim 2025-01-10 13:36:50 UTC
Thanks for the fix and quick reaction Phil!
Comment 7 Katrin Fischer 2025-01-10 18:21:25 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 8 Phil Ringnalda 2025-01-16 00:45:59 UTC
Better dependency, since it's still empty; and failing to empty() on 24.05, even though that doesn't have the patch to expose the failure in this particular way.
Comment 9 Eric Swenson 2025-02-20 22:08:26 UTC
Would it be possible to have this backported to 24.11?
Comment 10 Paul Derscheid 2025-03-19 23:02:15 UTC
Nice work everyone!

Pushed to 24.11.x for 24.11.03