Bug 32518 - Add reason option to cancel_unfilled_holds
Summary: Add reason option to cancel_unfilled_holds
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-12-22 21:02 UTC by Nick Clemens
Modified: 2023-12-28 20:45 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00


Attachments
Bug 32518: Add reason option to cancel_unfilled_holds cronjob (2.80 KB, patch)
2022-12-22 21:14 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32518: Add reason option to cancel_unfilled_holds cronjob (2.80 KB, patch)
2022-12-23 13:25 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 32518: Add reason option to cancel_unfilled_holds cronjob (2.86 KB, patch)
2023-01-27 15:07 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 32518: Add reason option to cancel_unfilled_holds cronjob (2.91 KB, patch)
2023-02-27 08:54 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 32518: (QA follow-up) Fix alignment (867 bytes, patch)
2023-02-27 08:54 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-12-22 21:02:34 UTC
In cancel_expired_holds we can add a reason to the cron send emails relating to cancellation, we should have the same here
Comment 1 Nick Clemens 2022-12-22 21:14:52 UTC
Created attachment 144806 [details] [review]
Bug 32518: Add reason option to cancel_unfilled_holds cronjob

This adds a reaosn parameter and passes it into the cancellation if supplied

To test:
1 - Place a hold for a patron in your system
2 - Run script with --days 0 -v
3 - verify that it would cancel the reserves (and that you are okay with cancelling the ones it found)
4 - Make sure you have a notice in the holds module with code 'HOLD_CANCELLATION'
5 - Set content of the notice like:
[% IF hold.cancellation_reason=='too_old' %]
Canceled old
[% END %]
6 - Run script with --days 0 -v --reason too_bad -c
7 - Confirm hold cancelled, no notice sent to patron
8 - Place another hold
9 - Run script with --days 0 -v --reason too_old -c
10 - Confirm hold cancelled, notice sent to patron
Comment 2 Nick Clemens 2022-12-23 13:25:19 UTC
Created attachment 144820 [details] [review]
Bug 32518: Add reason option to cancel_unfilled_holds cronjob

This adds a reaosn parameter and passes it into the cancellation if supplied

To test:
1 - Place a hold for a patron in your system
2 - Run script with --days 0 -v
3 - verify that it would cancel the reserves (and that you are okay with cancelling the ones it found)
4 - Make sure you have a notice in the holds module with code 'HOLD_CANCELLATION'
5 - Set content of the notice like:
[% IF hold.cancellation_reason=='too_old' %]
Canceled old
[% END %]
6 - Run script with --days 0 -v --reason too_bad -c
7 - Confirm hold cancelled, no notice sent to patron
8 - Place another hold
9 - Run script with --days 0 -v --reason too_old -c
10 - Confirm hold cancelled, notice sent to patron
Comment 3 Andrew Fuerste-Henry 2023-01-27 15:07:44 UTC
Created attachment 145755 [details] [review]
Bug 32518: Add reason option to cancel_unfilled_holds cronjob

This adds a reaosn parameter and passes it into the cancellation if supplied

To test:
1 - Place a hold for a patron in your system
2 - Run script with --days 0 -v
3 - verify that it would cancel the reserves (and that you are okay with cancelling the ones it found)
4 - Make sure you have a notice in the holds module with code 'HOLD_CANCELLATION'
5 - Set content of the notice like:
[% IF hold.cancellation_reason=='too_old' %]
Canceled old
[% END %]
6 - Run script with --days 0 -v --reason too_bad -c
7 - Confirm hold cancelled, no notice sent to patron
8 - Place another hold
9 - Run script with --days 0 -v --reason too_old -c
10 - Confirm hold cancelled, notice sent to patron

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 4 Victor Grousset/tuxayo 2023-02-27 08:54:13 UTC
Created attachment 147424 [details] [review]
Bug 32518: Add reason option to cancel_unfilled_holds cronjob

This adds a reaosn parameter and passes it into the cancellation if supplied

To test:
1 - Place a hold for a patron in your system
2 - Run script with --days 0 -v
3 - verify that it would cancel the reserves (and that you are okay with cancelling the ones it found)
4 - Make sure you have a notice in the holds module with code 'HOLD_CANCELLATION'
5 - Set content of the notice like:
[% IF hold.cancellation_reason=='too_old' %]
Canceled old
[% END %]
6 - Run script with --days 0 -v --reason too_bad -c
7 - Confirm hold cancelled, no notice sent to patron
8 - Place another hold
9 - Run script with --days 0 -v --reason too_old -c
10 - Confirm hold cancelled, notice sent to patron

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2023-02-27 08:54:15 UTC
Created attachment 147425 [details] [review]
Bug 32518: (QA follow-up) Fix alignment

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2023-02-27 08:54:41 UTC
Works, QA script happy, code looks good, passing QA :)
Comment 7 Tomás Cohen Arazi 2023-03-02 17:45:55 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!