Bug 26285 - Use country code + number (E.164) validation for SMS numbers
Summary: Use country code + number (E.164) validation for SMS numbers
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: David Cook
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 22594
Blocks:
  Show dependency treegraph
 
Reported: 2020-08-24 00:49 UTC by David Cook
Modified: 2020-09-09 13:40 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 26285: Follow E.164 pattern for validating SMS numbers (2.28 KB, patch)
2020-08-24 06:59 UTC, David Cook
Details | Diff | Splinter Review
Bug 26285: Follow E.164 pattern for validating SMS numbers (2.35 KB, patch)
2020-08-24 10:16 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 26285: Follow E.164 pattern for validating SMS numbers (2.40 KB, patch)
2020-08-24 20:01 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 26285: Follow E.164 pattern for OPAC too (1.80 KB, patch)
2020-08-26 04:07 UTC, David Cook
Details | Diff | Splinter Review
Bug 26285: Follow E.164 pattern for validating SMS numbers (2.47 KB, patch)
2020-09-02 14:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26285: Follow E.164 pattern for OPAC too (1.86 KB, patch)
2020-09-02 14:34 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-08-24 00:49:51 UTC
According to the E.164 standard, a phone number must be a maximum of 12 digits plus a 1-3 digit country code for a maximum of 15 digits. 

For North America, that would look like +1 306 555 5555 (11 total digits) or 306 555 5555 (10 digits).

For Australian phone numbers, that looks like +61 2 5555 5555 (11 total digits) or 02 5555 5555 (10 digits).

Unfortunately, since Bug 22594, Australian phone numbers without a country code aren't validating because they start with a 0. 


Further reading:
https://en.wikipedia.org/wiki/E.164
https://www.itu.int/rec/T-REC-E.164/
https://en.wikipedia.org/wiki/List_of_country_calling_codes
Comment 1 David Cook 2020-08-24 06:59:31 UTC
Created attachment 108964 [details] [review]
Bug 26285: Follow E.164 pattern for validating SMS numbers

Currently, Koha is not correctly validating SMS numbers using
E.164. This causes Australian phone numbers without a country code
to fail validation.

This patch uses the E.164 pattern of 1-3 digits for country code,
prefixed by a + symbol, and followed by up to 12 digits for
the remainder of the phone number.

To test:

0. Don't apply patch yet
1. Set "SMSSendDriver" to "Anything"
2. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
3. Type in 0455555555 and note "Please enter a valid phone number." message in browser

6. Apply the patch

7. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
8. Type in 0455555555 and note no validation error
9. Type in +61455555555 and note no validation error
10. Type in 123456789012 and note no validation error
11. Type in 1234567890123 and note "Please enter a valid phone number." error
12. Type in +900123456789012 and note no validation error
13. Type in 900123456789012 and note "Please enter a valid phone number." error
Comment 2 ByWater Sandboxes 2020-08-24 10:16:43 UTC
Created attachment 108979 [details] [review]
Bug 26285: Follow E.164 pattern for validating SMS numbers

Currently, Koha is not correctly validating SMS numbers using
E.164. This causes Australian phone numbers without a country code
to fail validation.

This patch uses the E.164 pattern of 1-3 digits for country code,
prefixed by a + symbol, and followed by up to 12 digits for
the remainder of the phone number.

To test:

0. Don't apply patch yet
1. Set "SMSSendDriver" to "Anything"
2. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
3. Type in 0455555555 and note "Please enter a valid phone number." message in browser

6. Apply the patch

7. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
8. Type in 0455555555 and note no validation error
9. Type in +61455555555 and note no validation error
10. Type in 123456789012 and note no validation error
11. Type in 1234567890123 and note "Please enter a valid phone number." error
12. Type in +900123456789012 and note no validation error
13. Type in 900123456789012 and note "Please enter a valid phone number." error

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 3 Sally 2020-08-24 10:17:48 UTC
This is valid for UK patrons too - our mobile numbers also start with zero. 

Test plan works, signed off!
Comment 4 Katrin Fischer 2020-08-24 20:01:24 UTC
Created attachment 109034 [details] [review]
Bug 26285: Follow E.164 pattern for validating SMS numbers

Currently, Koha is not correctly validating SMS numbers using
E.164. This causes Australian phone numbers without a country code
to fail validation.

This patch uses the E.164 pattern of 1-3 digits for country code,
prefixed by a + symbol, and followed by up to 12 digits for
the remainder of the phone number.

To test:

0. Don't apply patch yet
1. Set "SMSSendDriver" to "Anything"
2. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
3. Type in 0455555555 and note "Please enter a valid phone number." message in browser

6. Apply the patch

7. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
8. Type in 0455555555 and note no validation error
9. Type in +61455555555 and note no validation error
10. Type in 123456789012 and note no validation error
11. Type in 1234567890123 and note "Please enter a valid phone number." error
12. Type in +900123456789012 and note no validation error
13. Type in 900123456789012 and note "Please enter a valid phone number." error

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 David Cook 2020-08-25 03:43:39 UTC
(In reply to Sally from comment #3)
> This is valid for UK patrons too - our mobile numbers also start with zero. 
> 
> Test plan works, signed off!

Thanks Sally (and Katrin)!
Comment 6 Jonathan Druart 2020-08-25 09:49:07 UTC
What about the OPAC?
Bug 22594 modified opac-messaging with the same regex:

135                                     <label for="SMSnumber">SMS number:</label> <input type="text" id="SMSnumber" name="SMSnumber" value="[% SMSnumber | html %]" pattern="^\+?[1-9]\d{1,14}$" />
Comment 7 Katrin Fischer 2020-08-25 10:01:38 UTC
(In reply to Jonathan Druart from comment #6)
> What about the OPAC?
> Bug 22594 modified opac-messaging with the same regex:
> 
> 135                                     <label for="SMSnumber">SMS
> number:</label> <input type="text" id="SMSnumber" name="SMSnumber" value="[%
> SMSnumber | html %]" pattern="^\+?[1-9]\d{1,14}$" />

Hah! I checked the OPAC but expected it to use the same names... wrong!
Comment 8 David Cook 2020-08-26 01:11:21 UTC
Ah, thanks for that one, Jonathan. I'll look at updating this later today.

Didn't even notice that the same code isn't re-used between interfaces. Have to love having more than 1 way to do the same thing...
Comment 9 David Cook 2020-08-26 04:07:13 UTC
Created attachment 109135 [details] [review]
Bug 26285: Follow E.164 pattern for OPAC too

This patch uses the E.164 pattern for phone numbers
for the OPAC (and not just the staff interface).

To test:
0) Follow the same plan as the staff interface, but
go to /cgi-bin/koha/opac-messaging.pl after logging
into the OPAC
Comment 10 Martin Renvoize 2020-09-02 14:34:01 UTC
Created attachment 109542 [details] [review]
Bug 26285: Follow E.164 pattern for validating SMS numbers

Currently, Koha is not correctly validating SMS numbers using
E.164. This causes Australian phone numbers without a country code
to fail validation.

This patch uses the E.164 pattern of 1-3 digits for country code,
prefixed by a + symbol, and followed by up to 12 digits for
the remainder of the phone number.

To test:

0. Don't apply patch yet
1. Set "SMSSendDriver" to "Anything"
2. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
3. Type in 0455555555 and note "Please enter a valid phone number." message in browser

6. Apply the patch

7. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl?op=modify&destination=circ&borrowernumber=51
8. Type in 0455555555 and note no validation error
9. Type in +61455555555 and note no validation error
10. Type in 123456789012 and note no validation error
11. Type in 1234567890123 and note "Please enter a valid phone number." error
12. Type in +900123456789012 and note no validation error
13. Type in 900123456789012 and note "Please enter a valid phone number." error

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2020-09-02 14:34:04 UTC
Created attachment 109543 [details] [review]
Bug 26285: Follow E.164 pattern for OPAC too

This patch uses the E.164 pattern for phone numbers
for the OPAC (and not just the staff interface).

To test:
0) Follow the same plan as the staff interface, but
go to /cgi-bin/koha/opac-messaging.pl after logging
into the OPAC

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2020-09-02 14:34:46 UTC
Works as expected for me.. and I believe is now consistent OPAC and Staff side.

Passing QA
Comment 13 David Cook 2020-09-03 00:02:04 UTC
(In reply to Martin Renvoize from comment #12)
> Works as expected for me.. and I believe is now consistent OPAC and Staff
> side.
> 
> Passing QA

Cheers Martin :D
Comment 14 Jonathan Druart 2020-09-09 13:40:43 UTC
Pushed to master for 20.11, thanks to everybody involved!