TPAC: System should disallow patrons from entering SMS number in wrong format

Bug #1016654 reported by Kathy Lussier
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Low
Unassigned
2.3
Fix Released
Low
Unassigned

Bug Description

Evergreen version: 2.2 RC1

Wishlist item

Despite the hint telling users not to use hyphens when entering an text notify number, we have patrons who are entering the hyphens. The text notice subsequently fails. We would like some kind of check to prevent the user from entering the number in the wrong format.

Revision history for this message
Thomas Berezansky (tsbere) wrote :

The only way I could see implementing this is as a per-service regex. Even then I would be wary of being too strict with it.

For example, I could, in theory, use my "nickname" instead of my cell number and provided that it was stuck in front of the correct @domain I would get a text message.

Revision history for this message
Kathy Lussier (klussier) wrote :

I could see where using a nickname would be convenient for some power users, but I suspect there are fewer users looking to use a nickname than there are users who will overlook the hint and enter their SMS number in a format that will fail.

Giving it further thought, I suppose there are a couple of approaches that could be taken:

1. If the patron enters the SMS number in the wrong format, the system returns an error asking them to enter it in the correct format.

2. If the patron enters the SMS number in the wrong format, the system automatically removes any hyphens in the number (much more user friendly than option 1).

3. The text entry box only allows the patron to enter 10 characters. Once the patron notices that they aren't able to complete the entry, they re-read the screen and see what the correct format should be (or complain to their local library staff that the form doesn't work.)

4. The form provides three boxes, one for area code (limited to 3 characters), one for local exchange (limited to 3 characters), and one for the number (limited to 4 characters.) The system then takes these entries and enters it as one SMS number.

However, I don't think any of these options would allow patrons to use a nickname.

Revision history for this message
Michael Peters (mrpeters) wrote :

We've had a couple of complaints about this not "validating" like "the other phone fields" do since upgrading to 2.2. I guess people just don't see the hint. I tend to side with Kathy on this. We're more likely for people to use their phone number than some kind of alias, but I'm sure we can come up with some kind of compromise that makes things work for (nearly) everyone.

Changed in evergreen:
status: New → Confirmed
importance: Undecided → Wishlist
importance: Wishlist → Low
Revision history for this message
Thomas Berezansky (tsbere) wrote :

On the front of the listed options in comment #2:

#1 would work for per-service regex type deals fairly easily.

#2 could apply as well, but I still think a per-service config is needed, if only to validate that the correct number of digits was provided.

#3 and #4 are not portable outside of NANP numbers and I thus disagree with. We can't assume that Evergreen will only ever be used in the US and Canada type deal.

Revision history for this message
Michael Peters (mrpeters) wrote :

If someone whips up an internal solution to this, I'd enjoy seeing it. I'm comfortable saying Evergreen Indiana will never get used by someone with one of these non-US/Canada exceptions and willing to risk that in order to gain more reliable service for patrons who don't see the helper.

Revision history for this message
Michael Peters (mrpeters) wrote :
Revision history for this message
Thomas Berezansky (tsbere) wrote :

I believe something more robust (aka, server-side) is needed as we shouldn't be relying on JavaScript for input validation. Well, inside the staff client where we know it is turned on perhaps, but relying on it outside of the staff client seems problematic.

Revision history for this message
Michael Peters (mrpeters) wrote :

Maybe this should become a Wishlist item, then. This, at least, gets a working solution in place in the short term if anyone happens upon this thread. Yes, it can be modified to "break the script rules" but at least for 99% of the users it's just an enhancement to the usability of the field. Of course, it could be expanded (change colors on invalid input, pop up a message, etc.) but this is a quick and easy short term solution.

Revision history for this message
Thomas Berezansky (tsbere) wrote :

An alternative to client-side munging:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/tsbere/sms_munge

That branch says "If it looks to be all digits, spaces, hyphens, and parentheses then strip it down to just digits. Otherwise assume that it is some kind of alias the service supports". It applies at the A/T layer and thus requires no DB updates, and makes no assumptions about lengths of numbers.

Revision history for this message
Michael Peters (mrpeters) wrote :
tags: added: pullrequest
Ben Shum (bshum)
Changed in evergreen:
milestone: none → 2.4.0-alpha
Revision history for this message
Ben Shum (bshum) wrote :

Picked fix to master and rel_2_3. SMS being broken seemed more like a bug than a brand new feature to me.

The wording contained in the sms_helper commit was MIA from the link indicated, but I checked the working repo for it.

I'm not sure how to handle translation effects on the entry. Firstly, we probably shouldn't alter it in 2.3 anymore, but for master we could. Though should the two sentences be broken up separately translated? Or as that single line? And do we really have to update the POs or does that happen elsewhere in the process? Hmm...

Changed in evergreen:
status: Confirmed → Fix Committed
Revision history for this message
Ben Shum (bshum) wrote :

Oh, for those who go to look, this is the working branch that mrpeters started for updating the helper text:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mrpeters-isl/sms_helper

Ben Shum (bshum)
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.