Bug 23678 - Cancel holds in bulk
Summary: Cancel holds in bulk
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Agustín Moyano
QA Contact: Martin Renvoize
URL:
Keywords:
: 27922 (view as bug list)
Depends on: 29149
Blocks:
  Show dependency treegraph
 
Reported: 2019-09-25 22:45 UTC by Joy Nelson
Modified: 2021-10-08 13:45 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
This developments adds a way to choose multiple holds using checkboxes, to cancel them in bulk. It uses the new background jobs infrastructure recently introduced.
Version(s) released in:
21.11.00


Attachments
Bug 23678: Allow cancel holds in bulk (19.00 KB, patch)
2021-03-16 16:40 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23678: Allow cancel holds in bulk (31.80 KB, patch)
2021-03-19 21:14 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 23678: (QA follow-up) Add missing template filter (2.10 KB, patch)
2021-04-28 11:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23678: Allow cancel holds in bulk (31.92 KB, patch)
2021-07-26 10:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23678: (QA follow-up) Add missing template filter (2.13 KB, patch)
2021-07-26 10:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23678: (QA follow-up) Add missing filters (1.99 KB, patch)
2021-07-26 10:56 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23678: Allow cancel holds in bulk (31.95 KB, patch)
2021-09-28 13:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23678: (QA follow-up) Add missing template filter (2.17 KB, patch)
2021-09-28 13:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23678: (QA follow-up) Add missing filters (2.02 KB, patch)
2021-09-28 13:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23678: (QA follow-up) Use correct indentation (5.21 KB, patch)
2021-09-29 23:21 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23678: (QA follow-up) Use correct indentation (9.93 KB, patch)
2021-10-01 08:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23678: Allow cancel holds in bulk (35.21 KB, patch)
2021-10-01 08:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23678: Don't display code of the job type (1.47 KB, patch)
2021-10-01 08:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23678: Allow cancel holds in bulk (31.94 KB, patch)
2021-10-01 09:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23678: Don't display code of the job type (1.52 KB, patch)
2021-10-01 09:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23678: Include file for batch_hold_cancel (5.36 KB, patch)
2021-10-01 10:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23678: Allow cancel holds in bulk (31.97 KB, patch)
2021-10-01 13:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23678: Don't display code of the job type (1.57 KB, patch)
2021-10-01 13:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23678: Include file for batch_hold_cancel (5.41 KB, patch)
2021-10-01 13:20 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 23678: Include file for batch_hold_cancel (5.43 KB, patch)
2021-10-01 14:09 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joy Nelson 2019-09-25 22:45:51 UTC
Allow staff a way to cancel more than one hold at a time.
Provide checkboxes on holds waiting over tab and allow staff to select multiple holds and cancel those selected holds.
Comment 1 Martha Fuerst 2020-07-30 14:37:32 UTC
Chiming in with a YES PLEASE from Huntsville-Madison County Public Library.
Comment 2 Tomás Cohen Arazi 2020-09-02 20:20:21 UTC
Linking to bug 24599, as we need a spec for batch actions on holds, including cancelling them.
Comment 3 Christopher Brannon 2021-02-10 20:01:16 UTC
(In reply to Joy Nelson from comment #0)
> Allow staff a way to cancel more than one hold at a time.
> Provide checkboxes on holds waiting over tab and allow staff to select
> multiple holds and cancel those selected holds.

+1
Comment 4 Benjamin Daeuber 2021-02-10 20:05:56 UTC
Not that I'm opposed to batch action on holds, but is that strictly required for this development? We'd likely go through and check everything in anyway since there might be other transfers, materials specified notes or whatever to deal with.
Comment 5 Benjamin Daeuber 2021-02-10 20:10:00 UTC
(In reply to Benjamin Daeuber from comment #4)
> Not that I'm opposed to batch action on holds, but is that strictly required
> for this development? We'd likely go through and check everything in anyway
> since there might be other transfers, materials specified notes or whatever
> to deal with.

Ah, never mind. I think I see what you mean here. The bulk action for canceling the holds. I was sidetracked by a discussion on triggering multiple holds at one time.
Comment 6 Agustín Moyano 2021-03-16 16:40:03 UTC
Created attachment 118338 [details] [review]
Bug 23678: Allow cancel holds in bulk

This patch allows staff patrons to cancel multiple holds in bulk.

To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
      => When you uncheck one of the checkboxes, the one in the header also gets unchecked.
      => If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
      => If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
      => If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
      => Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
        => A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
   each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
Comment 7 Barbara Johnson 2021-03-16 18:22:56 UTC
Hi Agustín - I tested this and it works great.  However, the functionality is tied to cancelling holds on a queue for an individual title.  I actually just submitted a bug for that - #27922.

This bug states that it should allow cancellation of selected holds via checkboxes from the 'holds waiting over xx days' tab on waitingreserves.pl.  So while the patch works great I don't think it address the original intention of this bug.
Comment 8 Agustín Moyano 2021-03-17 15:57:00 UTC
(In reply to Barbara Johnson from comment #7)
> Hi Agustín - I tested this and it works great.  However, the functionality
> is tied to cancelling holds on a queue for an individual title.  I actually
> just submitted a bug for that - #27922.
> 
> This bug states that it should allow cancellation of selected holds via
> checkboxes from the 'holds waiting over xx days' tab on waitingreserves.pl. 
> So while the patch works great I don't think it address the original
> intention of this bug.

Thanks Barbara.. I'm such a dummy!
Comment 9 Agustín Moyano 2021-03-17 17:10:15 UTC
*** Bug 27922 has been marked as a duplicate of this bug. ***
Comment 10 Agustín Moyano 2021-03-19 21:14:25 UTC
Created attachment 118553 [details] [review]
Bug 23678: Allow cancel holds in bulk

This patch allows staff patrons to cancel multiple holds in bulk.

To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
      => When you uncheck one of the checkboxes, the one in the header also gets unchecked.
      => If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
      => If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
      => If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
      => Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
        => A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
   each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
10. Check In some of the items so the get in Waiting state.
11. Update expirationdate os some of those holds and set it to
    ReservesMaxPickUpDelay + 1 days earlier
NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be
     => update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...)
12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs.
Comment 11 Agustín Moyano 2021-03-19 21:19:11 UTC
(In reply to Barbara Johnson from comment #7)
> Hi Agustín - I tested this and it works great.  However, the functionality
> is tied to cancelling holds on a queue for an individual title.  I actually
> just submitted a bug for that - #27922.
> 
> This bug states that it should allow cancellation of selected holds via
> checkboxes from the 'holds waiting over xx days' tab on waitingreserves.pl. 
> So while the patch works great I don't think it address the original
> intention of this bug.

Hi Barbara, could you please try once again?

Thanks
Comment 12 Barbara Johnson 2021-04-23 13:45:56 UTC
Everything worked great.  Also checked to make sure that if a title had several holds that using the bulk cancel did not impact the rest of the holds queue.
Comment 13 Kyle M Hall 2021-04-28 11:42:01 UTC
Created attachment 120265 [details] [review]
Bug 23678: (QA follow-up) Add missing template filter
Comment 14 Katrin Fischer 2021-07-25 01:24:24 UTC
Requires another follow-up:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt
   FAIL	  filters
		missing_filter at line 180 (                                Hold on <a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% m.biblio_id | uri %]">[% m.biblio_title | html %]</a> for <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% m.patron_id | uri %]">[% m.patron_name %]</a> has successfully been cancelled.)
Comment 15 Tomás Cohen Arazi 2021-07-26 10:55:50 UTC
Created attachment 123185 [details] [review]
Bug 23678: Allow cancel holds in bulk

This patch allows staff patrons to cancel multiple holds in bulk.

To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
      => When you uncheck one of the checkboxes, the one in the header also gets unchecked.
      => If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
      => If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
      => If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
      => Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
        => A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
   each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
10. Check In some of the items so the get in Waiting state.
11. Update expirationdate os some of those holds and set it to
    ReservesMaxPickUpDelay + 1 days earlier
NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be
     => update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...)
12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2021-07-26 10:55:55 UTC
Created attachment 123186 [details] [review]
Bug 23678: (QA follow-up) Add missing template filter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Tomás Cohen Arazi 2021-07-26 10:56:00 UTC
Created attachment 123187 [details] [review]
Bug 23678: (QA follow-up) Add missing filters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2021-07-26 10:58:49 UTC
(In reply to Katrin Fischer from comment #14)
> Requires another follow-up:

Done, thanks
Comment 19 Martin Renvoize 2021-09-28 13:20:15 UTC
Created attachment 125404 [details] [review]
Bug 23678: Allow cancel holds in bulk

This patch allows staff patrons to cancel multiple holds in bulk.

To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
      => When you uncheck one of the checkboxes, the one in the header also gets unchecked.
      => If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
      => If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
      => If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
      => Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
        => A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
   each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
10. Check In some of the items so the get in Waiting state.
11. Update expirationdate os some of those holds and set it to
    ReservesMaxPickUpDelay + 1 days earlier
NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be
     => update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...)
12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize 2021-09-28 13:20:19 UTC
Created attachment 125405 [details] [review]
Bug 23678: (QA follow-up) Add missing template filter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 21 Martin Renvoize 2021-09-28 13:20:24 UTC
Created attachment 125406 [details] [review]
Bug 23678: (QA follow-up) Add missing filters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 22 Martin Renvoize 2021-09-28 13:21:51 UTC
This works as expected and passes QA scripts. I can't see any evidence of previous background jobs having associated tests so the lack of a unit test here seems reasonable.. but it did make me wonder.

Passing QA and differing to RM regards tests.
Comment 23 Tomás Cohen Arazi 2021-09-29 23:21:38 UTC
Created attachment 125470 [details] [review]
Bug 23678: (QA follow-up) Use correct indentation

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 24 Jonathan Druart 2021-09-30 14:33:45 UTC
1. I don't understand why we pass patron_name when we have the patron_id (same for biblio_title).
If you modify the patron's name, the name won't match.

If you cannot do it now, you may need Koha::BackgroundJob->additional_report from "Bug 28445: Add the capability to provide more info to the background job detail view"

2.
+            push @messages, {
+                type => 'error',
+                code => 'hold_not_cancelled',
+                patron_id => defined $patron?$patron->borrowernumber:'',
+                patron_name => defined $patron?($patron->firstname?$patron->firstname.', ':'').$patron->surname:'',
+                biblio_id => defined $biblio?$biblio->biblionumber:'',
+                biblio_title => defined $biblio?$biblio->title:'',
+                hold_id => $hold_id,
+                error => defined $hold?($@ ? $@ : 0):'No hold with id '.$hold_id.' found',
+            };

Erk, spaces, indentation? I can fix that.
Comment 25 Jonathan Druart 2021-10-01 08:34:38 UTC
Created attachment 125577 [details] [review]
Bug 23678: (QA follow-up) Use correct indentation

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

JD amended patch: also Koha/BackgroundJob/BatchCancelHold.pm
Comment 26 Jonathan Druart 2021-10-01 08:35:15 UTC
Created attachment 125578 [details] [review]
Bug 23678: Allow cancel holds in bulk

This patch allows staff patrons to cancel multiple holds in bulk.

To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
      => When you uncheck one of the checkboxes, the one in the header also gets unchecked.
      => If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
      => If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
      => If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
      => Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
        => A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
   each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
10. Check In some of the items so the get in Waiting state.
11. Update expirationdate os some of those holds and set it to
    ReservesMaxPickUpDelay + 1 days earlier
NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be
     => update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...)
12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Add missing template filter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Add missing filters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Use correct indentation

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

JD amended patch: also Koha/BackgroundJob/BatchCancelHold.pm
Comment 27 Jonathan Druart 2021-10-01 08:35:29 UTC
Patches squashed.
Comment 28 Jonathan Druart 2021-10-01 08:41:07 UTC
Created attachment 125579 [details] [review]
Bug 23678: Don't display code of the job type
Comment 29 Jonathan Druart 2021-10-01 09:16:05 UTC
Created attachment 125589 [details] [review]
Bug 23678: Allow cancel holds in bulk

This patch allows staff patrons to cancel multiple holds in bulk.

To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
      => When you uncheck one of the checkboxes, the one in the header also gets unchecked.
      => If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
      => If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
      => If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
      => Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
        => A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
   each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
10. Check In some of the items so the get in Waiting state.
11. Update expirationdate os some of those holds and set it to
    ReservesMaxPickUpDelay + 1 days earlier
NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be
     => update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...)
12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Add missing template filter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Add missing filters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Use correct indentation

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

JD amended patch: also Koha/BackgroundJob/BatchCancelHold.pm

JD Amended patch: Full rebase and adjustements made on top of bug 26080.
Comment 30 Jonathan Druart 2021-10-01 09:16:10 UTC
Created attachment 125590 [details] [review]
Bug 23678: Don't display code of the job type
Comment 31 Jonathan Druart 2021-10-01 09:16:54 UTC
Patches rebased and reworked on top of bug 26080 (PQA for a month already).
It refactored some of the background job code.
Comment 32 Jonathan Druart 2021-10-01 09:44:46 UTC
 99                 error        => defined $hold
100                 ? ( $@ ? $@ : 0 )
101                 : 'No hold with id ' . $hold_id . ' found',

Caught a non translatable error.
Comment 33 Jonathan Druart 2021-10-01 10:01:00 UTC
Created attachment 125600 [details] [review]
Bug 23678: Include file for batch_hold_cancel

Use additional_report to retrieve patron and biblio's info.
Comment 34 Jonathan Druart 2021-10-01 10:01:59 UTC
Tomas, have a look at the current version and the dependent bugs. I moved code out of bug 28445 to reuse it here.
Comment 35 Tomás Cohen Arazi 2021-10-01 13:20:40 UTC
Created attachment 125627 [details] [review]
Bug 23678: Allow cancel holds in bulk

This patch allows staff patrons to cancel multiple holds in bulk.

To test:
1. Apply this patch
2. restart_all
3. In cataloge go to a book and place many holds
CHECK => Holds table shows a column of checkboxes
4. Play with checkboxes (have some fun ;-P)
CHECK => When you manually check all checkboxes, the checkbox in the
header also gets checked.
      => When you uncheck one of the checkboxes, the one in the header also gets unchecked.
      => If no checkbox is checked and you check the one in the header,
all checkboxes get checked.
      => If there are some checkboxes that are checked and others are
not, when you click on the checkbox in the header all checkboxes get
unchecked.
      => If all checkboxes are checked, when you uncheck the one in the
