[Bug 23357] New: overdue_notices.pl can send library doubled emails for patrons with no email/sms if both transports enabled
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 Bug ID: 23357 Summary: overdue_notices.pl can send library doubled emails for patrons with no email/sms if both transports enabled Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Command-line Utilities Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: robin@catalyst.net.nz In overdue_notices.pl we have a check to avoid sending a patron two notices if they are missing an email address and an sms number, however, we don't apply this check to generating the notice content and so send the library the same info twice -- 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=23357 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Trivial patch -- 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=23357 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 91693 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=91693&action=edit Bug 23357: Check if generating a duplicate print notice before generating notice To test: 1 - Check out an overdue to a patron 2 - Set a notice trigger for the same number of days overdue for the patrons patron type 3 - Set the trigger to send via email and sms You may need to define SMSSendDriver preference to enable SMS, you can put 'Email' 4 - perl misc/cronjobs/overdue_notices.pl -v -t 5 - Check the patrons record, 1 notice generated 6 - Check the db: SELECT * FROM message_queue; 7 - Note the message 'Overdue notices' contains a doulbed content for the notice 8 - DELETE * FROM message_queue; 9 - Apply patch 10 - perl misc/cronjobs/overdue_notices.pl -v -t 11 - Chekc the patron's account, 1 message in 'Notices' 12 - Check the DB SELECT * FROM message_queue; 13 - The 'Overdue notices' notice is not doubled -- 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=23357 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, | |rhouk@ottumwapubliclibrary. | |org Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | -- 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=23357 Margaret <margaret@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |margaret@bywatersolutions.c | |om -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #91693|0 |1 is obsolete| | --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 100325 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100325&action=edit Bug 23357: Check if generating a duplicate print notice before generating notice To test: 1 - Check out an overdue to a patron 2 - Set a notice trigger for the same number of days overdue for the patrons patron type 3 - Set the trigger to send via email and sms You may need to define SMSSendDriver preference to enable SMS, you can put 'Email' 4 - perl misc/cronjobs/overdue_notices.pl -v -t 5 - Check the patrons record, 1 notice generated 6 - Check the db: SELECT * FROM message_queue; 7 - Note the message 'Overdue notices' contains a doulbed content for the notice 8 - DELETE * FROM message_queue; 9 - Apply patch 10 - perl misc/cronjobs/overdue_notices.pl -v -t 11 - Chekc the patron's account, 1 message in 'Notices' 12 - Check the DB SELECT * FROM message_queue; 13 - The 'Overdue notices' notice is not doubled 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=23357 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |martin.renvoize@ptfs-europe | |.com --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Code scans well and test plan passes. No obvious regressions, signing off. Thanks Nick -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Nick, what about this change: unless ( $effective_mtt eq 'print' and $print_sent == 1 ) { - if ( ($mtt eq 'email' and not scalar @emails_to_use) or ($mtt eq 'sms' and not $data->{smsalertnumber}) ) { + if ( $effective_mtt eq 'print' and not $print_sent ) { push @output_chunks, It makes the code more readable and less error prone, what do you think? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 100634 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100634&action=edit Bug 23357: [ALTERNATIVE PATH] Do not generate twice the same print notice -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 --- Comment #6 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It avoid to repeat the same test: 683 if ( ($mtt eq 'email' and not scalar @emails_to_use) or ($mtt eq 'sms' and not $data->{smsalertnumber}) ) { 684 # email or sms is requested but not exist, do a print. 685 $effective_mtt = 'print'; 686 } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 --- Comment #7 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 100639 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100639&action=edit Bug 23357: (follow-up) Do not generate twice the same print notice Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100634|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #5)
Created attachment 100634 [details] [review] Bug 23357: [ALTERNATIVE PATH] Do not generate twice the same print notice
This is a followup yes? I applied both and tested, works well, I can't see any issue in the code. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 --- Comment #9 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Situation 1 - Overdue 3 days for Patron: email + print A. No email defined, overdue -t generates * on master, 2 notices OK 1 print for patron 1 email for staff * on 23357 (only first patch) 1 print for patron * on 23357_2 (2 patches) OK 1 print for patron 1 email for staff B. With email defined * on master, 2 notices 1 print for patron 1 email for patron * on 23357 (only first patch) 1 print for patron 1 email for patron * on 23357_2 (2 patches) OK 1 print for patron 1 email for patron 1 email for staff Situation 2 - Overdue 3 days for Patron: email only A. No email defined, overdue -t generates * on master, 2 notices OK 1 print for patron 1 email for staff * on 23357 (only first patch) OK 1 print for patron 1 email for staff * on 23357_2 (2 patches) OK 1 print for patron 1 email for staff B. With email defined * on master, 2 notices 1 email for patron * on 23357 (only first patch) 1 email for patron * on 23357_2 (2 patches) 1 email for patron There is no OK in Situation 2 for B -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Previous comment is not correct, -t does not mean "trigger an email for staff" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 --- Comment #11 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Situation 1 - Overdue 3 days for Patron: email + print A. No email defined, overdue -t generates * on master, 2 notices OK 1 print for patron 1 email for staff * on 23357 (only first patch) 1 print for patron * on 23357_2 (2 patches) OK 1 print for patron 1 email for staff B. With email defined * on master, 2 notices OK 1 print for patron 1 email for patron * on 23357 (only first patch) OK 1 print for patron 1 email for patron * on 23357_2 (2 patches) 1 print for patron 1 email for patron 1 email for staff Situation 2 - Overdue 3 days for Patron: email only A. No email defined, overdue -t generates * on master, 2 notices OK 1 print for patron 1 email for staff * on 23357 (only first patch) OK 1 print for patron 1 email for staff * on 23357_2 (2 patches) OK 1 print for patron 1 email for staff B. With email defined * on master, 2 notices OK 1 email for patron * on 23357 (only first patch) OK 1 email for patron * on 23357_2 (2 patches) OK 1 email for patron -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- So situation 1 does not pass with our patches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |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=23357 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30343 CC| |magnus@libriotech.no -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23357 Magnus Enger <magnus@libriotech.no> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23497 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org