Bug 28308 - Select 'Days in advance' = 0 for Advance notice effectively disables PREDUE notices
Summary: Select 'Days in advance' = 0 for Advance notice effectively disables PREDUE n...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-10 13:57 UTC by Nick Clemens (kidclamp)
Modified: 2025-10-16 10:56 UTC (History)
6 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 28308: Remove 0 from "days in advance" for "Advance notice" (3.67 KB, patch)
2025-10-16 10:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28308: JS changes (1.49 KB, patch)
2025-10-16 10:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28308: DBrev (1.49 KB, patch)
2025-10-16 10:56 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 Nick Clemens (kidclamp) 2021-05-10 13:57:15 UTC
Looking at the code below, it seems that on the day an item is due we will only send the DUE notices, predues will not be sent

In the advance_notices.pl cron:
276     if ( 0 == $upcoming->{'days_until_due'} ) {
277         # This item is due today. Send an 'item due' message.
...
323     } else {
324         $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $upcoming->{'borrowernumber'},
325                                                                                    message_name   => 'advance_notice' } );
326         next UPCOMINGITEM unless $borrower_preferences && exists $borrower_preferences->{'days_in_advance'};
327         next UPCOMINGITEM unless $borrower_preferences->{'days_in_advance'} == $upcoming->{'days_until_due'};

In this case, allowing the user to choose '0' is effectively disabling the notice. 

I think we need to remove 0 as an option, and uncheck all transports for patrons who have '0' set for days in advance
Comment 1 Jonathan Druart 2021-07-16 10:06:53 UTC
Wasn't there a trick here? I cannot remember.

But yes that does not make sense to have "advance notice 0 day in advance by email" and to not receive a notification!
Comment 2 Katrin Fischer 2021-07-25 01:08:33 UTC
How should we fix this?

We could remove the 0 or replace it with a "-" or "no email" option, but actually it would not be needed. 

Having a drop down of 0-19 seems a little arbitrary anyway. Is there a reason we could not change this to be a validated number input field? Then empty would be the default and we could add some validation on it needing to be set when one of the message transports is selected.
Comment 3 hebah 2023-09-11 15:48:16 UTC
Honestly, I would love to see 'days in advance' move to a syspref default that could be superseded by category exceptions. It's entirely too easy to miss a patron category when populating category messaging prefs, especially for libraries with many categories, and that would enable something like 1 being the default value.
Comment 4 Anneli Österman 2025-10-16 06:24:24 UTC
We have hidden the 0 from the dropdown for years now with IntranetUserCSS so it would be nice if this was fixed. :)
Comment 5 Jonathan Druart 2025-10-16 10:55:56 UTC
Created attachment 187992 [details] [review]
Bug 28308: Remove 0 from "days in advance" for "Advance notice"

0 days in advance means that the notice should not be sent.

This patch contains the pl/pm changes: we set to null if 0 is passed,
and do not insert a row in borrower_message_transport_preferences

Test plan:
0. Do not apply the patches
1. Create some entries with email and days in advance = 0
2. Create some entries with email and days in advance > 0
3. Apply the patches
4. Run the updatedatabase script
=> The rows in borrower_message_transport_preferences have been removed
for days in advance = 0
5. Edit some messaging preferences
=> 0 has been replace with a blank entry
=> checkboxes are disabled when blank is selected
6. Save
=> No new entry in borrower_message_transport_preferences is blank is
selected
7. advance_notices.pl still works correctly
Comment 6 Jonathan Druart 2025-10-16 10:55:58 UTC
Created attachment 187993 [details] [review]
Bug 28308: JS changes
Comment 7 Jonathan Druart 2025-10-16 10:56:01 UTC
Created attachment 187994 [details] [review]
Bug 28308: DBrev

To remove the unnecessary entries from the DB