Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Location details page is not displaying certain phone number attributes #415

Open
1 of 2 tasks
monfresh opened this issue May 21, 2014 · 8 comments · May be fixed by #683
Open
1 of 2 tasks

Location details page is not displaying certain phone number attributes #415

monfresh opened this issue May 21, 2014 · 8 comments · May be fixed by #683

Comments

@monfresh
Copy link
Member

There are a few phone attributes that are not being displayed on the details page.

  • extension
  • vanity_number
@monfresh
Copy link
Member Author

monfresh commented Sep 7, 2014

Extension has been added, but not vanity_number. Here is a location that should have its vanity number displayed: http://ohana-web-search-demo.herokuapp.com/locations/admin-test-org/admin-test-location

@monfresh monfresh added this to the Knight outputs milestone Sep 7, 2014
@anselmbradford
Copy link
Member

What's up with the languages spoken value on http://ohana-web-search-demo.herokuapp.com/locations/admin-test-org/admin-test-location? Is that an Ohana Web Search bug?

@anselmbradford
Copy link
Member

Before this is implemented in the view format_phone in app/helpers/detail_format_helper needs to be updated to handle vanity numbers, or a new helper needs to be created to handle them (@monfresh, preference on combined vs separate helpers for formatting the numbers?)

@anselmbradford
Copy link
Member

(Referencing #172)

@monfresh
Copy link
Member Author

Rails provides a number_to_phone formatter, so we don't have to write our own. It works best when the input only contains digits, so we can either ensure that format on the client side, or update the API app to either enforce that format, or have the API make the transformation.

To achieve the format we have now, you would do something like this:

number_to_phone(phone.number.gsub(/[^\d]/, ''), area_code: true)

Vanity numbers cannot be handled because they are not formatted consistently. They usually contain words of varying length that are meant to be together. For example, you would not want to format 800-FLOWERS into (800) FLO-WERS. There is no way of knowing in advance what letters are in the vanity number, so it would be futile to try to come up with a formatter that works for all of them.

So, as far as this issue is concerned, simply adding HTML to display the vanity number as it is returned by the API is what's needed.

@anselmbradford
Copy link
Member

Rails provides a number_to_phone formatter, so we don't have to write our own. It works best when the input only contains digits, so we can either ensure that format on the client side, or update the API app to either enforce that format, or have the API make the transformation.

Are you saying this should be used in place of https://github.com/codeforamerica/ohana-web-search/blob/master/app/helpers/detail_format_helper.rb#L26 and that helper should be removed? (Perhaps in this PR.)

Vanity numbers cannot be handled because they are not formatted consistently. They usually contain words of varying length that are meant to be together. For example, you would not want to format 800-FLOWERS into (800) FLO-WERS. There is no way of knowing in advance what letters are in the vanity number, so it would be futile to try to come up with a formatter that works for all of them.

Ah yes, makes sense. Is there any consistency in the area code? That would always be digits, right? The example vanity number you referenced earlier looks like 703555-ABCD, it would be great to at least format the area code like (703) 555-ABCD. So then if there was an entry that had 800FLOWERS, it would become (800) FLOWERS.

Right now the demo entry looks like:

703555-ABCD 
(703) 555-1212 x1223
CalFresh

Without the area code formatting its confusing to even tell that first number is a phone number because it's formatted to look like a serial number or something.

@anselmbradford
Copy link
Member

Hm, I guess the area code wouldn't HAVE to be digits. But I think this could still be formatted. For example given GETFLOWERS a basic formatter could format it (GET) FLO-WERS but it doesn't seem too terribly complicated to also have a check that:

  • if a letter appears on both sides of the hyphen, don't include the hyphen (GET) FLOWERS
  • if a letter appears on both sides of the ), don't include the parentheses and space, ending up with GETFLOWERS

@monfresh
Copy link
Member Author

Let's not overthink this. People should be free to format phone numbers however they want, so yes, we should replace our current helper with the one Rails provides. That way, if one instance wants to format the phones like 703.555.1212, all they have to do is specify the delimiter, like this: number_to_phone(phone.number, delimiter: '.'). If people want to enforce the same or a separate style for vanity numbers, they can do that in the admin interface.

703555-ABCD is not how a typical vanity number would be formatted. I added that manually a while back, but I updated it earlier today to 800-45NOFUME, which is an actual vanity number in SMC.

anselmbradford added a commit that referenced this issue Nov 14, 2014
Fixes #415 - adds vanity number to phone partial and test spec.
@anselmbradford anselmbradford linked a pull request Nov 14, 2014 that will close this issue
anselmbradford added a commit that referenced this issue Nov 14, 2014
Fixes #415 - adds vanity number to phone partial and test spec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants