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: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks:
 
Reported: 2021-05-10 13:57 UTC by Nick Clemens (kidclamp)
Modified: 2024-06-19 15:24 UTC (History)
5 users (show)

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


Attachments

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.