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

feat(Send-SMS): Add the Send SMS feature doc. #227

Merged
merged 2 commits into from Mar 20, 2017
Merged

feat(Send-SMS): Add the Send SMS feature doc. #227

merged 2 commits into from Mar 20, 2017

Conversation

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Feb 27, 2017

No description provided.

@shane-tomlinson shane-tomlinson force-pushed the fxa-53-send-sms branch from 2d6913e to ae28a78 Feb 27, 2017
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Mar 2, 2017

@vbudhram, @philbooth, @davismtl, @rfk - I tag all of you!

###### Request POST data

```
sessionToken,

This comment has been minimized.

@philbooth

philbooth Mar 2, 2017
Contributor

You probably want to remove this line, the session token is in the header not the body. (the original error was mine when making notes in the Google Doc, sorry)


Check whether sending an SMS is enabled for a particular user. Auth-server will determine whether user is in a country to which sending an SMS is enabled as well as whether the SMS provider account has sufficient funds to send a message.

###### Request POST data

This comment has been minimized.

@philbooth

philbooth Mar 2, 2017
Contributor

There's no POST data for this endpoint.


```json
{
ok: <boolean>

This comment has been minimized.

@philbooth

philbooth Mar 2, 2017
Contributor

If I was being pedantic, I'd point out that "ok" should be quoted here.

Copy link
Contributor

@philbooth philbooth left a comment

Tremendous!

Copy link
Contributor

@davismtl davismtl left a comment

Looks good. Re-reading it I had some new questions or identified points we should probably clarify.

<td># of people that click “maybe later”</td>
</tr>
</table>

This comment has been minimized.

@davismtl

davismtl Mar 2, 2017
Contributor

I apparently added a table at this spot after you moved the doc to Github. See: https://docs.google.com/document/d/133_Cu0TlWQZIw-ihMH1s-U_80I12MXZcvHQEuFhWbPg/edit#heading=h.eeevjsod1m6i

I just want to outline how we will track the SMS steps (clicks + installs).

<td>sms.1.sent / flow.sms.submit </td>
</tr>
<tr>
<td>% of people that sign in that have gone through this flow</td>

This comment has been minimized.

@davismtl

davismtl Mar 2, 2017
Contributor

Do we know yet how this will be measured? I think it will be important to determine the priority of phase 3 and 4. But perhaps the mechanism to track this is part of phase 3 where we intend to pass parameters to the app. Let me know your thoughts.

* The "send SMS screen" is displayed
* Success text: "This Firefox is connected"

<img alt="Send SMS screen" src="./image_9.png" width=300 />

This comment has been minimized.

@davismtl

davismtl Mar 2, 2017
Contributor

Just a reminder to add Canada to legal text if all is good on that front.


This test will target:

* USA, Canada have cheap SMS fees via Nexmo.

This comment has been minimized.

@davismtl

davismtl Mar 2, 2017
Contributor

I just want to call out that other users outside of these geo regions won't see this SMS UI. They will be exposed to current user experience.

Also, I want to call out that the SMS will be exposed to desktop users after email confirmation. Mobile users will continue to have the app store buttons displayed to them.

<img alt="Send Firefox to your smartphone screen" src="./image_9.png" width=300 />

* SMS Message: "As requested, here is your link to install Firefox on your mobile device: moz.la/vwxyz “

This comment has been minimized.

@davismtl

davismtl Mar 2, 2017
Contributor

We should call out what the user will see after install. Will they go to registration form directly or will they go through standard onboarding flow of mobile apps?

@rfk
rfk approved these changes Mar 3, 2017

* Measured with SMS provider

* **SMS send rate**: Will confirm if users are adopting new mechanism.

This comment has been minimized.

@rfk

rfk Mar 3, 2017
Member

I think it's worth defining more precisely what this is - is it the number of login flows where an SMS is sent, as a percentage of the number of flows that see the SMS form? Does it map directly to one of the formula listed below?


* A modal dialog appears containing help text that explains why Sync requires two devices.

* Clicking "Got it" or anywhere outside of the modal dialog closes the dialog and returns the user to `/sms`

This comment has been minimized.

@rfk

rfk Mar 3, 2017
Member

Can they also use the esc key?

<td></td>
<td></td>
</tr>
</table>

This comment has been minimized.

@rfk

rfk Mar 3, 2017
Member

❤️ having this ready to populate!

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Mar 6, 2017

from mtg: needs final review and merge...


A new check is needed on the customs server to limit the number of SMS messages sent by a single IP address or to a single phone number.

TODO - Do the fxa-customs-server docs need to be updated? https://github.com/mozilla/fxa-customs-server/blob/master/docs/api.md

This comment has been minimized.

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Mar 20, 2017

remove the TODO - Do the fxa-customs-server docs need to be updated? and merge! I'll merge

@vladikoff vladikoff self-assigned this Mar 20, 2017
@vladikoff vladikoff merged commit 24e815c into master Mar 20, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@vladikoff vladikoff deleted the fxa-53-send-sms branch Mar 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants