Bug 22862 - It should be possible to paste formatted phone numbers into the SMS messaging number field
Summary: It should be possible to paste formatted phone numbers into the SMS messaging...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: About (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 22594
Blocks: 23901
  Show dependency treegraph
 
Reported: 2019-05-07 18:52 UTC by Kyle M Hall
Modified: 2020-06-04 20:32 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This bugfix improves the likelihood of pasted patron phone numbers passing validation as we will now attempt to normalise out illegal characters often used to human-friendly formatting.
Version(s) released in:
19.11.00,19.05.03


Attachments
Bug 22862 - Normalize SMS messaging numbers before validating them (2.67 KB, patch)
2019-05-08 10:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22862: Normalize SMS messaging numbers before validating them (2.71 KB, patch)
2019-05-08 14:53 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22862: (QA follow-up) Make regexes match (923 bytes, patch)
2019-05-09 18:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 22862: Normalize SMS messaging numbers before validating them (2.77 KB, patch)
2019-05-11 12:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22862: (QA follow-up) Make regexes match (964 bytes, patch)
2019-05-11 12:19 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2019-05-07 18:52:09 UTC
Librarians often copy and paste patron data, including phone numbers. SMS phone numbers are now being validated to conform to the E.164 specification. It would be nice to try to normalize that data by stripping non-numeric data from the paste (i.e. dashes, parens, etc ).
Comment 1 Kyle M Hall 2019-05-08 10:47:27 UTC
Created attachment 89458 [details] [review]
Bug 22862 - Normalize SMS messaging numbers before validating them

Librarians often copy and paste patron data, including phone numbers. SMS phone numbers are now being validated to conform to the E.164 specification. It would be nice to try to normalize that data by stripping non-numeric data from the paste (i.e. dashes, parens, etc ).

Test Plan:
1) Apply this patch
2) On the staff side, Attempt to enter invalid characters the SMS number field
3) Note you cannot enter invalid characters
4) Attempt to paste a phone number with invalid characters
5) Note those characters are removed on paste
6) Repeat these steps on the OPAC
Comment 2 Liz Rea 2019-05-08 14:53:25 UTC
Created attachment 89474 [details] [review]
Bug 22862: Normalize SMS messaging numbers before validating them

Librarians often copy and paste patron data, including phone numbers. SMS phone numbers are now being validated to conform to the E.164 specification. It would be nice to try to normalize that data by stripping non-numeric data from the paste (i.e. dashes, parens, etc ).

Test Plan:
1) Apply this patch
2) On the staff side, Attempt to enter invalid characters the SMS number field
3) Note you cannot enter invalid characters
4) Attempt to paste a phone number with invalid characters
5) Note those characters are removed on paste
6) Repeat these steps on the OPAC

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 3 Jonathan Druart 2019-05-08 19:51:30 UTC
A couple of things:
1. regexs are different
2. There is the 'pattern' attribute that should deal with that (opac-messaging.tt), can you explain why we need the JS code as well?
Comment 4 Kyle M Hall 2019-05-09 18:10:14 UTC
(In reply to Jonathan Druart from comment #3)
> A couple of things:
> 1. regexs are different

I'll get that fixed. They are functionally identical, but it would be good to have them patch.

> 2. There is the 'pattern' attribute that should deal with that
> (opac-messaging.tt), can you explain why we need the JS code as well?

The pattern attribute is for data validation, not data normalization.
Comment 5 Kyle M Hall 2019-05-09 18:14:44 UTC
Created attachment 89522 [details] [review]
Bug 22862: (QA follow-up) Make regexes match
Comment 6 Katrin Fischer 2019-05-11 12:18:27 UTC
Could we maybe trigger the validation in a less 'intrusive' way? At the moment it will immediately delete a space entered, which is a little irritating maybe when you are still entering the data.

I am not sure what would work better here for a trigger and I think this is also helpful as is, so passing QA.
Comment 7 Katrin Fischer 2019-05-11 12:19:11 UTC
Created attachment 89597 [details] [review]
Bug 22862: Normalize SMS messaging numbers before validating them

Librarians often copy and paste patron data, including phone numbers. SMS phone numbers are now being validated to conform to the E.164 specification. It would be nice to try to normalize that data by stripping non-numeric data from the paste (i.e. dashes, parens, etc ).

Test Plan:
1) Apply this patch
2) On the staff side, Attempt to enter invalid characters the SMS number field
3) Note you cannot enter invalid characters
4) Attempt to paste a phone number with invalid characters
5) Note those characters are removed on paste
6) Repeat these steps on the OPAC

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Katrin Fischer 2019-05-11 12:19:15 UTC
Created attachment 89598 [details] [review]
Bug 22862: (QA follow-up) Make regexes match

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Martin Renvoize 2019-06-24 14:48:00 UTC
Nice work!

Pushed to master for 19.11.00
Comment 10 Fridolin Somers 2019-07-24 07:15:56 UTC
Pushed to 19.05.x for 19.05.03
Comment 11 Lucas Gass 2019-07-25 20:37:32 UTC
lacking dependencies for 18.11.x, no backport