Skip to content
This repository has been archived by the owner. It is now read-only.

feat(sms): Allow the user to resend and go "back" from /sms/sent #4777

Merged
merged 3 commits into from Mar 10, 2017

Conversation

@shane-tomlinson
Copy link
Member

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

No description provided.

@shane-tomlinson shane-tomlinson changed the title feat(sms): Allow the user to resend and go "back" from /sms/sent WIP feat(sms): Allow the user to resend and go "back" from /sms/sent Mar 2, 2017
@shane-tomlinson shane-tomlinson self-assigned this Mar 2, 2017
@shane-tomlinson shane-tomlinson force-pushed the send-sms-back-from-sms-sent branch from d343db6 to 65faccf Mar 2, 2017
@shane-tomlinson shane-tomlinson changed the title WIP feat(sms): Allow the user to resend and go "back" from /sms/sent [WIP] feat(sms): Allow the user to resend and go "back" from /sms/sent Mar 2, 2017
@shane-tomlinson shane-tomlinson force-pushed the send-sms-back-from-sms-sent branch 2 times, most recently from 367e1c5 to 4083d9c Mar 2, 2017
@shane-tomlinson shane-tomlinson removed their assignment Mar 7, 2017
@shane-tomlinson shane-tomlinson changed the title [WIP] feat(sms): Allow the user to resend and go "back" from /sms/sent feat(sms): Allow the user to resend and go "back" from /sms/sent Mar 7, 2017
@shane-tomlinson
Copy link
Member Author

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

I can call this ready for review!

@mozilla/fxa-devs - r?

<section>
<div class="success visible">
{{#t}}App link sent to %(phoneNumber)s.{{/t}} <a id="back" href="#">{{#t}}Mistyped number?{{/t}}</a>

This comment has been minimized.

@vladikoff

vladikoff Mar 7, 2017
Contributor

This should be good for l10n and rtl I hope!

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Mar 9, 2017
Author Member

I'll merge the two strings.

Shane Tomlinson added 2 commits Feb 21, 2017
Combine `App link sent to %(phoneNumber)s` and
`Mistyped email?` for better l10n.
@shane-tomlinson shane-tomlinson force-pushed the send-sms-back-from-sms-sent branch from 4083d9c to 4f0c858 Mar 9, 2017
@shane-tomlinson
Copy link
Member Author

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

@vladikoff - updated, mind an r?

Copy link
Contributor

@vbudhram vbudhram left a comment

@shane-tomlinson LGTM 👍 , merge at will

screen shot 2017-03-09 at 4 02 23 pm

.then(testElementExists(SELECTOR_SMS_SENT_HEADER))

// Give a slight delay or else nexmo throttles the request
.sleep(10000)

This comment has been minimized.

@vbudhram

vbudhram Mar 9, 2017
Contributor

10 seconds seem really long for a delay?

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Mar 9, 2017
Author Member

Yeah, that's Nexmo though. I was thinking for the functional tests, maybe we could open the content server page with a query parameter that causes the /sms page to not actually send the request, but instead just go to the next screen to make sure the UI acts as we expect. Maybe we'd send 1 or 2 requests, 1 to check whether backend phone number validation works, 1 to check whether SMS send is successful.

This comment has been minimized.

@vbudhram

vbudhram Mar 9, 2017
Contributor

That sounds good. Doesn't have to go in this PR, just my spider sense going off for when I saw 10seconds.

}
const t = msg => msg;

const UNTRANSLATED_RESENT_MESSAGE = t('Download link resent to %(phoneNumber)s');

This comment has been minimized.

@shane-tomlinson

shane-tomlinson Mar 9, 2017
Author Member

@ryanfeeley asked me to change the other text from Download link to App link, we should do the same here.

@shane-tomlinson shane-tomlinson merged commit 8a2c5e7 into master Mar 10, 2017
4 checks passed
4 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 98.369%
Details
@shane-tomlinson shane-tomlinson deleted the send-sms-back-from-sms-sent branch Mar 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants