Bug 40960

Summary: Only generate a notice for patrons about holds filled if they have set messaging preferences
Product: Koha Reporter: Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski>
Component: NoticesAssignee: Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski>
Status: Pushed to main --- QA Contact: Kyle M Hall (khall) <kyle>
Severity: minor    
Priority: P5 - low CC: david, kebliss, lucas
Version: Main   
Hardware: All   
OS: All   
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Currently, if a patron has not set any messaging preferences for notifying them about holds filled, a print notice is still generated. With this change, a notice is now only generated for a patron if their messaging preferences for 'Hold filled' are set. This matches the behavor for overdue and hold reminder notices.
Version(s) released in:
26.05.00
Circulation function:
Attachments: Bug 40960: Fix unit tests
Bug 40960: Print notification only if selected type is impossible
Bug 40960: Fix unit tests
Bug 40960: Print notification only if selected type is impossible
Bug 40960: Fix unit tests
Bug 40960: Print notification only if selected type is impossible
Bug 40960: Fix unit tests
Bug 40960: Print notification only if selected type is impossible

Description Baptiste Wojtkowski (bwoj) 2025-10-06 14:29:53 UTC
In overdue_notice.pl, holds_reminder.pl, no paper notice is sent to patron if no checkbox is checked.

This should be the same for notifying holds.
Comment 1 Baptiste Wojtkowski (bwoj) 2025-10-09 14:47:00 UTC
Created attachment 187669 [details] [review]
Bug 40960: Fix unit tests

Two unit tests have been moved/replaced to fit the desired behaviour

- "patron notified when item set to waiting" -> does not fit the desired behaviour.
  The desired behaviour for mails/sms is already tested
  The desired behaviour for prints is changed to "patron not notified if
  no messaging preference is specified"
- "patron not notified a second time (bug 11445)" -> Moved into
  _koha_notify_reserve tests
Comment 2 Baptiste Wojtkowski (bwoj) 2025-10-09 14:47:03 UTC
Created attachment 187670 [details] [review]
Bug 40960: Print notification only if selected type is impossible

Test plan (ktd):
1 - Have henry acevedo have no mail and set messaging preference "Hold filled to sms"
2 - Have edna acosta have not messaging preference
3 - Place an hold on two books for edna acosta and henry acevedo, check in both books and select the button to fill the hold
4 - Check in notices and see that both received a notice
5 - Apply tests and run tests -> they should fail
6 - Apply patch, run rest -> they should pass
7 - Unfill both holds and fill them back. Henry should keep receiving notices while Edna should not anymore.
Comment 3 David Nind 2025-12-08 21:26:35 UTC
The patch no longer applies 8-(...

git bz apply 40960

Bug 40960 - Notifying holds should not print notice if no messaging preference is notified

187669 - Bug 40960: Fix unit tests
187670 - Bug 40960: Print notification only if selected type is impossible

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 40960: Fix unit tests
Using index info to reconstruct a base tree...
M	t/db_dependent/Reserves.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Reserves.t
CONFLICT (content): Merge conflict in t/db_dependent/Reserves.t
error: Failed to merge in the changes.
Patch failed at 0001 Bug 40960: Fix unit tests
Comment 4 Baptiste Wojtkowski (bwoj) 2025-12-29 11:21:20 UTC
Created attachment 190758 [details] [review]
Bug 40960: Fix unit tests

Two unit tests have been moved/replaced to fit the desired behaviour

- "patron notified when item set to waiting" -> does not fit the desired behaviour.
  The desired behaviour for mails/sms is already tested
  The desired behaviour for prints is changed to "patron not notified if
  no messaging preference is specified"
- "patron not notified a second time (bug 11445)" -> Moved into
  _koha_notify_reserve tests
Comment 5 Baptiste Wojtkowski (bwoj) 2025-12-29 11:21:23 UTC
Created attachment 190759 [details] [review]
Bug 40960: Print notification only if selected type is impossible

Test plan (ktd):
1 - Have henry acevedo have no mail and set messaging preference "Hold filled to sms"
2 - Have edna acosta have not messaging preference
3 - Place an hold on two books for edna acosta and henry acevedo, check in both books and select the button to fill the hold
4 - Check in notices and see that both received a notice
5 - Apply tests and run tests -> they should fail
6 - Apply patch, run rest -> they should pass
7 - Unfill both holds and fill them back. Henry should keep receiving notices while Edna should not anymore.
Comment 6 David Nind 2025-12-29 16:59:42 UTC
Created attachment 190763 [details] [review]
Bug 40960: Fix unit tests

Two unit tests have been moved/replaced to fit the desired behaviour

- "patron notified when item set to waiting" -> does not fit the desired behaviour.
  The desired behaviour for mails/sms is already tested
  The desired behaviour for prints is changed to "patron not notified if
  no messaging preference is specified"
- "patron not notified a second time (bug 11445)" -> Moved into
  _koha_notify_reserve tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2025-12-29 16:59:45 UTC
Created attachment 190764 [details] [review]
Bug 40960: Print notification only if selected type is impossible

Test plan (ktd):
1 - Have henry acevedo have no mail and set messaging preference "Hold filled to sms"
2 - Have edna acosta have not messaging preference
3 - Place an hold on two books for edna acosta and henry acevedo, check in both books and select the button to fill the hold
4 - Check in notices and see that both received a notice
5 - Apply tests and run tests -> they should fail
6 - Apply patch, run rest -> they should pass
7 - Unfill both holds and fill them back. Henry should keep receiving notices while Edna should not anymore.

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2025-12-29 17:01:02 UTC
Testing notes (using KTD):

1. Enable the SMSSendDriver system preference - set to Email.
2. Running the tests: prove t/db_dependent/Reserves.t
Comment 9 Kyle M Hall (khall) 2026-02-06 14:36:34 UTC
Created attachment 192620 [details] [review]
Bug 40960: Fix unit tests

Two unit tests have been moved/replaced to fit the desired behaviour

- "patron notified when item set to waiting" -> does not fit the desired behaviour.
  The desired behaviour for mails/sms is already tested
  The desired behaviour for prints is changed to "patron not notified if
  no messaging preference is specified"
- "patron not notified a second time (bug 11445)" -> Moved into
  _koha_notify_reserve tests

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall (khall) 2026-02-06 14:36:35 UTC
Created attachment 192621 [details] [review]
Bug 40960: Print notification only if selected type is impossible

Test plan (ktd):
1 - Have henry acevedo have no mail and set messaging preference "Hold filled to sms"
2 - Have edna acosta have not messaging preference
3 - Place an hold on two books for edna acosta and henry acevedo, check in both books and select the button to fill the hold
4 - Check in notices and see that both received a notice
5 - Apply tests and run tests -> they should fail
6 - Apply patch, run rest -> they should pass
7 - Unfill both holds and fill them back. Henry should keep receiving notices while Edna should not anymore.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Lucas Gass (lukeg) 2026-02-06 22:20:32 UTC
Nice work everyone!

Pushed to main for 26.05