header, all checkboxes get unchecked.
      => Every time you play with checkboxes, the number in the button
"Cancel selected" changes.
5. Check some of the checkboxes and click on cancel selected.
SUCCESS => A background job gets fired to cancel all selected holds.
        => A message should appear with a link to the job.
6. Wait a few seconds and click on the link
SUCCESS => A message appears with the report of the execution of the
background job.
7. Grab a patron and search to hold
8. Select multiple biblios and click on "place hold for <patron>"
CHECK => After holds are confirmed, multiple holds table are shown.. one for
   each record. Checkboxes work exactly the same as before, but scoped
for each individual table. Checkboxes from one table will not affect
checkboxes from other tables.
9. Repeat steps 4 to 6.
10. Check In some of the items so the get in Waiting state.
11. Update expirationdate os some of those holds and set it to
    ReservesMaxPickUpDelay + 1 days earlier
NOTE => ReservesMaxPickUpDelay = 7 days by default, so sql syntax to update would be
     => update reserves set expirationdate = date_sub(expirationdate, interval 8 day) where reserve_id in (...)
12. Repeat steps 4 to 6 but in waitingreserves.pl, in both tabs.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Add missing template filter

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Add missing filters

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Bug 23678: (QA follow-up) Use correct indentation

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

JD amended patch: also Koha/BackgroundJob/BatchCancelHold.pm

JD Amended patch: Full rebase and adjustements made on top of bug 26080.
Comment 36 Tomás Cohen Arazi 2021-10-01 13:20:46 UTC
Created attachment 125628 [details] [review]
Bug 23678: Don't display code of the job type

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 37 Tomás Cohen Arazi 2021-10-01 13:20:54 UTC
Created attachment 125629 [details] [review]
Bug 23678: Include file for batch_hold_cancel

Use additional_report to retrieve patron and biblio's info.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 38 Jonathan Druart 2021-10-01 14:09:45 UTC
Created attachment 125631 [details] [review]
Bug 23678: Include file for batch_hold_cancel

Use additional_report to retrieve patron and biblio's info.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 39 Jonathan Druart 2021-10-01 14:30:02 UTC
Pushed to master for 21.11, thanks to everybody involved!