[Bug 23411] New: SMS messages should not fall back to 'email'
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Bug ID: 23411 Summary: SMS messages should not fall back to 'email' Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Bug 15967 ensured that an email notice that falls back to print when the patron has no email will send a print notice using the email template if there is no print template. When SMS and fallback were added this means that an SMS message where there is no SMS template will use the email template - it shouldn't as SMS messages need to be shorter, we should instead fail sending SMS if no SMS template defined -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 91907 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91907&action=edit Bug 23411 - SMS messages should not fall back to 'email' To test: 1 - Have a patron wiht an sms number, but no email 2 - Enable sms by setting SMSDriver syspref to Email 3 - Set Notice triggers to send an overdue via sms and email 4 - Ensure the selected notice is defined only for 'email' 5 - Checkout an overdue to the patron above You can set a specify a due date in the past 6 - perl misc/cronjobs/overdue_notices.pl -t 7 - Check the patron notices tab, they have a print notice and an SMS notice pending 8 - The SMS notice used the 'email' template 9 - Apply patch 10 - sudo koha-mysql kohadev 11 - DELETE * FROM message_queue WHERE borrowernumber={borrower as above}; 12 - Run the cron again 13 - Patron should have a print notice queued, no sms 14 - Define an SMS notice 15 - delete the message queue 16 - Run again 17 - The patron should have a print and an SMS -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Michal Denar <black23@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |black23@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91907|0 |1 is obsolete| | --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 99511 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99511&action=edit Bug 23411 - SMS messages should not fall back to 'email' To test: 1 - Have a patron wiht an sms number, but no email 2 - Enable sms by setting SMSDriver syspref to Email 3 - Set Notice triggers to send an overdue via sms and email 4 - Ensure the selected notice is defined only for 'email' 5 - Checkout an overdue to the patron above You can set a specify a due date in the past 6 - perl misc/cronjobs/overdue_notices.pl -t 7 - Check the patron notices tab, they have a print notice and an SMS notice pending 8 - The SMS notice used the 'email' template 9 - Apply patch 10 - sudo koha-mysql kohadev 11 - DELETE * FROM message_queue WHERE borrowernumber={borrower as above}; 12 - Run the cron again 13 - Patron should have a print notice queued, no sms 14 - Define an SMS notice 15 - delete the message queue 16 - Run again 17 - The patron should have a print and an SMS Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org Depends on| |15967 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Nick, the test plan is considering SMS, but the code is for any MTT. What about the test plan from bug 15967? Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15967 [Bug 15967] Print notices are not generated if the patron cannot be notified -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #3)
Nick, the test plan is considering SMS, but the code is for any MTT.
The only time $effective_mtt and $mtt should be different are for email/sms - for email nothing will change, only SMS is affected. Future types could be affected by this code, however, I think it is the correct behaviour
What about the test plan from bug 15967?
This does not invalidate that - there we fellback to email if we were sending a print but didn't have a print notice. We continue to do that, but only change falling back to SMS when sending SMS via print if there is no print notice. The issue was that sometimes we would send an email notice via SMS if we didn't have an SMS notice defined - that causes problems and can generate multiple messages etc. If sending SMS and no SMS notice is defined, then we should not send. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Sorry but I am not convince by the code. If what we want is to not send a SMS notice if there is not SMS template, why are not we written that explicitly in the code? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 --- Comment #6 from Nick Clemens <nick@bywatersolutions.com> --- This patch makes a very small change in the case that we cannot send our notice directly because the patron doesn't have a valid address to send to. Current logic: If we don't have an email address, send a print notice - if we can't find a print template use the email template If we don't have an SMS number, send a print notice - if we can't find a print template use the email template New logic: If we don't have an email address, send a print notice - if we can't find a print template use the email template If we don't have an SMS number, send a print notice - if we can't find a print template use the SMS template (In reply to Jonathan Druart from comment #5)
If what we want is to not send a SMS notice if there is not SMS template, why are not we written that explicitly in the code?
If I set a notice to have a print version and no email version, and I trigger it as an email, no message is sent. We get an error: No circulation CHECKOUT letter transported by email at /kohadevbox/koha/C4/Letters.pm line 608. Why do we not do the same for SMS? This code is not changing that anyways, it simply changes that if we try to send an SMS but cannot and are going to send a 'print' then we should use the SMS template if there is no print template. This affects the @output_chunks of the code - the notice that gets sent to libraries saying "These messages were not sent directly to the patron" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|SMS messages should not |SMS messages sent as print |fall back to 'email' |should not fall back to | |'email' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99511|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 101091 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101091&action=edit Bug 23411: Do not fall back to 'email' for SMS messages To test: 1 - Have a patron wiht an sms number, but no email 2 - Enable sms by setting SMSDriver syspref to Email 3 - Set Notice triggers to send an overdue via sms and email 4 - Ensure the selected notice is defined only for 'email' 5 - Checkout an overdue to the patron above You can set a specify a due date in the past 6 - perl misc/cronjobs/overdue_notices.pl -t 7 - Check the patron notices tab, they have a print notice and an SMS notice pending 8 - The SMS notice used the 'email' template 9 - Apply patch 10 - sudo koha-mysql kohadev 11 - DELETE * FROM message_queue WHERE borrowernumber={borrower as above}; 12 - Run the cron again 13 - Patron should have a print notice queued, no sms 14 - Define an SMS notice 15 - delete the message queue 16 - Run again 17 - The patron should have a print and an SMS Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I still have the feeling that something is not 100% correct here, but cannot find what, so passing QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #101091|0 |1 is obsolete| | --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 101092 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101092&action=edit Bug 23411: Do not fall back to 'email' for SMS messages To test: 1 - Have a patron wiht an sms number, but no email 2 - Enable sms by setting SMSDriver syspref to Email 3 - Set Notice triggers to send an overdue via sms and email 4 - Ensure the selected notice is defined only for 'email' 5 - Checkout an overdue to the patron above You can set a specify a due date in the past 6 - perl misc/cronjobs/overdue_notices.pl -t 7 - Check the patron notices tab, they have a print notice and an SMS notice pending 8 - The SMS notice used the 'email' template 9 - Apply patch 10 - sudo koha-mysql kohadev 11 - DELETE * FROM message_queue WHERE borrowernumber={borrower as above}; 12 - Run the cron again 13 - Patron should have a print notice queued, no sms 14 - Define an SMS notice 15 - delete the message queue 16 - Run again 17 - The patron should have a print and an SMS Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|20.05.00 |20.05.00, 19.11.05 released in| | CC| |joy@bywatersolutions.com --- Comment #11 from Joy Nelson <joy@bywatersolutions.com> --- backported to 19.11.x for 19.11.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|20.05.00, 19.11.05 |20.05.00, 19.11.05, released in| |19.05.10 CC| |lucas@bywatersolutions.com --- Comment #12 from Lucas Gass <lucas@bywatersolutions.com> --- backported to 19.05.x for 19.05.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23411 Hayley Mapley <hayleymapley@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hayleymapley@catalyst.net.n | |z --- Comment #13 from Hayley Mapley <hayleymapley@catalyst.net.nz> --- Minor/normal will not be backported to 18.11.x series -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org