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
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!
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.
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.
We have hidden the 0 from the dropdown for years now with IntranetUserCSS so it would be nice if this was fixed. :)
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
Created attachment 187993 [details] [review] Bug 28308: JS changes
Created attachment 187994 [details] [review] Bug 28308: DBrev To remove the unnecessary entries from the DB