[Bug 30026] New: International Phone Number Not supported for SMSAlert
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30026 Bug ID: 30026 Summary: International Phone Number Not supported for SMSAlert Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: florian.bontemps@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com The SMSNumber field used when registering a phone number for SMS alerts does not work with some international number formats. Currently, the format 00 AAAXXXXXXXXX (A = country indicator between 1 and 3 digits, X = regular digits) is not supported. While the format +AAAXXXXXXXXX can sometimes work, some network providers don't accept the latter format. These formats are very used in Europe for instance (Germany, Sweden, France, etc) -- 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=30026 --- Comment #1 from Florian <florian.bontemps@biblibre.com> --- Created attachment 130167 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130167&action=edit Bug 30026: SMSAlert regex fixed for international When registering a phone number for SMS alerts, the number goes through a regex filter to make sure the number is valid. One of the formats used in Europe is 00 AAA XXXXXXXXX (A = country indicator) and it is not supported. This patch adjusts the regex to either accept 00 AAA XXXXXXXXX or +AAAXXXXXXXXX in this case. (Note : +AAA and 00AAA are mutually exclusive). To test: 1) You need to have installed a SMSAlert plugin. Make sure the SMSSendDriver syspref is also set up with your SMS plugin of choice. 2) Pick a patron and edit its profile. 3) In the messaging preferences fieldset, under SMS number, enter any number with the format 00 AAA XXXXXXXXX (ie : 00111123456789). Try to save the modification : you should have an error message. 4) Try instead a number with the format +AAAXXXXXXXXX (ie : 111123456789), that one should be accepted. 5) Save your modification then check that the number has been changed. 6) Apply patch. 7) Repeat step 3 to 5. 8) Observe the error is gone. 9) Try it once more with the format +00 AAA XXXXXXXXX (+00111123456789), that format should not be accepted: this is intentional. 10) Sign off. Thanks-to: Didier Gautheron <didier.gautheron@biblibre.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=30026 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion CC| |dcook@prosentient.com.au --- Comment #2 from David Cook <dcook@prosentient.com.au> --- That's interesting. So that 00 is an exit code which is very country specific, so I don't think this patch will work in all contexts. While I wouldn't call it Euro-centric per se, since 00 is used elsewhere, it doesn't cover all international use cases. For instance, the exit code for Australia is 0011 (instead of 00). Canada/USA is 011. Russia is 810. Some places like Israel also use carrier codes instead of exit codes. Perhaps we should have a system preference that allows a library to specify 1+ exit codes to use in lieu of a "+" for the regex. -- 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=30026 --- Comment #3 from David Cook <dcook@prosentient.com.au> --- Since the exit code is specific to the dialler rather than the recipient maybe the data input isn't the right place to check this either. Maybe the SMSAlert plugin should check if the phone number has a "+" prefix and if not... then add an exit code? Is the use case that the library would be SMSing mobiles outside their own national area? -- 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=30026 --- Comment #4 from Florian <florian.bontemps@biblibre.com> --- I did take a look at other countries' exit code to make sure none would be penalized by this regex, and from what I could find online, all of your examples would still work. The regex isn't exactly foolproof but it should be flexible enough to allow for all valid phones numbers. I did consider the alternative of switching the SMSAlert input field from a text to a tel, since it seems to naturally check for valid phone numbers.
That's interesting.
So that 00 is an exit code which is very country specific, so I don't think this patch will work in all contexts. While I wouldn't call it Euro-centric per se, since 00 is used elsewhere, it doesn't cover all international use cases.
For instance, the exit code for Australia is 0011 (instead of 00). Canada/USA is 011. Russia is 810. Some places like Israel also use carrier codes instead of exit codes.
Perhaps we should have a system preference that allows a library to specify 1+ exit codes to use in lieu of a "+" for the regex.
-- 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=30026 --- Comment #5 from Florian <florian.bontemps@biblibre.com> --- (In reply to David Cook from comment #3)
Since the exit code is specific to the dialler rather than the recipient maybe the data input isn't the right place to check this either.
Maybe the SMSAlert plugin should check if the phone number has a "+" prefix and if not... then add an exit code?
Is the use case that the library would be SMSing mobiles outside their own national area?
It seems to me that forcing the addition of an exit code might cause a lot more problems down the line, since you would need to check if, for instance, a number starting with 011 is for the USA/Canada or just a local territorial number in your country. We do have some libraries that are located next to country borders, and it's not uncommon for residents of these areas to use two phones or sim cards, one for each side of the border. -- 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=30026 --- Comment #6 from David Cook <dcook@prosentient.com.au> --- (In reply to Florian from comment #5)
It seems to me that forcing the addition of an exit code might cause a lot more problems down the line
100% agreed. I think for many/most libraries, they'll never want to have exit codes included. I still don't see the utility of adding exit codes. It sounds like it's something you'd need for your plugin, but it seems to me that is something you could add via your plugin and doesn't need to be in Koha. -- 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=30026 --- Comment #7 from Florian <florian.bontemps@biblibre.com> --- (In reply to David Cook from comment #6)
(In reply to Florian from comment #5)
It seems to me that forcing the addition of an exit code might cause a lot more problems down the line
100% agreed. I think for many/most libraries, they'll never want to have exit codes included.
I still don't see the utility of adding exit codes. It sounds like it's something you'd need for your plugin, but it seems to me that is something you could add via your plugin and doesn't need to be in Koha.
But it's already there in the code. The current version already supports vast majority of exit codes, this patch aims to make sure we support even more. -- 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=30026 --- Comment #8 from David Cook <dcook@prosentient.com.au> --- (In reply to Florian from comment #7)
But it's already there in the code. The current version already supports vast majority of exit codes, this patch aims to make sure we support even more.
I'm not sure if you're referring to Koha or your Koha plugin here. If it's Koha, no it doesn't? Anyway, that's just my 2 cents. Testers and QA might think differently. -- 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=30026 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff Summary|International Phone Number |International phone number |Not supported for SMSAlert |not supported for sending | |SMS --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- +49 or 0049 are equivalent here and both are accepted for example when adding contacts in my phone. I don't see the issue in making the regex for the SMS number accept more valid combinations. -- 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=30026 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |florian.bontemps@biblibre.c |ity.org |om --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am putting this back in the queue to get more opinions. I am ok with this change, but if we do it, we should adapt the hint below the field too. It currently reads: Please enter numbers only. Prefix the number with + if including the country code. -- 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=30026 --- Comment #11 from Florian <florian.bontemps@biblibre.com> --- (In reply to Katrin Fischer from comment #10)
I am putting this back in the queue to get more opinions.
I am ok with this change, but if we do it, we should adapt the hint below the field too. It currently reads:
Please enter numbers only. Prefix the number with + if including the country code.
Good catch! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30026 --- Comment #12 from Florian <florian.bontemps@biblibre.com> --- Created attachment 131375 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=131375&action=edit Bug 30026: Quick fix for SmsAlert Hint Just modifying the hint provided in patrons' messaging preferences to allow either + or 00 for international formats. No test plan needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30026 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch 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=30026 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130167|0 |1 is obsolete| | Attachment #131375|0 |1 is obsolete| | --- Comment #13 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 134756 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134756&action=edit Bug 30026: SMSAlert regex fixed for international When registering a phone number for SMS alerts, the number goes through a regex filter to make sure the number is valid. One of the formats used in Europe is 00 AAA XXXXXXXXX (A = country indicator) and it is not supported. This patch adjusts the regex to either accept 00 AAA XXXXXXXXX or +AAAXXXXXXXXX in this case. (Note : +AAA and 00AAA are mutually exclusive). To test: 1) You need to have installed a SMSAlert plugin. Make sure the SMSSendDriver syspref is also set up with your SMS plugin of choice. 2) Pick a patron and edit its profile. 3) In the messaging preferences fieldset, under SMS number, enter any number with the format 00 AAA XXXXXXXXX (ie : 00111123456789). Try to save the modification : you should have an error message. 4) Try instead a number with the format +AAAXXXXXXXXX (ie : 111123456789), that one should be accepted. 5) Save your modification then check that the number has been changed. 6) Apply patch. 7) Repeat step 3 to 5. 8) Observe the error is gone. 9) Try it once more with the format +00 AAA XXXXXXXXX (+00111123456789), that format should not be accepted: this is intentional. 10) Sign off. Thanks-to: Didier Gautheron <didier.gautheron@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30026 --- Comment #14 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 134757 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134757&action=edit Bug 30026: Quick fix for SmsAlert Hint Just modifying the hint provided in patrons' messaging preferences to allow either + or 00 for international formats. No test plan needed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30026 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=30026 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134756|0 |1 is obsolete| | --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 134954 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134954&action=edit Bug 30026: SMSAlert regex fixed for international When registering a phone number for SMS alerts, the number goes through a regex filter to make sure the number is valid. One of the formats used in Europe is 00 AAA XXXXXXXXX (A = country indicator) and it is not supported. This patch adjusts the regex to either accept 00 AAA XXXXXXXXX or +AAAXXXXXXXXX in this case. (Note : +AAA and 00AAA are mutually exclusive). To test: 1) You need to have installed a SMSAlert plugin. Make sure the SMSSendDriver syspref is also set up with your SMS plugin of choice. 2) Pick a patron and edit its profile. 3) In the messaging preferences fieldset, under SMS number, enter any number with the format 00 AAA XXXXXXXXX (ie : 00111123456789). Try to save the modification : you should have an error message. 4) Try instead a number with the format +AAAXXXXXXXXX (ie : 111123456789), that one should be accepted. 5) Save your modification then check that the number has been changed. 6) Apply patch. 7) Repeat step 3 to 5. 8) Observe the error is gone. 9) Try it once more with the format +00 AAA XXXXXXXXX (+00111123456789), that format should not be accepted: this is intentional. 10) Sign off. Thanks-to: Didier Gautheron <didier.gautheron@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> 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=30026 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134757|0 |1 is obsolete| | --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 134955 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134955&action=edit Bug 30026: Quick fix for SmsAlert Hint Just modifying the hint provided in patrons' messaging preferences to allow either + or 00 for international formats. No test plan needed. Signed-off-by: Owen Leonard <oleonard@myacpl.org> 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=30026 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think this is in a bit of grey area between enh and bug. As it modifies strings in the OPAC, I think it would be ok to push after release and then backport. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30026 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=30026 --- Comment #18 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=30026 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|22.11.00 |22.11.00, 22.05.03 released in| | Status|Pushed to master |Pushed to stable CC| |lucas@bywatersolutions.com --- Comment #19 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 22.05.x for 22.05.03 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30026 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25520 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org