[Bug 30838] New: to_address is misleading for sms transports
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Bug ID: 30838 Summary: to_address is misleading for sms transports Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Notices Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org The SMS sending system relies solely on smsalertnumber, but some notices get queued via C4::Message->enqueue which has a fallback mechanism to populate the to_address field in the message queue. If you then write a report to show all delivery failures, the to_address can be populated with an entirely unrelated number and is very misleading. -- 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=30838 --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135298 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135298&action=edit Bug 30838: Don't fallback through phone numbers for sms This patch removes the fallback handling for smsalartnumber as the to_address in notices. We ignore the to_address field in the message queue at send time for sms anyway and use smsalertnumber exclusively so having this field populated is just confusing to the end user -- 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=30838 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|koha-bugs@lists.koha-commun |martin.renvoize@ptfs-europe |ity.org |.com -- 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=30838 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bernard.scaife@ptfs-europe. | |com, | |katrin.fischer@bsz-bw.de, | |lucy.vaux-harvey@ptfs-europ | |e.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #2 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Test plan, 1) Create a report to list all failed notices SELECT borrowernumber, letter_code, message_transport_type, status, failure_code, time_queued, updated_on, to_address FROM message_queue WHERE date(time_queued) BETWEEN <<Sent BETWEEN (yyyy-mm-dd)|date>> AND <<and (yyyy-mm-dd)|date>> ORDER BY time_queued DESC 2) Add an SMS template for the CHECKIN notice 3) Set a patron to use SMS as their preferred notice transport 4) Ensure the patron does not have smsalertnumber set (but does have another phone number field populated) 5) Trigger the notice 6) Send notices using process_message_queue.pl 7) Confirm you get a failure and that the to_address is populated with a number that's not in the smsalertnumber 8) Apply the patch 9) Repeat and confirm the to_address is no longer populated 10) Confirm that _send_message_by_sms only ever refers to smsalertnumber -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #3 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 135299 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135299&action=edit Bug 30838: Set to_address to smsalertnumber at send It may be helpful to know exactly what number was used for the sms alert that was sent. As such, we should ensure it's set at the time of sending. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Test plan for followup patch.. As above, but now confirm that smsalertnumber should appear in the to_address for the report after running process_messagequeue.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |minor -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Martin Renvoize <martin.renvoize@ptfs-europe.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=30838 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Kyle M Hall <kyle@bywatersolutions.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=30838 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #135298|0 |1 is obsolete| | Attachment #135299|0 |1 is obsolete| | --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 136674 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136674&action=edit Bug 30838: Don't fallback through phone numbers for sms This patch removes the fallback handling for smsalartnumber as the to_address in notices. We ignore the to_address field in the message queue at send time for sms anyway and use smsalertnumber exclusively so having this field populated is just confusing to the end user 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=30838 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 136675 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136675&action=edit Bug 30838: Set to_address to smsalertnumber at send It may be helpful to know exactly what number was used for the sms alert that was sent. As such, we should ensure it's set at the time of sending. 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=30838 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 136676 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136676&action=edit Bug 30838: (QA Follow-up) Add missing semicolon 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=30838 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Can you add a test for this change, please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- The more I read this code, the more funky I find it.. I feel like we could actually get rid of C4::Message entirely.. it's only used in one place and that use feels a bit strange already. Anyways.. I'm working on a unit test here for now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 137202 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=137202&action=edit Bug 30838: (QA follow-up) Add unit tests Two tests added, one in t/db_dependent/Circulation.t to catch the initial setting of to_address at enqueue time and a second in t/db_dependent/Letters.t to catch the correcting at send time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | Version|unspecified |master Summary|to_address is misleading |to_address is misleading |for sms transports |for SMS transports -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Katrin Fischer <katrin.fischer@bsz-bw.de> 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=30838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136674|0 |1 is obsolete| | --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 138025 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138025&action=edit Bug 30838: Don't fallback through phone numbers for sms This patch removes the fallback handling for smsalartnumber as the to_address in notices. We ignore the to_address field in the message queue at send time for sms anyway and use smsalertnumber exclusively so having this field populated is just confusing to the end user Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136675|0 |1 is obsolete| | --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 138026 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138026&action=edit Bug 30838: Set to_address to smsalertnumber at send It may be helpful to know exactly what number was used for the sms alert that was sent. As such, we should ensure it's set at the time of sending. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136676|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 138027 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138027&action=edit Bug 30838: (QA Follow-up) Add missing semicolon Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #137202|0 |1 is obsolete| | --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 138028 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138028&action=edit Bug 30838: (QA follow-up) Add unit tests Two tests added, one in t/db_dependent/Circulation.t to catch the initial setting of to_address at enqueue time and a second in t/db_dependent/Letters.t to catch the correcting at send time. Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think using the smsalertnumber is the correct behaviour, but I am a little worried about someone relying on the fallback. I think because of the behaviour change this might not be for backporting and needs a note? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 138052 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138052&action=edit Bug 30838: (QA follow-up) Add missing semicolon Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #17 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.11.00 |22.11.00, 22.05.05 released in| | Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #18 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 22.05.x for 22.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.11.00, 22.05.05 |22.11.00,21.11, 22.05.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 --- Comment #19 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- Thanks! Pushed to 21.11 for 21.11.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net Status|Pushed to stable |RESOLVED Resolution|--- |FIXED --- Comment #20 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document it seems, marking resolved. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30838 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org