From 4139552c45af287fb19bbf621e94040dd36f97a6 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 1 Sep 2020 16:53:26 -0400 Subject: [PATCH 01/55] fix(NewAccountWizard): Create an OtpUser entry when user clicks Next after accepting terms. This is for compatibility with the phone verification backend. --- lib/components/user/new-account-wizard.js | 3 ++- lib/components/user/sequential-pane-display.js | 13 ++++++++++--- lib/components/user/user-account-screen.js | 14 +++++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index f590c4200..7823fb0cc 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -5,7 +5,7 @@ import SequentialPaneDisplay from './sequential-pane-display' /** * This component is the new account wizard. */ -const NewAccountWizard = ({ onComplete, panes, userData }) => { +const NewAccountWizard = ({ onComplete, onCreate, panes, userData }) => { const { hasConsentedToTerms, notificationChannel = 'email' @@ -15,6 +15,7 @@ const NewAccountWizard = ({ onComplete, panes, userData }) => { terms: { disableNext: !hasConsentedToTerms, nextId: 'notifications', + onNext: onCreate, pane: panes.terms, title: 'Create a new account' }, diff --git a/lib/components/user/sequential-pane-display.js b/lib/components/user/sequential-pane-display.js index 0caab78d5..b0d12883b 100644 --- a/lib/components/user/sequential-pane-display.js +++ b/lib/components/user/sequential-pane-display.js @@ -28,17 +28,24 @@ class SequentialPaneDisplay extends Component { } } - _handleToNextPane = () => { + _handleToNextPane = async () => { const { onComplete, paneSequence } = this.props const { activePaneId } = this.state - const nextId = paneSequence[activePaneId].nextId + const currentPane = paneSequence[activePaneId] + const nextId = currentPane.nextId if (nextId) { + // Execute pane-specific action, if any (e.g. save a user account) + // when clicking next. + if (typeof currentPane.onNext === 'function') { + await currentPane.onNext() + } + this.setState({ activePaneId: nextId }) } else if (onComplete) { - onComplete() + onComplete() // FIXME: use await? } } diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 0c77fa85d..edcec8f56 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -25,6 +25,11 @@ class UserAccountScreen extends Component { super(props) this.state = { + // Capture whether user is a new user here and retain that value as long as this screen is active. + // (When a new user progresses through the account steps, isNewUser(loggedInUser) will change to false.) + isNewUser: isNewUser(props.loggedInUser), + + // Work on a copy of the logged-in user data. userData: clone(props.loggedInUser) } } @@ -46,6 +51,8 @@ class UserAccountScreen extends Component { const { userData } = this.state await createOrUpdateUser(userData) + // FIXME: do not show confirmation message in wizard mode. + // TODO: Handle UI feedback (currently an alert() dialog inside createOrUpdateUser). } @@ -88,11 +95,11 @@ class UserAccountScreen extends Component { // TODO: Update title bar during componentDidMount. render () { - const { auth, loggedInUser } = this.props - const { userData } = this.state + const { auth } = this.props + const { isNewUser, userData } = this.state let formContents - if (isNewUser(loggedInUser)) { + if (isNewUser) { if (!auth.user.email_verified) { // Check and prompt for email verification first to avoid extra user wait. formContents = @@ -102,6 +109,7 @@ class UserAccountScreen extends Component { formContents = ( From 7f1851b413b6002ee33788a51c474d381eba9269 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 1 Sep 2020 17:39:16 -0400 Subject: [PATCH 02/55] refactor(UserAccountScreen): Update working copy of state.user on componentDidUpdate --- lib/components/user/user-account-screen.js | 24 ++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index edcec8f56..0bee81731 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -1,10 +1,11 @@ import clone from 'lodash/cloneDeep' +import isEqual from 'lodash.isequal' import React, { Component } from 'react' import { connect } from 'react-redux' import { withLoginRequired } from 'use-auth0-hooks' -import { routeTo } from '../../actions/ui' -import { createOrUpdateUser } from '../../actions/user' +import * as uiActions from '../../actions/ui' +import * as userActions from '../../actions/user' import { isNewUser } from '../../util/user' import DesktopNav from '../app/desktop-nav' import AccountSetupFinishPane from './account-setup-finish-pane' @@ -34,6 +35,9 @@ class UserAccountScreen extends Component { } } + /** + * Updates state.userData with new data (can be just one prop or the entire user record). + */ _updateUserState = newUserData => { const { userData } = this.state this.setState({ @@ -92,6 +96,18 @@ class UserAccountScreen extends Component { finish: AccountSetupFinishPane } + componentDidUpdate (prevProps) { + // If the loggedInUser record has been updated while this screen is shown + // (e.g. when a new user clicks next after agreeing on terms), + // then update the working copy in state.userData with the latest + // Changes in the previous working copy will be discarded (hopefully, there are none). + const { loggedInUser } = this.props + + if (!isEqual(prevProps.loggedInUser, loggedInUser)) { + this._updateUserState(loggedInUser) + } + } + // TODO: Update title bar during componentDidMount. render () { @@ -147,8 +163,8 @@ const mapStateToProps = (state, ownProps) => { } const mapDispatchToProps = { - createOrUpdateUser, - routeTo + createOrUpdateUser: userActions.createOrUpdateUser, + routeTo: uiActions.routeTo } export default withLoggedInUserSupport( From 190735259cc2cd642341aed724459a125b411966 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 2 Sep 2020 22:22:28 -0400 Subject: [PATCH 03/55] fix(FavoriteLocationsPane): Remove console warning regarding null value. --- lib/components/user/favorite-locations-pane.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/components/user/favorite-locations-pane.js b/lib/components/user/favorite-locations-pane.js index e143a70e2..a3a18113a 100644 --- a/lib/components/user/favorite-locations-pane.js +++ b/lib/components/user/favorite-locations-pane.js @@ -118,12 +118,12 @@ class FavoriteLocationsPane extends Component { // In theory there could be multiple home or work locations. // Just pick the first one. const homeLocation = savedLocations.find(isHome) || { - address: null, + address: '', icon: 'home', type: 'home' } const workLocation = savedLocations.find(isWork) || { - address: null, + address: '', icon: 'briefcase', type: 'work' } From 3100de8a57a1dbbe2e1bd23d0b8ed0a6fab09ded Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 2 Sep 2020 22:24:10 -0400 Subject: [PATCH 04/55] fix(NotificationPrefsPane): Prepare UI support for phone verification. --- .../user/notification-prefs-pane.js | 146 +++++++++++++++++- 1 file changed, 138 insertions(+), 8 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 51323c194..7489dd703 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -1,6 +1,6 @@ import PropTypes from 'prop-types' import React, { Component } from 'react' -import { ButtonToolbar, ControlLabel, FormControl, FormGroup, ToggleButton, ToggleButtonGroup } from 'react-bootstrap' +import { Button, ButtonToolbar, ControlLabel, FormControl, FormGroup, Glyphicon, HelpBlock, Modal, ToggleButton, ToggleButtonGroup } from 'react-bootstrap' import styled from 'styled-components' const allowedNotificationChannels = [ @@ -21,7 +21,8 @@ const allowedNotificationChannels = [ // Styles // HACK: Preverve container height. const Details = styled.div` - height: 150px; + min-height: 150px; + margin-bottom: 15px; ` /** @@ -33,24 +34,110 @@ class NotificationPrefsPane extends Component { userData: PropTypes.object.isRequired } + constructor (props) { + super(props) + + this.state = { + // Set to true when the field is changed, to display validation errors subsequently. + isPhoneFieldModified: false, + // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. + isVerificationPending: false, + // Holds the entered phone number that needs to be validated. + pendingPhoneNumber: props.phoneNumber, + // Holds the validation code. + phoneValidationCode: null + } + } + _handleNotificationChannelChange = e => { const { onUserDataChange } = this.props onUserDataChange({ notificationChannel: e }) } - _handlePhoneNumberChange = e => { + _handlePhoneNumberVerified = e => { const { onUserDataChange } = this.props onUserDataChange({ phoneNumber: e.target.value }) } + _handlePhoneNumberChange = e => { + // Mark field as modified, update pending phone number state. + this.setState({ + isPhoneFieldModified: true, + pendingPhoneNumber: e.target.value + }) + } + + _handlePhoneValidationCodeChange = e => { + // Update validation code state. + this.setState({ + phoneValidationCode: e.target.value + }) + } + + _handlePhoneValidationCancel = () => { + // Exit the phone verification process. + this.setState({ + isVerificationPending: false + }) + } + + _handleRevertPhoneNumber = () => { + // Revert entered phone number to the one from the user record. + // Reset the modified and pending states. + this.setState({ + isPhoneFieldModified: false, + isVerificationPending: false, + pendingPhoneNumber: this.props.userData.phoneNumber + }) + } + + _handleStartPhoneVerification = () => { + // Send the request for phone verification, + // show controls for entering and sending the validation code. + + // send request + + // Prompt for code + this.setState({ + isVerificationPending: true, + phoneValidationCode: null + }) + } + + _handleSendPhoneVerification = () => { + + } + render () { const { userData } = this.props + const { isPhoneFieldModified, isVerificationPending, pendingPhoneNumber = '', phoneValidationCode } = this.state const { email, + isPhoneNumberVerified, notificationChannel, phoneNumber } = userData + const shouldVerifyPhone = pendingPhoneNumber && pendingPhoneNumber.length && (!isPhoneNumberVerified || pendingPhoneNumber !== phoneNumber) + + let phoneStatusGlyph // one of the Bootstrap glyphs. + let phoneStatusText + let phoneFieldValidationState // one of the Bootstrap validationState values. + + if (isPhoneNumberVerified) { + phoneStatusGlyph = 'ok' + phoneStatusText = 'Verified' + phoneFieldValidationState = 'success' + } else if (shouldVerifyPhone) { + phoneStatusGlyph = 'remove' + phoneStatusText = 'Verification required' + phoneFieldValidationState = 'error' + } else if (isPhoneFieldModified) { + phoneStatusGlyph = 'remove' + phoneStatusText = 'No number provided' + phoneFieldValidationState = 'error' + } + return (

@@ -85,13 +172,56 @@ class NotificationPrefsPane extends Component { )} {notificationChannel === 'sms' && ( - - Enter your phone number for SMS notifications: - {/* TODO: Add field validation. */} - - +

+ + Enter your phone number for SMS notifications: + {/* TODO: Add field validation. */} + + + {/* Show glyphs underneath the inpiut control + (there are some alignment issues with in mobile view). */} + + {phoneStatusGlyph && } {phoneStatusText} + + + + {shouldVerifyPhone && ( +
+ + {' '} + +
+ )} +
)} + + {/* The dialog prompt for validation code. */} + + + Enter verification code + + + + +

+ Please check the SMS messaging app on your mobile phone + for a text message with a verification code, and enter the code below. +

+ Verification code: + +
+
+ + + + + +
) } From 12d7afe5a5e5a7a71c7cc1fe2c06313ec43fb30e Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 3 Sep 2020 19:42:09 -0400 Subject: [PATCH 05/55] fix(Hook to send phone verification number.): --- lib/actions/user.js | 95 +++++++++++++++++++ .../user/existing-account-display.js | 3 +- lib/components/user/new-account-wizard.js | 2 +- .../user/notification-prefs-pane.js | 85 ++++++++++------- lib/components/user/user-account-screen.js | 31 +++++- lib/util/middleware.js | 19 +++- 6 files changed, 195 insertions(+), 40 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index b5c094073..154ea4233 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -6,6 +6,8 @@ import { deleteTrip, fetchUser, getTrips, + startPhoneVerification, + // sendPhoneVerification, updateTrip, updateUser } from '../util/middleware' @@ -181,3 +183,96 @@ export function deleteUserMonitoredTrip (id) { } } } + +/** + * Initiates the phone number verification process for the logged-in user. + * // FIXME: This requires saving the pending phone number to the OtpUser object. + */ +export function startUserPhoneVerification (userData, previousPhone) { + return async function (dispatch, getState) { + const { otp, user } = getState() + const { otp_middleware: otpMiddleware = null } = otp.config.persistence + + if (otpMiddleware) { + const { accessToken } = user + + // FIXME: Temporarily save the user record with the pending phone number (required by middleware). + // TODO: Figure out what should happen if the user refreshes the browser at this stage. + const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) + + if (userUpdateResult.status === 'success' && userUpdateResult.data) { + const startPhoneVerificationResult = await startPhoneVerification(otpMiddleware, accessToken, userData.id) + if (startPhoneVerificationResult.status === 'success') { + // Update application state AFTER the phone verification request has been sent. + const newUserData = userUpdateResult.data + + // FIXME: Temporarily save the previous phone number so we can revert it in case verification is aborted by user. + // We do it here instead of inside the user account UI because this is a backend-specific behavior. + newUserData.previousPhoneNumber = previousPhone + + dispatch(setCurrentUser({ accessToken, user: newUserData })) + } else { + alert(`An error was encountered:\n${JSON.stringify(startPhoneVerificationResult)}`) + + // Also if there was an error in sending the verificaton request, revert the phone number. + userData.phoneNumber = previousPhone + const userRevertResult = await updateUser(otpMiddleware, accessToken, userData) + const revertedUserData = userRevertResult.data + dispatch(setCurrentUser({ accessToken, user: revertedUserData })) + } + } else { + alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) + } + } + } +} + +/** + * Reverts the phone number to the one in place prior to the verification process (if any). + * // FIXME: This method assumes the middleware requires saving the users pending phone number prior to verification. + */ +export function revertUserPhoneNumber (userData) { + return async function (dispatch, getState) { + const { otp, user } = getState() + const { otp_middleware: otpMiddleware = null } = otp.config.persistence + + if (otpMiddleware) { + const { accessToken } = user + + userData.phoneNumber = userData.previousPhoneNumber + + // FIXME: Revert the user record. + // This operation should get rid of the previousPhoneNumber field + // because that field is not supported by the middleware. + const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) + if (userUpdateResult.status === 'success' && userUpdateResult.data) { + // Update application state with the user entry as saved + // (as returned) by the middleware. + const newUserData = userUpdateResult.data + dispatch(setCurrentUser({ accessToken, user: newUserData })) + } else { + alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) + } + } + } +} + +/** + * Sends the phone number verification code for the logged-in user. + */ +export function sendUserPhoneVerification (userData, previousPhone) { + return async function (dispatch, getState) { + const { otp, user } = getState() + const { otp_middleware: otpMiddleware = null } = otp.config.persistence + + if (otpMiddleware) { + const { accessToken } = user + const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) + if (userUpdateResult.status === 'success' && userUpdateResult.data) { + // To be completed. + } else { + alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) + } + } + } +} diff --git a/lib/components/user/existing-account-display.js b/lib/components/user/existing-account-display.js index 0513914f4..1de8d830b 100644 --- a/lib/components/user/existing-account-display.js +++ b/lib/components/user/existing-account-display.js @@ -8,7 +8,7 @@ import StackedPaneDisplay from './stacked-pane-display' */ class ExistingAccountDisplay extends Component { render () { - const { onCancel, onComplete, panes } = this.props + const { onCancel, onComplete, onRevertUserPhoneNumber, onStartPhoneVerification, panes } = this.props const paneSequence = [ { pane: () =>

Edit my trips

, @@ -21,6 +21,7 @@ class ExistingAccountDisplay extends Component { }, { pane: panes.notifications, + props: { onRevertUserPhoneNumber, onStartPhoneVerification }, title: 'Notifications' }, { diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 7823fb0cc..7b3837d27 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -5,7 +5,7 @@ import SequentialPaneDisplay from './sequential-pane-display' /** * This component is the new account wizard. */ -const NewAccountWizard = ({ onComplete, onCreate, panes, userData }) => { +const NewAccountWizard = ({ onComplete, onCreate, onStartPhoneVerification, panes, userData }) => { const { hasConsentedToTerms, notificationChannel = 'email' diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 7489dd703..9f7dcfd92 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -38,14 +38,14 @@ class NotificationPrefsPane extends Component { super(props) this.state = { + // Holds the initial phone number or the last confirmed phone number + initialPhoneNumber: props.userData.phoneNumber, // Set to true when the field is changed, to display validation errors subsequently. isPhoneFieldModified: false, // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. isVerificationPending: false, - // Holds the entered phone number that needs to be validated. - pendingPhoneNumber: props.phoneNumber, // Holds the validation code. - phoneValidationCode: null + phoneValidationCode: '' } } @@ -60,10 +60,12 @@ class NotificationPrefsPane extends Component { } _handlePhoneNumberChange = e => { - // Mark field as modified, update pending phone number state. + const { onUserDataChange } = this.props + onUserDataChange({ phoneNumber: e.target.value }) + + // Mark field as modified. this.setState({ - isPhoneFieldModified: true, - pendingPhoneNumber: e.target.value + isPhoneFieldModified: true }) } @@ -86,21 +88,20 @@ class NotificationPrefsPane extends Component { // Reset the modified and pending states. this.setState({ isPhoneFieldModified: false, - isVerificationPending: false, - pendingPhoneNumber: this.props.userData.phoneNumber + isVerificationPending: false }) + + this.props.onRevertUserPhoneNumber(this.state.userData) } _handleStartPhoneVerification = () => { - // Send the request for phone verification, - // show controls for entering and sending the validation code. - - // send request + // Send phone verification request + this.props.onStartPhoneVerification() // Prompt for code this.setState({ isVerificationPending: true, - phoneValidationCode: null + phoneValidationCode: '' }) } @@ -110,7 +111,7 @@ class NotificationPrefsPane extends Component { render () { const { userData } = this.props - const { isPhoneFieldModified, isVerificationPending, pendingPhoneNumber = '', phoneValidationCode } = this.state + const { isPhoneFieldModified, isVerificationPending, initialPhoneNumber, phoneValidationCode } = this.state const { email, isPhoneNumberVerified, @@ -118,24 +119,41 @@ class NotificationPrefsPane extends Component { phoneNumber } = userData - const shouldVerifyPhone = pendingPhoneNumber && pendingPhoneNumber.length && (!isPhoneNumberVerified || pendingPhoneNumber !== phoneNumber) + // Here are the states we are dealing with: + // - First time entering a phone number (phoneNumber blank, not modified) + // => no color, no feedback indication. + // - Typing backspace all the way to erase a number (phoneNumber blank, modified) + // => red, "[X] Please provide a number" indication. + // - Viewing a verified phone number (phoneNumber non-blank, same as initial, verified) + // => green, "[V] Verified" indication. + // - Editing a phone number (phoneNumber non-blank, different than initial or not verified) + // => red, "[X] Verification required" indication. + + const isPhoneNumberBlank = !(phoneNumber && phoneNumber.length) + const isPhoneNumberSameAsInitial = phoneNumber === initialPhoneNumber let phoneStatusGlyph // one of the Bootstrap glyphs. let phoneStatusText let phoneFieldValidationState // one of the Bootstrap validationState values. - - if (isPhoneNumberVerified) { + let shouldVerifyPhone = false + + if (isPhoneNumberBlank) { + if (!isPhoneFieldModified) { + // Do not show an indication. + } else { + phoneStatusGlyph = 'remove' + phoneStatusText = 'Please provide a number' + phoneFieldValidationState = 'error' + } + } else if (isPhoneNumberSameAsInitial && isPhoneNumberVerified) { phoneStatusGlyph = 'ok' phoneStatusText = 'Verified' phoneFieldValidationState = 'success' - } else if (shouldVerifyPhone) { - phoneStatusGlyph = 'remove' - phoneStatusText = 'Verification required' - phoneFieldValidationState = 'error' - } else if (isPhoneFieldModified) { + } else { phoneStatusGlyph = 'remove' - phoneStatusText = 'No number provided' + phoneStatusText = 'Verification required.' phoneFieldValidationState = 'error' + shouldVerifyPhone = true } return ( @@ -179,11 +197,13 @@ class NotificationPrefsPane extends Component { - {/* Show glyphs underneath the inpiut control - (there are some alignment issues with in mobile view). */} + {/* Show glyphs underneath the input control + (there are some alignment issues with in mobile view), + so we use instead. */} {phoneStatusGlyph && } {phoneStatusText} @@ -191,8 +211,7 @@ class NotificationPrefsPane extends Component { {shouldVerifyPhone && (
- - {' '} +
)} @@ -207,11 +226,11 @@ class NotificationPrefsPane extends Component { +

+ Please check the SMS messaging app on your mobile phone + for a text message with a verification code, and enter the code below. +

-

- Please check the SMS messaging app on your mobile phone - for a text message with a verification code, and enter the code below. -

Verification code:
diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 0bee81731..fb1d80912 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -26,8 +26,9 @@ class UserAccountScreen extends Component { super(props) this.state = { - // Capture whether user is a new user here and retain that value as long as this screen is active. - // (When a new user progresses through the account steps, isNewUser(loggedInUser) will change to false.) + // Capture whether user is a new user at this stage, and retain that value as long as this screen is active. + // Reminder: When a new user progresses through the account steps, + // isNewUser(loggedInUser) will change to false as the database gets updated. isNewUser: isNewUser(props.loggedInUser), // Work on a copy of the logged-in user data. @@ -70,6 +71,22 @@ class UserAccountScreen extends Component { this._handleExit() } + _handleStartPhoneVerification = async () => { + // FIXME: For this function's purpose, + // only update the user phone number with the pending one, + // so we can verify in the middleware. + // The full user record will be updated upon the user clicking Finish/Save preferences. + const tempUserData = clone(this.props.loggedInUser) + const previousPhoneNumber = tempUserData.phoneNumber + + // These settings will be reverted if validation is not completed. + tempUserData.phoneNumber = this.state.userData.phoneNumber + tempUserData.isPhoneNumberVerified = false + tempUserData.notificationChannel = 'sms' + + await this.props.startUserPhoneVerification(tempUserData, previousPhoneNumber) + } + /** * Hook userData, onUserDataChange on some panes upon rendering. * This returns a new render function for the passed component @@ -111,7 +128,7 @@ class UserAccountScreen extends Component { // TODO: Update title bar during componentDidMount. render () { - const { auth } = this.props + const { auth, revertUserPhoneNumber } = this.props const { isNewUser, userData } = this.state let formContents @@ -126,6 +143,8 @@ class UserAccountScreen extends Component { @@ -137,6 +156,8 @@ class UserAccountScreen extends Component { ) @@ -164,7 +185,9 @@ const mapStateToProps = (state, ownProps) => { const mapDispatchToProps = { createOrUpdateUser: userActions.createOrUpdateUser, - routeTo: uiActions.routeTo + revertUserPhoneNumber: userActions.revertUserPhoneNumber, + routeTo: uiActions.routeTo, + startUserPhoneVerification: userActions.startUserPhoneVerification } export default withLoggedInUserSupport( diff --git a/lib/util/middleware.js b/lib/util/middleware.js index 10403b465..b73153e7f 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -1,7 +1,8 @@ if (typeof (fetch) === 'undefined') require('isomorphic-fetch') -const API_USER_PATH = '/api/secure/user' const API_MONITORTRIP_PATH = '/api/secure/monitoredtrip' +const API_USER_PATH = '/api/secure/user' +const API_USER_VERIFYSMS_PATH = '/verify_sms' /** * This method builds the options object for call to the fetch method. @@ -139,3 +140,19 @@ export async function deleteTrip (middlewareConfig, token, id) { } } } + +export async function startPhoneVerification (middlewareConfig, token, userId) { + const { apiBaseUrl, apiKey } = middlewareConfig + const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}` + + return secureFetch(requestUrl, token, apiKey, 'GET') +} + +export async function sendPhoneVerification (middlewareConfig, token, data) { +// const { apiBaseUrl, apiKey } = middlewareConfig +// const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` + +// return secureFetch(requestUrl, token, apiKey, 'POST', { +// body: JSON.stringify(data) +// }) +} From 74ab103650b7c266d3194efd1325177fc292bfcc Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 4 Sep 2020 11:19:52 -0400 Subject: [PATCH 06/55] feat(NotificationPrefsPane): Add phone verification for existing users. --- lib/actions/user.js | 62 ++++++++++--------- .../user/existing-account-display.js | 4 +- .../user/notification-prefs-pane.js | 28 +++++++-- lib/components/user/user-account-screen.js | 10 ++- lib/util/middleware.js | 10 ++- 5 files changed, 72 insertions(+), 42 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 154ea4233..b211c0700 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -7,7 +7,7 @@ import { fetchUser, getTrips, startPhoneVerification, - // sendPhoneVerification, + sendPhoneVerification, updateTrip, updateUser } from '../util/middleware' @@ -98,7 +98,7 @@ export function fetchOrInitializeUser (auth) { * Updates (or creates) a user entry in the middleware, * then, if that was successful, updates the redux state with that user. */ -export function createOrUpdateUser (userData) { +export function createOrUpdateUser (userData, silentOnSuccess = false) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence @@ -115,12 +115,14 @@ export function createOrUpdateUser (userData) { // TODO: improve the UI feedback messages for this. if (result.status === 'success' && result.data) { - alert('Your preferences have been saved.') + if (!silentOnSuccess) { + alert('Your preferences have been saved.') + } // Update application state with the user entry as saved // (as returned) by the middleware. - const userData = result.data - dispatch(setCurrentUser({ accessToken, user: userData })) + const newUserData = result.data + dispatch(setCurrentUser({ accessToken, user: newUserData })) } else { alert(`An error was encountered:\n${JSON.stringify(result)}`) } @@ -128,6 +130,13 @@ export function createOrUpdateUser (userData) { } } +/** + * Silent-on-success version of function above. + */ +export function createOrUpdateUserSilent (userData) { + return createOrUpdateUser(userData, true) +} + /** * Updates a logged-in user's monitored trip, * then, if that was successful, refreshes the redux monitoredTrips @@ -233,26 +242,12 @@ export function startUserPhoneVerification (userData, previousPhone) { */ export function revertUserPhoneNumber (userData) { return async function (dispatch, getState) { - const { otp, user } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence - - if (otpMiddleware) { - const { accessToken } = user - + if (userData.previousPhoneNumber) { userData.phoneNumber = userData.previousPhoneNumber + // Must delete extra fields to avoid error messages. + delete userData.previousPhoneNumber - // FIXME: Revert the user record. - // This operation should get rid of the previousPhoneNumber field - // because that field is not supported by the middleware. - const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) - if (userUpdateResult.status === 'success' && userUpdateResult.data) { - // Update application state with the user entry as saved - // (as returned) by the middleware. - const newUserData = userUpdateResult.data - dispatch(setCurrentUser({ accessToken, user: newUserData })) - } else { - alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) - } + dispatch(createOrUpdateUserSilent(userData)) } } } @@ -260,18 +255,29 @@ export function revertUserPhoneNumber (userData) { /** * Sends the phone number verification code for the logged-in user. */ -export function sendUserPhoneVerification (userData, previousPhone) { +export function sendUserPhoneVerification (userData, code) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence if (otpMiddleware) { const { accessToken } = user - const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) - if (userUpdateResult.status === 'success' && userUpdateResult.data) { - // To be completed. + const sendResult = await sendPhoneVerification(otpMiddleware, accessToken, userData.id, code) + if (sendResult.status === 'success' && sendResult.data) { + if (sendResult.data.status === 'approved') { + alert('Your phone is now verified and set to receive trip notifications.') + + // Update phone verification in user state. + userData.isPhoneNumberVerified = true + // Must delete extra fields to avoid error messages. + delete userData.previousPhoneNumber + + dispatch(createOrUpdateUserSilent(userData)) + } else { + alert('You entered in incorrect validation code. Please try again.') + } } else { - alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) + alert(`Your phone could not be verified:\n${JSON.stringify(sendResult)}`) } } } diff --git a/lib/components/user/existing-account-display.js b/lib/components/user/existing-account-display.js index 1de8d830b..ef1ee64fd 100644 --- a/lib/components/user/existing-account-display.js +++ b/lib/components/user/existing-account-display.js @@ -8,7 +8,7 @@ import StackedPaneDisplay from './stacked-pane-display' */ class ExistingAccountDisplay extends Component { render () { - const { onCancel, onComplete, onRevertUserPhoneNumber, onStartPhoneVerification, panes } = this.props + const { onCancel, onComplete, onRevertUserPhoneNumber, onSendPhoneVerification, onStartPhoneVerification, panes } = this.props const paneSequence = [ { pane: () =>

Edit my trips

, @@ -21,7 +21,7 @@ class ExistingAccountDisplay extends Component { }, { pane: panes.notifications, - props: { onRevertUserPhoneNumber, onStartPhoneVerification }, + props: { onRevertUserPhoneNumber, onSendPhoneVerification, onStartPhoneVerification }, title: 'Notifications' }, { diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 9f7dcfd92..52dea7ba0 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -30,6 +30,8 @@ const Details = styled.div` */ class NotificationPrefsPane extends Component { static propTypes = { + onStartPhoneVerification: PropTypes.func, + onSendPhoneVerification: PropTypes.func, onUserDataChange: PropTypes.func.isRequired, userData: PropTypes.object.isRequired } @@ -79,7 +81,8 @@ class NotificationPrefsPane extends Component { _handlePhoneValidationCancel = () => { // Exit the phone verification process. this.setState({ - isVerificationPending: false + isVerificationPending: false, + phoneValidationCode: '' }) } @@ -95,18 +98,33 @@ class NotificationPrefsPane extends Component { } _handleStartPhoneVerification = () => { - // Send phone verification request + // Send phone verification request. this.props.onStartPhoneVerification() - // Prompt for code + // Prompt for code. this.setState({ isVerificationPending: true, phoneValidationCode: '' }) } - _handleSendPhoneVerification = () => { + _handleSendPhoneVerification = async () => { + // Send phone verification code. + await this.props.onSendPhoneVerification(this.state.phoneValidationCode) + // Exit verification modal and update "initialPhoneNumber" if isPhoneNumberVerified was changed to true. + // Erase the verification code in all cases for discretion. + if (this.props.userData.isPhoneNumberVerified) { + this.setState({ + initialPhoneNumber: this.props.userData.phoneNumber, + isVerificationPending: false, + phoneValidationCode: '' + }) + } else { + this.setState({ + phoneValidationCode: '' + }) + } } render () { @@ -238,7 +256,7 @@ class NotificationPrefsPane extends Component { - + diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index fb1d80912..719ee5ed1 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -79,7 +79,7 @@ class UserAccountScreen extends Component { const tempUserData = clone(this.props.loggedInUser) const previousPhoneNumber = tempUserData.phoneNumber - // These settings will be reverted if validation is not completed. + // FIXME: Revert these settings if validation is not completed. tempUserData.phoneNumber = this.state.userData.phoneNumber tempUserData.isPhoneNumberVerified = false tempUserData.notificationChannel = 'sms' @@ -87,6 +87,11 @@ class UserAccountScreen extends Component { await this.props.startUserPhoneVerification(tempUserData, previousPhoneNumber) } + _handleSendPhoneVerification = async code => { + // Send phone validation code. + await this.props.sendUserPhoneVerification(this.state.userData, code) + } + /** * Hook userData, onUserDataChange on some panes upon rendering. * This returns a new render function for the passed component @@ -144,6 +149,7 @@ class UserAccountScreen extends Component { onComplete={this._handleExitAndSave} onCreate={this._updateUserPrefs} onRevertUserPhoneNumber={revertUserPhoneNumber} + onSendPhoneVerification={this._handleSendPhoneVerification} onStartPhoneVerification={this._handleStartPhoneVerification} panes={this._panes} userData={userData} @@ -157,6 +163,7 @@ class UserAccountScreen extends Component { onCancel={this._handleExit} onComplete={this._handleExitAndSave} onRevertUserPhoneNumber={revertUserPhoneNumber} + onSendPhoneVerification={this._handleSendPhoneVerification} onStartPhoneVerification={this._handleStartPhoneVerification} panes={this._panes} /> @@ -187,6 +194,7 @@ const mapDispatchToProps = { createOrUpdateUser: userActions.createOrUpdateUser, revertUserPhoneNumber: userActions.revertUserPhoneNumber, routeTo: uiActions.routeTo, + sendUserPhoneVerification: userActions.sendUserPhoneVerification, startUserPhoneVerification: userActions.startUserPhoneVerification } diff --git a/lib/util/middleware.js b/lib/util/middleware.js index b73153e7f..bf693c3c1 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -148,11 +148,9 @@ export async function startPhoneVerification (middlewareConfig, token, userId) { return secureFetch(requestUrl, token, apiKey, 'GET') } -export async function sendPhoneVerification (middlewareConfig, token, data) { -// const { apiBaseUrl, apiKey } = middlewareConfig -// const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` +export async function sendPhoneVerification (middlewareConfig, token, userId, code) { + const { apiBaseUrl, apiKey } = middlewareConfig + const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${code}` -// return secureFetch(requestUrl, token, apiKey, 'POST', { -// body: JSON.stringify(data) -// }) + return secureFetch(requestUrl, token, apiKey, 'POST') } From 33686c0cec720cded911131c9a0ed718db0c7027 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 4 Sep 2020 12:14:11 -0400 Subject: [PATCH 07/55] feat(NewAccountWizard): Add phone verification in account setup wizard. --- lib/components/user/new-account-wizard.js | 13 ++++------- .../user/notification-prefs-pane.js | 5 +++- .../user/phone-verification-pane.js | 23 ------------------- lib/components/user/user-account-screen.js | 13 ++++++----- 4 files changed, 15 insertions(+), 39 deletions(-) delete mode 100644 lib/components/user/phone-verification-pane.js diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 7b3837d27..47b469456 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -5,7 +5,7 @@ import SequentialPaneDisplay from './sequential-pane-display' /** * This component is the new account wizard. */ -const NewAccountWizard = ({ onComplete, onCreate, onStartPhoneVerification, panes, userData }) => { +const NewAccountWizard = ({ onComplete, onCreate, onSendPhoneVerification, onStartPhoneVerification, panes, userData }) => { const { hasConsentedToTerms, notificationChannel = 'email' @@ -20,18 +20,13 @@ const NewAccountWizard = ({ onComplete, onCreate, onStartPhoneVerification, pane title: 'Create a new account' }, notifications: { - nextId: notificationChannel === 'sms' ? 'verifyPhone' : 'places', + disableNext: notificationChannel === 'sms' && !userData.isPhoneNumberVerified, + nextId: 'places', pane: panes.notifications, prevId: 'terms', + props: { onSendPhoneVerification, onStartPhoneVerification }, title: 'Notification preferences' }, - verifyPhone: { - disableNext: true, // TODO: implement verification. - nextId: 'places', - pane: panes.verifyPhone, - prevId: 'notifications', - title: 'Verify your phone' - }, places: { nextId: 'finish', pane: panes.locations, diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 52dea7ba0..09a12f7bc 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -94,7 +94,10 @@ class NotificationPrefsPane extends Component { isVerificationPending: false }) - this.props.onRevertUserPhoneNumber(this.state.userData) + const { onRevertUserPhoneNumber } = this.props + if (onRevertUserPhoneNumber) { + onRevertUserPhoneNumber(this.state.userData) + } } _handleStartPhoneVerification = () => { diff --git a/lib/components/user/phone-verification-pane.js b/lib/components/user/phone-verification-pane.js deleted file mode 100644 index d1f9dbac5..000000000 --- a/lib/components/user/phone-verification-pane.js +++ /dev/null @@ -1,23 +0,0 @@ -import React from 'react' -import { Alert, FormControl, FormGroup } from 'react-bootstrap' - -/** - * User phone verification pane. - * TODO: to be completed. - */ -const PhoneVerificationPane = () => ( -
- - Under construction! - -

- Please check your mobile phone's SMS messaging app for a text - message with a verification code and copy the code below: -

- - - -
-) - -export default PhoneVerificationPane diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 719ee5ed1..bac43d308 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -13,7 +13,6 @@ import ExistingAccountDisplay from './existing-account-display' import FavoriteLocationsPane from './favorite-locations-pane' import NewAccountWizard from './new-account-wizard' import NotificationPrefsPane from './notification-prefs-pane' -import PhoneVerificationPane from './phone-verification-pane' import TermsOfUsePane from './terms-of-use-pane' import VerifyEmailScreen from './verify-email-screen' import withLoggedInUserSupport from './with-logged-in-user-support' @@ -49,18 +48,21 @@ class UserAccountScreen extends Component { }) } - _updateUserPrefs = async () => { + _updateUserPrefs = async (silentOnSucceed = false) => { // TODO: Change state of Save button while the update action takes place. const { createOrUpdateUser } = this.props const { userData } = this.state - await createOrUpdateUser(userData) - // FIXME: do not show confirmation message in wizard mode. + await createOrUpdateUser(userData, silentOnSucceed) // TODO: Handle UI feedback (currently an alert() dialog inside createOrUpdateUser). } + _handleCreateNewUser = async () => { + await this._updateUserPrefs(true) + } + _handleExit = () => { // On exit, route to default search route. this.props.routeTo('/') @@ -113,7 +115,6 @@ class UserAccountScreen extends Component { _panes = { terms: this._hookUserData(TermsOfUsePane), notifications: this._hookUserData(NotificationPrefsPane), - verifyPhone: PhoneVerificationPane, locations: this._hookUserData(FavoriteLocationsPane), finish: AccountSetupFinishPane } @@ -147,7 +148,7 @@ class UserAccountScreen extends Component { formContents = ( Date: Fri, 4 Sep 2020 17:39:24 -0400 Subject: [PATCH 08/55] improvement(UserAccountScreen): Try throttling SMS code requests. --- lib/actions/user.js | 79 ++++++------ .../user/notification-prefs-pane.js | 4 +- lib/components/user/user-account-screen.js | 119 ++++++++++++++---- lib/util/middleware.js | 3 + 4 files changed, 136 insertions(+), 69 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index b211c0700..92fbc5403 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -1,3 +1,4 @@ +import clone from 'lodash/cloneDeep' import { createAction } from 'redux-actions' import { @@ -195,9 +196,8 @@ export function deleteUserMonitoredTrip (id) { /** * Initiates the phone number verification process for the logged-in user. - * // FIXME: This requires saving the pending phone number to the OtpUser object. */ -export function startUserPhoneVerification (userData, previousPhone) { +export function startUserPhoneVerification (originalUserData, newPhoneNumber) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence @@ -205,29 +205,35 @@ export function startUserPhoneVerification (userData, previousPhone) { if (otpMiddleware) { const { accessToken } = user - // FIXME: Temporarily save the user record with the pending phone number (required by middleware). + // FIXME: (required by middleware) Only temporarily update the user's phone number + // in the database with the pending one, + // and cache the previous value and verification state so they can be reverted. + // The full user record will be updated upon the user clicking Finish/Save preferences. // TODO: Figure out what should happen if the user refreshes the browser at this stage. + + // Make a clone of the original userData object. + const userData = clone(originalUserData) + const previousPhoneNumber = userData.phoneNumber + const previousIsPhoneNumberVerified = userData.isPhoneNumberVerified + + userData.phoneNumber = newPhoneNumber + userData.isPhoneNumberVerified = false + const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) if (userUpdateResult.status === 'success' && userUpdateResult.data) { const startPhoneVerificationResult = await startPhoneVerification(otpMiddleware, accessToken, userData.id) if (startPhoneVerificationResult.status === 'success') { - // Update application state AFTER the phone verification request has been sent. - const newUserData = userUpdateResult.data - - // FIXME: Temporarily save the previous phone number so we can revert it in case verification is aborted by user. - // We do it here instead of inside the user account UI because this is a backend-specific behavior. - newUserData.previousPhoneNumber = previousPhone - - dispatch(setCurrentUser({ accessToken, user: newUserData })) + // Update application state on success. + dispatch(setCurrentUser({ accessToken, user: userUpdateResult.data })) } else { alert(`An error was encountered:\n${JSON.stringify(startPhoneVerificationResult)}`) - // Also if there was an error in sending the verificaton request, revert the phone number. - userData.phoneNumber = previousPhone - const userRevertResult = await updateUser(otpMiddleware, accessToken, userData) - const revertedUserData = userRevertResult.data - dispatch(setCurrentUser({ accessToken, user: revertedUserData })) + // Also if there was an error in sending the verificaton request, + // Revert the phone number and verification status in the database. + userData.phoneNumber = previousPhoneNumber + userData.isPhoneNumberVerified = previousIsPhoneNumberVerified + await updateUser(otpMiddleware, accessToken, userData) } } else { alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) @@ -236,43 +242,36 @@ export function startUserPhoneVerification (userData, previousPhone) { } } -/** - * Reverts the phone number to the one in place prior to the verification process (if any). - * // FIXME: This method assumes the middleware requires saving the users pending phone number prior to verification. - */ -export function revertUserPhoneNumber (userData) { - return async function (dispatch, getState) { - if (userData.previousPhoneNumber) { - userData.phoneNumber = userData.previousPhoneNumber - // Must delete extra fields to avoid error messages. - delete userData.previousPhoneNumber - - dispatch(createOrUpdateUserSilent(userData)) - } - } -} - /** * Sends the phone number verification code for the logged-in user. */ -export function sendUserPhoneVerification (userData, code) { +export function sendUserPhoneVerification (originalUserData, code) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence if (otpMiddleware) { const { accessToken } = user - const sendResult = await sendPhoneVerification(otpMiddleware, accessToken, userData.id, code) + const sendResult = await sendPhoneVerification(otpMiddleware, accessToken, originalUserData.id, code) if (sendResult.status === 'success' && sendResult.data) { if (sendResult.data.status === 'approved') { - alert('Your phone is now verified and set to receive trip notifications.') + // Make a clone of the original userData object. + const userData = clone(originalUserData) - // Update phone verification in user state. + // Update phone number and verification in database record. userData.isPhoneNumberVerified = true - // Must delete extra fields to avoid error messages. - delete userData.previousPhoneNumber - - dispatch(createOrUpdateUserSilent(userData)) + userData.notificationChannel = 'sms' + const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) + + if (userUpdateResult.status === 'success' && userUpdateResult.data) { + // FIXME: This action and text assumes the middleware requires saving the user's phone number prior to verification. + alert('Your phone is now verified and set to receive trip notifications.') + + // Update application state + dispatch(setCurrentUser({ accessToken, user: userUpdateResult.data })) + } else { + alert(`Error updating your phone's verified status:\n${JSON.stringify(sendResult)}`) + } } else { alert('You entered in incorrect validation code. Please try again.') } diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 09a12f7bc..a649c5907 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -231,9 +231,9 @@ class NotificationPrefsPane extends Component { {shouldVerifyPhone && ( -
- +
+
)}
diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index bac43d308..29c543ffa 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -30,7 +30,18 @@ class UserAccountScreen extends Component { // isNewUser(loggedInUser) will change to false as the database gets updated. isNewUser: isNewUser(props.loggedInUser), - // Work on a copy of the logged-in user data. + // Last number and last time we requested a code for (to avoid sending SMS over and over to verify the same number). + lastPhoneNumberRequested: null, + lastPhoneRequestTime: null, + + // Previous phone verification status + // if the user needs to revert their phone number in the middle of the verification process. + previousIsPhoneNumberVerified: null, + + // Previous phone number if the user needs to revert it in the middle of the verification process. + previousPhoneNumber: null, + + // Work on a copy of the logged-in user data captured when this component is created. userData: clone(props.loggedInUser) } } @@ -39,7 +50,19 @@ class UserAccountScreen extends Component { * Updates state.userData with new data (can be just one prop or the entire user record). */ _updateUserState = newUserData => { - const { userData } = this.state + const { previousPhoneNumber, userData } = this.state + const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser + + // If the phone number changed from the original and none was previously recorded, then + // save the original number and verification status. + if (newUserData.phoneNumber !== phoneNumber && !previousPhoneNumber) { + this.setState({ + previousIsPhoneNumberVerified: isPhoneNumberVerified, + previousPhoneNumber: phoneNumber + }) + } + + // Update the copy of user's data being edited. this.setState({ userData: { ...userData, @@ -73,25 +96,55 @@ class UserAccountScreen extends Component { this._handleExit() } + /** + * Reverts the user phone number and verification state in the database. + */ + _handleRevertPhoneNumber = () => { + // FIXME: This assumes the pending phone number must be saved in the database prior to verification. + const { createOrUpdateUser, loggedInUser } = this.props + + // Make a clone of the original userData object. + const userData = clone(loggedInUser) + userData.phoneNumber = this.state.previousPhoneNumber + userData.isPhoneNumberVerified = this.state.previousIsPhoneNumberVerified + + createOrUpdateUser(userData, true) + } + + /** + * Requests a phone verification code. + */ _handleStartPhoneVerification = async () => { - // FIXME: For this function's purpose, - // only update the user phone number with the pending one, - // so we can verify in the middleware. - // The full user record will be updated upon the user clicking Finish/Save preferences. - const tempUserData = clone(this.props.loggedInUser) - const previousPhoneNumber = tempUserData.phoneNumber - - // FIXME: Revert these settings if validation is not completed. - tempUserData.phoneNumber = this.state.userData.phoneNumber - tempUserData.isPhoneNumberVerified = false - tempUserData.notificationChannel = 'sms' - - await this.props.startUserPhoneVerification(tempUserData, previousPhoneNumber) + const { lastPhoneNumberRequested, lastPhoneRequestTime, userData } = this.state + const { phoneNumber } = userData + const timestamp = new Date() + + // Request a new verification code if we are requesting a different number. + // or enough time has ellapsed since the last request (1 minute?). + if (lastPhoneNumberRequested !== phoneNumber || + (lastPhoneRequestTime && lastPhoneRequestTime <= timestamp + 60000)) { + this.setState({ + lastPhoneNumberRequested: phoneNumber, + lastPhoneRequestTime: timestamp + }) + + // Use the original user data to avoid persisting any other pending edits. + await this.props.startUserPhoneVerification(this.props.loggedInUser, phoneNumber) + } } + /** + * Sends the phone verification code. + */ _handleSendPhoneVerification = async code => { - // Send phone validation code. - await this.props.sendUserPhoneVerification(this.state.userData, code) + // Use the original user data to avoid persisting any other pending edits. + await this.props.sendUserPhoneVerification(this.props.loggedInUser, code) + + // state.user.isPhoneNumberVerified will be set to true on success. + // Clear previous phone number and state if phone is verified. + if (this.props.loggedInUser.isPhoneNumberVerified) { + this.setState({ previousPhoneNumber: null, previousIsPhoneNumberVerified: null }) + } } /** @@ -120,21 +173,34 @@ class UserAccountScreen extends Component { } componentDidUpdate (prevProps) { - // If the loggedInUser record has been updated while this screen is shown - // (e.g. when a new user clicks next after agreeing on terms), - // then update the working copy in state.userData with the latest - // Changes in the previous working copy will be discarded (hopefully, there are none). + // We need to update some fields, but not erase the user's other pending changes + // when the loggedInUser record has been updated while this screen is shown, e.g.: + // - when a new user clicks next after agreeing on terms, + // - when the phone verification status changes (this is a middleware constraint). const { loggedInUser } = this.props - if (!isEqual(prevProps.loggedInUser, loggedInUser)) { - this._updateUserState(loggedInUser) + const { id, isPhoneNumberVerified, notificationChannel, phoneNumber } = loggedInUser + this._updateUserState({ + id, + isPhoneNumberVerified, + notificationChannel, + phoneNumber + }) + + // Clear previous phone states if values are identical to the ones set above. + if (phoneNumber === this.state.previousPhoneNumber) { + this.setState({ + previousIsPhoneNumberVerified: null, + previousPhoneNumber: null + }) + } } } // TODO: Update title bar during componentDidMount. render () { - const { auth, revertUserPhoneNumber } = this.props + const { auth } = this.props const { isNewUser, userData } = this.state let formContents @@ -149,7 +215,7 @@ class UserAccountScreen extends Component { { const mapDispatchToProps = { createOrUpdateUser: userActions.createOrUpdateUser, - revertUserPhoneNumber: userActions.revertUserPhoneNumber, routeTo: uiActions.routeTo, sendUserPhoneVerification: userActions.sendUserPhoneVerification, startUserPhoneVerification: userActions.startUserPhoneVerification diff --git a/lib/util/middleware.js b/lib/util/middleware.js index bf693c3c1..b571fea3f 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -142,6 +142,9 @@ export async function deleteTrip (middlewareConfig, token, id) { } export async function startPhoneVerification (middlewareConfig, token, userId) { + // TODO: There is potential to combine the variable extraction/assignments + // in this method and its peers + // with the variable extraction/assignments in lib/actions/user.js. const { apiBaseUrl, apiKey } = middlewareConfig const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}` From 106fa6d427202684cbf8dce94c4e8c4933781ac5 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Sep 2020 12:43:45 -0400 Subject: [PATCH 09/55] refactor(NotificationPrefsPane): Add button to resend code. Rename methods. --- lib/actions/user.js | 4 ++-- .../user/existing-account-display.js | 4 ++-- lib/components/user/new-account-wizard.js | 4 ++-- .../user/notification-prefs-pane.js | 13 ++++++----- lib/components/user/user-account-screen.js | 22 ++++++++++--------- 5 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 92fbc5403..62f002f18 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -197,7 +197,7 @@ export function deleteUserMonitoredTrip (id) { /** * Initiates the phone number verification process for the logged-in user. */ -export function startUserPhoneVerification (originalUserData, newPhoneNumber) { +export function requestPhoneVerificationCode (originalUserData, newPhoneNumber) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence @@ -245,7 +245,7 @@ export function startUserPhoneVerification (originalUserData, newPhoneNumber) { /** * Sends the phone number verification code for the logged-in user. */ -export function sendUserPhoneVerification (originalUserData, code) { +export function sendPhoneVerificationCode (originalUserData, code) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence diff --git a/lib/components/user/existing-account-display.js b/lib/components/user/existing-account-display.js index ef1ee64fd..b1a7f44f9 100644 --- a/lib/components/user/existing-account-display.js +++ b/lib/components/user/existing-account-display.js @@ -8,7 +8,7 @@ import StackedPaneDisplay from './stacked-pane-display' */ class ExistingAccountDisplay extends Component { render () { - const { onCancel, onComplete, onRevertUserPhoneNumber, onSendPhoneVerification, onStartPhoneVerification, panes } = this.props + const { onCancel, onComplete, onRevertUserPhoneNumber, onSendPhoneVerificationCode, onRequestPhoneVerificationCode, panes } = this.props const paneSequence = [ { pane: () =>

Edit my trips

, @@ -21,7 +21,7 @@ class ExistingAccountDisplay extends Component { }, { pane: panes.notifications, - props: { onRevertUserPhoneNumber, onSendPhoneVerification, onStartPhoneVerification }, + props: { onRevertUserPhoneNumber, onSendPhoneVerificationCode, onRequestPhoneVerificationCode }, title: 'Notifications' }, { diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 47b469456..48ee4c314 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -5,7 +5,7 @@ import SequentialPaneDisplay from './sequential-pane-display' /** * This component is the new account wizard. */ -const NewAccountWizard = ({ onComplete, onCreate, onSendPhoneVerification, onStartPhoneVerification, panes, userData }) => { +const NewAccountWizard = ({ onComplete, onCreate, onSendPhoneVerificationCode, onRequestPhoneVerificationCode, panes, userData }) => { const { hasConsentedToTerms, notificationChannel = 'email' @@ -24,7 +24,7 @@ const NewAccountWizard = ({ onComplete, onCreate, onSendPhoneVerification, onSta nextId: 'places', pane: panes.notifications, prevId: 'terms', - props: { onSendPhoneVerification, onStartPhoneVerification }, + props: { onSendPhoneVerificationCode, onRequestPhoneVerificationCode }, title: 'Notification preferences' }, places: { diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index a649c5907..fb9583c13 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -30,8 +30,8 @@ const Details = styled.div` */ class NotificationPrefsPane extends Component { static propTypes = { - onStartPhoneVerification: PropTypes.func, - onSendPhoneVerification: PropTypes.func, + onRequestPhoneVerificationCode: PropTypes.func, + onSendPhoneVerificationCode: PropTypes.func, onUserDataChange: PropTypes.func.isRequired, userData: PropTypes.object.isRequired } @@ -102,7 +102,7 @@ class NotificationPrefsPane extends Component { _handleStartPhoneVerification = () => { // Send phone verification request. - this.props.onStartPhoneVerification() + this.props.onRequestPhoneVerificationCode() // Prompt for code. this.setState({ @@ -113,7 +113,7 @@ class NotificationPrefsPane extends Component { _handleSendPhoneVerification = async () => { // Send phone verification code. - await this.props.onSendPhoneVerification(this.state.phoneValidationCode) + await this.props.onSendPhoneVerificationCode(this.state.phoneValidationCode) // Exit verification modal and update "initialPhoneNumber" if isPhoneNumberVerified was changed to true. // Erase the verification code in all cases for discretion. @@ -241,7 +241,7 @@ class NotificationPrefsPane extends Component { {/* The dialog prompt for validation code. */} - + Enter verification code @@ -255,6 +255,9 @@ class NotificationPrefsPane extends Component { Verification code: +

+ +

diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 29c543ffa..7478cd815 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -113,8 +113,10 @@ class UserAccountScreen extends Component { /** * Requests a phone verification code. + * This handler is called when the user clicks "Verify my phone" after entering a new number, + * and also when the user clicks "Request a new code" from the verification modal. */ - _handleStartPhoneVerification = async () => { + _handleRequestPhoneVerificationCode = async () => { const { lastPhoneNumberRequested, lastPhoneRequestTime, userData } = this.state const { phoneNumber } = userData const timestamp = new Date() @@ -129,16 +131,16 @@ class UserAccountScreen extends Component { }) // Use the original user data to avoid persisting any other pending edits. - await this.props.startUserPhoneVerification(this.props.loggedInUser, phoneNumber) + await this.props.requestPhoneVerificationCode(this.props.loggedInUser, phoneNumber) } } /** * Sends the phone verification code. */ - _handleSendPhoneVerification = async code => { + _handleSendPhoneVerificationCode = async code => { // Use the original user data to avoid persisting any other pending edits. - await this.props.sendUserPhoneVerification(this.props.loggedInUser, code) + await this.props.sendPhoneVerificationCode(this.props.loggedInUser, code) // state.user.isPhoneNumberVerified will be set to true on success. // Clear previous phone number and state if phone is verified. @@ -215,9 +217,9 @@ class UserAccountScreen extends Component { @@ -229,9 +231,9 @@ class UserAccountScreen extends Component { ) @@ -259,9 +261,9 @@ const mapStateToProps = (state, ownProps) => { const mapDispatchToProps = { createOrUpdateUser: userActions.createOrUpdateUser, + requestPhoneVerificationCode: userActions.requestPhoneVerificationCode, routeTo: uiActions.routeTo, - sendUserPhoneVerification: userActions.sendUserPhoneVerification, - startUserPhoneVerification: userActions.startUserPhoneVerification + sendPhoneVerificationCode: userActions.sendPhoneVerificationCode } export default withLoggedInUserSupport( From f821a257b42a1d59c22c8bc001d95ca4cf585f0a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Sep 2020 16:46:45 -0400 Subject: [PATCH 10/55] refactor: Rename symbols and tweak comments. --- lib/actions/user.js | 25 +++---- .../user/existing-account-display.js | 70 ++++++++++--------- lib/components/user/new-account-wizard.js | 9 ++- .../user/notification-prefs-pane.js | 45 +++++++----- .../user/sequential-pane-display.js | 4 +- lib/components/user/user-account-screen.js | 16 +++-- lib/util/middleware.js | 4 +- 7 files changed, 95 insertions(+), 78 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 62f002f18..28b41d818 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -7,8 +7,8 @@ import { deleteTrip, fetchUser, getTrips, - startPhoneVerification, - sendPhoneVerification, + getPhoneVerificationCode, + postPhoneVerificationCode, updateTrip, updateUser } from '../util/middleware' @@ -98,6 +98,8 @@ export function fetchOrInitializeUser (auth) { /** * Updates (or creates) a user entry in the middleware, * then, if that was successful, updates the redux state with that user. + * @param userData the user entry to persist. + * @param silentOnSuccess true to suppress the confirmation if the operation is successful (e.g. immediately after user accepts the terms). */ export function createOrUpdateUser (userData, silentOnSuccess = false) { return async function (dispatch, getState) { @@ -131,13 +133,6 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) { } } -/** - * Silent-on-success version of function above. - */ -export function createOrUpdateUserSilent (userData) { - return createOrUpdateUser(userData, true) -} - /** * Updates a logged-in user's monitored trip, * then, if that was successful, refreshes the redux monitoredTrips @@ -195,7 +190,7 @@ export function deleteUserMonitoredTrip (id) { } /** - * Initiates the phone number verification process for the logged-in user. + * Requests a verification code for the logged-in user. */ export function requestPhoneVerificationCode (originalUserData, newPhoneNumber) { return async function (dispatch, getState) { @@ -222,15 +217,15 @@ export function requestPhoneVerificationCode (originalUserData, newPhoneNumber) const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) if (userUpdateResult.status === 'success' && userUpdateResult.data) { - const startPhoneVerificationResult = await startPhoneVerification(otpMiddleware, accessToken, userData.id) + const startPhoneVerificationResult = await getPhoneVerificationCode(otpMiddleware, accessToken, userData.id) if (startPhoneVerificationResult.status === 'success') { // Update application state on success. dispatch(setCurrentUser({ accessToken, user: userUpdateResult.data })) } else { alert(`An error was encountered:\n${JSON.stringify(startPhoneVerificationResult)}`) - // Also if there was an error in sending the verificaton request, - // Revert the phone number and verification status in the database. + // FIXME: Also if there was an error in sending the verificaton request, + // revert the phone number and verification status in the database. userData.phoneNumber = previousPhoneNumber userData.isPhoneNumberVerified = previousIsPhoneNumberVerified await updateUser(otpMiddleware, accessToken, userData) @@ -252,19 +247,19 @@ export function sendPhoneVerificationCode (originalUserData, code) { if (otpMiddleware) { const { accessToken } = user - const sendResult = await sendPhoneVerification(otpMiddleware, accessToken, originalUserData.id, code) + const sendResult = await postPhoneVerificationCode(otpMiddleware, accessToken, originalUserData.id, code) if (sendResult.status === 'success' && sendResult.data) { if (sendResult.data.status === 'approved') { // Make a clone of the original userData object. const userData = clone(originalUserData) // Update phone number and verification in database record. + // FIXME: The call to updateUer below assumes the middleware requires saving the user's phone number prior to verification. userData.isPhoneNumberVerified = true userData.notificationChannel = 'sms' const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) if (userUpdateResult.status === 'success' && userUpdateResult.data) { - // FIXME: This action and text assumes the middleware requires saving the user's phone number prior to verification. alert('Your phone is now verified and set to receive trip notifications.') // Update application state diff --git a/lib/components/user/existing-account-display.js b/lib/components/user/existing-account-display.js index b1a7f44f9..40cce473b 100644 --- a/lib/components/user/existing-account-display.js +++ b/lib/components/user/existing-account-display.js @@ -1,4 +1,4 @@ -import React, { Component } from 'react' +import React from 'react' import LinkButton from './link-button' import StackedPaneDisplay from './stacked-pane-display' @@ -6,39 +6,43 @@ import StackedPaneDisplay from './stacked-pane-display' /** * This component handles the existing account display. */ -class ExistingAccountDisplay extends Component { - render () { - const { onCancel, onComplete, onRevertUserPhoneNumber, onSendPhoneVerificationCode, onRequestPhoneVerificationCode, panes } = this.props - const paneSequence = [ - { - pane: () =>

Edit my trips

, - title: 'My trips' - }, - { - pane: panes.terms, - props: { disableCheckTerms: true }, - title: 'Terms' - }, - { - pane: panes.notifications, - props: { onRevertUserPhoneNumber, onSendPhoneVerificationCode, onRequestPhoneVerificationCode }, - title: 'Notifications' - }, - { - pane: panes.locations, - title: 'My locations' - } - ] +const ExistingAccountDisplay = ({ + onCancel, + onComplete, + onRequestPhoneVerificationCode, + onRevertUserPhoneNumber, + onSendPhoneVerificationCode, + panes +}) => { + const paneSequence = [ + { + pane: () =>

Edit my trips

, + title: 'My trips' + }, + { + pane: panes.terms, + props: { disableCheckTerms: true }, + title: 'Terms' + }, + { + pane: panes.notifications, + props: { onRequestPhoneVerificationCode, onRevertUserPhoneNumber, onSendPhoneVerificationCode }, + title: 'Notifications' + }, + { + pane: panes.locations, + title: 'My locations' + } + ] - return ( - - ) - } + return ( + + ) } export default ExistingAccountDisplay diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 48ee4c314..66f81c1d3 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -5,7 +5,14 @@ import SequentialPaneDisplay from './sequential-pane-display' /** * This component is the new account wizard. */ -const NewAccountWizard = ({ onComplete, onCreate, onSendPhoneVerificationCode, onRequestPhoneVerificationCode, panes, userData }) => { +const NewAccountWizard = ({ + onComplete, + onCreate, + onRequestPhoneVerificationCode, + onSendPhoneVerificationCode, + panes, + userData +}) => { const { hasConsentedToTerms, notificationChannel = 'email' diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index fb9583c13..27d24d4ee 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -1,6 +1,17 @@ import PropTypes from 'prop-types' import React, { Component } from 'react' -import { Button, ButtonToolbar, ControlLabel, FormControl, FormGroup, Glyphicon, HelpBlock, Modal, ToggleButton, ToggleButtonGroup } from 'react-bootstrap' +import { + Button, + ButtonToolbar, + ControlLabel, + FormControl, + FormGroup, + Glyphicon, + HelpBlock, + Modal, + ToggleButton, + ToggleButtonGroup +} from 'react-bootstrap' import styled from 'styled-components' const allowedNotificationChannels = [ @@ -56,11 +67,6 @@ class NotificationPrefsPane extends Component { onUserDataChange({ notificationChannel: e }) } - _handlePhoneNumberVerified = e => { - const { onUserDataChange } = this.props - onUserDataChange({ phoneNumber: e.target.value }) - } - _handlePhoneNumberChange = e => { const { onUserDataChange } = this.props onUserDataChange({ phoneNumber: e.target.value }) @@ -100,7 +106,7 @@ class NotificationPrefsPane extends Component { } } - _handleStartPhoneVerification = () => { + _handleRequestPhoneVerificationCode = () => { // Send phone verification request. this.props.onRequestPhoneVerificationCode() @@ -111,12 +117,11 @@ class NotificationPrefsPane extends Component { }) } - _handleSendPhoneVerification = async () => { - // Send phone verification code. + _handleSendPhoneVerificationCode = async () => { await this.props.onSendPhoneVerificationCode(this.state.phoneValidationCode) // Exit verification modal and update "initialPhoneNumber" if isPhoneNumberVerified was changed to true. - // Erase the verification code in all cases for discretion. + // Erase the verification code in all cases. if (this.props.userData.isPhoneNumberVerified) { this.setState({ initialPhoneNumber: this.props.userData.phoneNumber, @@ -132,7 +137,12 @@ class NotificationPrefsPane extends Component { render () { const { userData } = this.props - const { isPhoneFieldModified, isVerificationPending, initialPhoneNumber, phoneValidationCode } = this.state + const { + initialPhoneNumber, + isPhoneFieldModified, + isVerificationPending, + phoneValidationCode + } = this.state const { email, isPhoneNumberVerified, @@ -172,7 +182,7 @@ class NotificationPrefsPane extends Component { phoneFieldValidationState = 'success' } else { phoneStatusGlyph = 'remove' - phoneStatusText = 'Verification required.' + phoneStatusText = 'Verification required' phoneFieldValidationState = 'error' shouldVerifyPhone = true } @@ -222,9 +232,8 @@ class NotificationPrefsPane extends Component { type='tel' value={phoneNumber} /> - {/* Show glyphs underneath the input control - (there are some alignment issues with in mobile view), - so we use instead. */} + {/* Show glyphs underneath the input control with + (there are some alignment issues with in mobile view). */} {phoneStatusGlyph && } {phoneStatusText} @@ -232,7 +241,7 @@ class NotificationPrefsPane extends Component { {shouldVerifyPhone && (
- +
)} @@ -256,13 +265,13 @@ class NotificationPrefsPane extends Component {

- +

- + diff --git a/lib/components/user/sequential-pane-display.js b/lib/components/user/sequential-pane-display.js index b0d12883b..b6f4c3a9a 100644 --- a/lib/components/user/sequential-pane-display.js +++ b/lib/components/user/sequential-pane-display.js @@ -44,8 +44,8 @@ class SequentialPaneDisplay extends Component { this.setState({ activePaneId: nextId }) - } else if (onComplete) { - onComplete() // FIXME: use await? + } else if (typeof onComplete === 'function') { + onComplete() } } diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 7478cd815..a56b342ef 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -30,15 +30,13 @@ class UserAccountScreen extends Component { // isNewUser(loggedInUser) will change to false as the database gets updated. isNewUser: isNewUser(props.loggedInUser), - // Last number and last time we requested a code for (to avoid sending SMS over and over to verify the same number). + // Last number and last time we requested a code for (to avoid repeat SMS and not waste SMS quota). lastPhoneNumberRequested: null, lastPhoneRequestTime: null, - // Previous phone verification status - // if the user needs to revert their phone number in the middle of the verification process. + // Previous phone number and verification status, in case the + // user needs to revert their phone number after a code has been requested. previousIsPhoneNumberVerified: null, - - // Previous phone number if the user needs to revert it in the middle of the verification process. previousPhoneNumber: null, // Work on a copy of the logged-in user data captured when this component is created. @@ -83,6 +81,9 @@ class UserAccountScreen extends Component { } _handleCreateNewUser = async () => { + // Silently create the user record upon user accepting terms. + // Creating the user record before the user finishes the account creation steps + // is required by the middleware in order to perform phone verification. await this._updateUserPrefs(true) } @@ -100,7 +101,6 @@ class UserAccountScreen extends Component { * Reverts the user phone number and verification state in the database. */ _handleRevertPhoneNumber = () => { - // FIXME: This assumes the pending phone number must be saved in the database prior to verification. const { createOrUpdateUser, loggedInUser } = this.props // Make a clone of the original userData object. @@ -108,6 +108,8 @@ class UserAccountScreen extends Component { userData.phoneNumber = this.state.previousPhoneNumber userData.isPhoneNumberVerified = this.state.previousIsPhoneNumberVerified + // FIXME: Silently update the user record. + // (This assumes the pending phone number must be saved in the database prior to verification.) createOrUpdateUser(userData, true) } @@ -177,7 +179,7 @@ class UserAccountScreen extends Component { componentDidUpdate (prevProps) { // We need to update some fields, but not erase the user's other pending changes // when the loggedInUser record has been updated while this screen is shown, e.g.: - // - when a new user clicks next after agreeing on terms, + // - when a new user clicks "Next" after agreeing on terms, // - when the phone verification status changes (this is a middleware constraint). const { loggedInUser } = this.props if (!isEqual(prevProps.loggedInUser, loggedInUser)) { diff --git a/lib/util/middleware.js b/lib/util/middleware.js index b571fea3f..2447c7ed6 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -141,7 +141,7 @@ export async function deleteTrip (middlewareConfig, token, id) { } } -export async function startPhoneVerification (middlewareConfig, token, userId) { +export async function getPhoneVerificationCode (middlewareConfig, token, userId) { // TODO: There is potential to combine the variable extraction/assignments // in this method and its peers // with the variable extraction/assignments in lib/actions/user.js. @@ -151,7 +151,7 @@ export async function startPhoneVerification (middlewareConfig, token, userId) { return secureFetch(requestUrl, token, apiKey, 'GET') } -export async function sendPhoneVerification (middlewareConfig, token, userId, code) { +export async function postPhoneVerificationCode (middlewareConfig, token, userId, code) { const { apiBaseUrl, apiKey } = middlewareConfig const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${code}` From 586fa23270d610bfc878f130715abfd7a3c75f0a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 8 Sep 2020 16:55:33 -0400 Subject: [PATCH 11/55] refactor(NotificationPrefsPane): Move styles into styled component. --- lib/components/user/notification-prefs-pane.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 27d24d4ee..c0e254ea0 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -35,6 +35,11 @@ const Details = styled.div` min-height: 150px; margin-bottom: 15px; ` +const ButtonStrip = styled.div` + > * { + margin-right: 4px; + } +` /** * User notification preferences pane. @@ -240,10 +245,10 @@ class NotificationPrefsPane extends Component { {shouldVerifyPhone && ( -
+ - -
+ + )} )} From 626fdb7a50e90c7a827c98e3d9a00d29f2025365 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 15 Sep 2020 11:09:33 -0400 Subject: [PATCH 12/55] refactor: Address PR comments. --- lib/actions/user.js | 48 +++++++++---------- .../user/notification-prefs-pane.js | 6 +-- lib/components/user/user-account-screen.js | 8 ++-- lib/util/middleware.js | 4 +- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 28b41d818..47c34f068 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -1,4 +1,4 @@ -import clone from 'lodash/cloneDeep' +import clone from 'clone' import { createAction } from 'redux-actions' import { @@ -7,8 +7,8 @@ import { deleteTrip, fetchUser, getTrips, - getPhoneVerificationCode, - postPhoneVerificationCode, + sendPhoneVerificationSms, + validatePhoneVerificationCode, updateTrip, updateUser } from '../util/middleware' @@ -190,9 +190,9 @@ export function deleteUserMonitoredTrip (id) { } /** - * Requests a verification code for the logged-in user. + * Requests a verification code via SMS for the logged-in user. */ -export function requestPhoneVerificationCode (originalUserData, newPhoneNumber) { +export function requestPhoneVerificationSms (originalUserData, newPhoneNumber) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence @@ -207,28 +207,28 @@ export function requestPhoneVerificationCode (originalUserData, newPhoneNumber) // TODO: Figure out what should happen if the user refreshes the browser at this stage. // Make a clone of the original userData object. - const userData = clone(originalUserData) - const previousPhoneNumber = userData.phoneNumber - const previousIsPhoneNumberVerified = userData.isPhoneNumberVerified + const newUserData = clone(originalUserData) + const previousPhoneNumber = newUserData.phoneNumber + const previousIsPhoneNumberVerified = newUserData.isPhoneNumberVerified - userData.phoneNumber = newPhoneNumber - userData.isPhoneNumberVerified = false + newUserData.phoneNumber = newPhoneNumber + newUserData.isPhoneNumberVerified = false - const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) + const userUpdateResult = await updateUser(otpMiddleware, accessToken, newUserData) if (userUpdateResult.status === 'success' && userUpdateResult.data) { - const startPhoneVerificationResult = await getPhoneVerificationCode(otpMiddleware, accessToken, userData.id) - if (startPhoneVerificationResult.status === 'success') { + const sendSmsResult = await sendPhoneVerificationSms(otpMiddleware, accessToken, newUserData.id) + if (sendSmsResult.status === 'success') { // Update application state on success. dispatch(setCurrentUser({ accessToken, user: userUpdateResult.data })) } else { - alert(`An error was encountered:\n${JSON.stringify(startPhoneVerificationResult)}`) + alert(`An error was encountered:\n${JSON.stringify(sendSmsResult)}`) // FIXME: Also if there was an error in sending the verificaton request, // revert the phone number and verification status in the database. - userData.phoneNumber = previousPhoneNumber - userData.isPhoneNumberVerified = previousIsPhoneNumberVerified - await updateUser(otpMiddleware, accessToken, userData) + newUserData.phoneNumber = previousPhoneNumber + newUserData.isPhoneNumberVerified = previousIsPhoneNumberVerified + await updateUser(otpMiddleware, accessToken, newUserData) } } else { alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) @@ -238,26 +238,26 @@ export function requestPhoneVerificationCode (originalUserData, newPhoneNumber) } /** - * Sends the phone number verification code for the logged-in user. + * Validate the phone number verification code for the logged-in user. */ -export function sendPhoneVerificationCode (originalUserData, code) { +export function verifyPhoneNumber (originalUserData, code) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence if (otpMiddleware) { const { accessToken } = user - const sendResult = await postPhoneVerificationCode(otpMiddleware, accessToken, originalUserData.id, code) + const sendResult = await validatePhoneVerificationCode(otpMiddleware, accessToken, originalUserData.id, code) if (sendResult.status === 'success' && sendResult.data) { if (sendResult.data.status === 'approved') { // Make a clone of the original userData object. - const userData = clone(originalUserData) + const newUserData = clone(originalUserData) // Update phone number and verification in database record. // FIXME: The call to updateUer below assumes the middleware requires saving the user's phone number prior to verification. - userData.isPhoneNumberVerified = true - userData.notificationChannel = 'sms' - const userUpdateResult = await updateUser(otpMiddleware, accessToken, userData) + newUserData.isPhoneNumberVerified = true + newUserData.notificationChannel = 'sms' + const userUpdateResult = await updateUser(otpMiddleware, accessToken, newUserData) if (userUpdateResult.status === 'success' && userUpdateResult.data) { alert('Your phone is now verified and set to receive trip notifications.') diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index c0e254ea0..5ad59117f 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -186,9 +186,9 @@ class NotificationPrefsPane extends Component { phoneStatusText = 'Verified' phoneFieldValidationState = 'success' } else { - phoneStatusGlyph = 'remove' + phoneStatusGlyph = 'warning-sign' phoneStatusText = 'Verification required' - phoneFieldValidationState = 'error' + phoneFieldValidationState = 'warning' shouldVerifyPhone = true } @@ -233,7 +233,7 @@ class NotificationPrefsPane extends Component { diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index a56b342ef..1861cf6c5 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -133,7 +133,7 @@ class UserAccountScreen extends Component { }) // Use the original user data to avoid persisting any other pending edits. - await this.props.requestPhoneVerificationCode(this.props.loggedInUser, phoneNumber) + await this.props.requestPhoneVerificationSms(this.props.loggedInUser, phoneNumber) } } @@ -142,7 +142,7 @@ class UserAccountScreen extends Component { */ _handleSendPhoneVerificationCode = async code => { // Use the original user data to avoid persisting any other pending edits. - await this.props.sendPhoneVerificationCode(this.props.loggedInUser, code) + await this.props.verifyPhoneNumber(this.props.loggedInUser, code) // state.user.isPhoneNumberVerified will be set to true on success. // Clear previous phone number and state if phone is verified. @@ -263,9 +263,9 @@ const mapStateToProps = (state, ownProps) => { const mapDispatchToProps = { createOrUpdateUser: userActions.createOrUpdateUser, - requestPhoneVerificationCode: userActions.requestPhoneVerificationCode, + requestPhoneVerificationSms: userActions.requestPhoneVerificationSms, routeTo: uiActions.routeTo, - sendPhoneVerificationCode: userActions.sendPhoneVerificationCode + verifyPhoneNumber: userActions.verifyPhoneNumber } export default withLoggedInUserSupport( diff --git a/lib/util/middleware.js b/lib/util/middleware.js index 2447c7ed6..f89b4749f 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -141,7 +141,7 @@ export async function deleteTrip (middlewareConfig, token, id) { } } -export async function getPhoneVerificationCode (middlewareConfig, token, userId) { +export async function sendPhoneVerificationSms (middlewareConfig, token, userId) { // TODO: There is potential to combine the variable extraction/assignments // in this method and its peers // with the variable extraction/assignments in lib/actions/user.js. @@ -151,7 +151,7 @@ export async function getPhoneVerificationCode (middlewareConfig, token, userId) return secureFetch(requestUrl, token, apiKey, 'GET') } -export async function postPhoneVerificationCode (middlewareConfig, token, userId, code) { +export async function validatePhoneVerificationCode (middlewareConfig, token, userId, code) { const { apiBaseUrl, apiKey } = middlewareConfig const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${code}` From 714d795f57a51f437eb93d77dc8ac0a5f57ed44c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 15 Sep 2020 11:12:18 -0400 Subject: [PATCH 13/55] docs(NotificationPrefsPane): Add comment+link regarding the fake phone number placeholder. --- lib/components/user/notification-prefs-pane.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 5ad59117f..f6924e921 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -233,6 +233,7 @@ class NotificationPrefsPane extends Component { Date: Fri, 2 Oct 2020 12:38:47 -0400 Subject: [PATCH 14/55] refactor(actions/user): Address PR comments. --- lib/actions/user.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 47c34f068..ad471a9a0 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -205,18 +205,17 @@ export function requestPhoneVerificationSms (originalUserData, newPhoneNumber) { // and cache the previous value and verification state so they can be reverted. // The full user record will be updated upon the user clicking Finish/Save preferences. // TODO: Figure out what should happen if the user refreshes the browser at this stage. + const previousPhoneNumber = originalUserData.phoneNumber + const previousIsPhoneNumberVerified = originalUserData.isPhoneNumberVerified - // Make a clone of the original userData object. + // Make a clone of the original userData object and persist it (temporarily). const newUserData = clone(originalUserData) - const previousPhoneNumber = newUserData.phoneNumber - const previousIsPhoneNumberVerified = newUserData.isPhoneNumberVerified - newUserData.phoneNumber = newPhoneNumber newUserData.isPhoneNumberVerified = false - const userUpdateResult = await updateUser(otpMiddleware, accessToken, newUserData) if (userUpdateResult.status === 'success' && userUpdateResult.data) { + // With the user's record updated, send the SMS request. const sendSmsResult = await sendPhoneVerificationSms(otpMiddleware, accessToken, newUserData.id) if (sendSmsResult.status === 'success') { // Update application state on success. @@ -248,21 +247,22 @@ export function verifyPhoneNumber (originalUserData, code) { if (otpMiddleware) { const { accessToken } = user const sendResult = await validatePhoneVerificationCode(otpMiddleware, accessToken, originalUserData.id, code) + + // If the check is successful, status in the returned data will be "approved". if (sendResult.status === 'success' && sendResult.data) { if (sendResult.data.status === 'approved') { // Make a clone of the original userData object. const newUserData = clone(originalUserData) // Update phone number and verification in database record. - // FIXME: The call to updateUer below assumes the middleware requires saving the user's phone number prior to verification. + // FIXME: The call to updateUser below assumes the middleware requires saving the user's phone number prior to verification. newUserData.isPhoneNumberVerified = true newUserData.notificationChannel = 'sms' const userUpdateResult = await updateUser(otpMiddleware, accessToken, newUserData) if (userUpdateResult.status === 'success' && userUpdateResult.data) { - alert('Your phone is now verified and set to receive trip notifications.') - - // Update application state + // Update application state. + // The new phone verification status will be shown underneath the phone number. dispatch(setCurrentUser({ accessToken, user: userUpdateResult.data })) } else { alert(`Error updating your phone's verified status:\n${JSON.stringify(sendResult)}`) From 2dd7e108921359a433f572fbd9e77aa6c866a278 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 2 Oct 2020 16:47:27 -0400 Subject: [PATCH 15/55] refactor(actions/user): Extract common code from all methods. --- lib/actions/user.js | 214 ++++++++++++++++++++++---------------------- 1 file changed, 105 insertions(+), 109 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index b5c094073..0a47f8e4c 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -28,102 +28,111 @@ function createNewUser (auth0User) { } /** - * Fetches the saved/monitored trips for a user. - * We use the accessToken to fetch the data regardless of - * whether the process to populate state.user is completed or not. + * Helper function that + * - extracts key variables from the state + * (state.otp.config.persistence.otp_middleware, state.user.accessToken, state.user.loggedIUnUser) + * - checks that otp_middleware is set, and throws an error if not. + * @param functionToExecute the code to execute, with parameters (dispatch, otpMiddleware, accessToken, loggedInUser) + * @return an action of type () */ -export function fetchUserMonitoredTrips (accessToken) { +function executeWithMiddleware (functionToExecute) { return async function (dispatch, getState) { - const { otp } = getState() + const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence if (otpMiddleware) { - const { data: trips, status: fetchStatus } = await getTrips(otpMiddleware, accessToken) - if (fetchStatus === 'success') { - dispatch(setCurrentUserMonitoredTrips(trips)) - } + const { accessToken, loggedInUser } = user + functionToExecute(dispatch, otpMiddleware, accessToken, loggedInUser) + } else { + throw new Error('This action requires a valid middleware configuration.') } } } +/** + * Fetches the saved/monitored trips for a user. + * We use the accessToken to fetch the data regardless of + * whether the process to populate state.user is completed or not. + */ +export function fetchUserMonitoredTrips (accessToken) { + return executeWithMiddleware(async (dispatch, otpMiddleware) => { + const { data: trips, status: fetchStatus } = await getTrips(otpMiddleware, accessToken) + if (fetchStatus === 'success') { + dispatch(setCurrentUserMonitoredTrips(trips)) + } + }) +} + /** * Fetches user preferences to state.user, or set initial values under state.user if no user has been loaded. */ export function fetchOrInitializeUser (auth) { - return async function (dispatch, getState) { - const { otp } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence - - if (otpMiddleware) { - const { accessToken, user: authUser } = auth - const { data: user, status: fetchUserStatus } = await fetchUser(otpMiddleware, accessToken) - - // Beware! On AWS API gateway, if a user is not found in the middleware - // (e.g. they just created their Auth0 password but have not completed the account setup form yet), - // the call above will return, for example: - // { - // status: 'success', - // data: { - // "result": "ERR", - // "message": "No user with id=000000 found.", - // "code": 404, - // "detail": null - // } - // } - // - // The same call to a middleware instance that is not behind an API gateway - // will return: - // { - // status: 'error', - // message: 'Error get-ing user...' - // } - // TODO: Improve AWS response. - - const isNewAccount = fetchUserStatus === 'error' || (user && user.result === 'ERR') - if (!isNewAccount) { - // Load user's monitored trips before setting the user state. - await dispatch(fetchUserMonitoredTrips(accessToken)) - - dispatch(setCurrentUser({ accessToken, user })) - } else { - dispatch(setCurrentUser({ accessToken, user: createNewUser(authUser) })) - } + return executeWithMiddleware(async (dispatch, otpMiddleware) => { + const { accessToken, user: authUser } = auth + const { data: user, status: fetchUserStatus } = await fetchUser(otpMiddleware, accessToken) + + // Beware! On AWS API gateway, if a user is not found in the middleware + // (e.g. they just created their Auth0 password but have not completed the account setup form yet), + // the call above will return, for example: + // { + // status: 'success', + // data: { + // "result": "ERR", + // "message": "No user with id=000000 found.", + // "code": 404, + // "detail": null + // } + // } + // + // The same call to a middleware instance that is not behind an API gateway + // will return: + // { + // status: 'error', + // message: 'Error get-ing user...' + // } + // TODO: Improve AWS response. + + const isNewAccount = fetchUserStatus === 'error' || (user && user.result === 'ERR') + if (!isNewAccount) { + // Load user's monitored trips before setting the user state. + await dispatch(fetchUserMonitoredTrips(accessToken)) + + dispatch(setCurrentUser({ accessToken, user })) + } else { + dispatch(setCurrentUser({ accessToken, user: createNewUser(authUser) })) } - } + }) } /** * Updates (or creates) a user entry in the middleware, * then, if that was successful, updates the redux state with that user. + * @param userData the user entry to persist. + * @param silentOnSuccess true to suppress the confirmation if the operation is successful (e.g. immediately after user accepts the terms). */ -export function createOrUpdateUser (userData) { - return async function (dispatch, getState) { - const { otp, user } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence - - if (otpMiddleware) { - const { accessToken, loggedInUser } = user - - let result - if (isNewUser(loggedInUser)) { - result = await addUser(otpMiddleware, accessToken, userData) - } else { - result = await updateUser(otpMiddleware, accessToken, userData) - } +export function createOrUpdateUser (userData, silentOnSuccess = false) { + return executeWithMiddleware(async (dispatch, otpMiddleware, accessToken, loggedInUser) => { + let result + if (isNewUser(loggedInUser)) { + result = await addUser(otpMiddleware, accessToken, userData) + } else { + result = await updateUser(otpMiddleware, accessToken, userData) + } - // TODO: improve the UI feedback messages for this. - if (result.status === 'success' && result.data) { + // TODO: improve the UI feedback messages for this. + if (result.status === 'success' && result.data) { + if (!silentOnSuccess) { alert('Your preferences have been saved.') - - // Update application state with the user entry as saved - // (as returned) by the middleware. - const userData = result.data - dispatch(setCurrentUser({ accessToken, user: userData })) - } else { - alert(`An error was encountered:\n${JSON.stringify(result)}`) } + + // Update application state with the user entry as saved + // (as returned) by the middleware. + const newUserData = result.data + dispatch(setCurrentUser({ accessToken, user: newUserData })) + } else { + alert(`An error was encountered:\n${JSON.stringify(result)}`) } - } + }) } /** @@ -132,31 +141,24 @@ export function createOrUpdateUser (userData) { * with the updated trip. */ export function createOrUpdateUserMonitoredTrip (tripData, isNew) { - return async function (dispatch, getState) { - const { otp, user } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence - - if (otpMiddleware) { - const { accessToken } = user - - let result - if (isNew) { - result = await addTrip(otpMiddleware, accessToken, tripData) - } else { - result = await updateTrip(otpMiddleware, accessToken, tripData) - } + return executeWithMiddleware(async (dispatch, otpMiddleware, accessToken) => { + let result + if (isNew) { + result = await addTrip(otpMiddleware, accessToken, tripData) + } else { + result = await updateTrip(otpMiddleware, accessToken, tripData) + } - // TODO: improve the UI feedback messages for this. - if (result.status === 'success' && result.data) { - alert('Your preferences have been saved.') + // TODO: improve the UI feedback messages for this. + if (result.status === 'success' && result.data) { + alert('Your preferences have been saved.') - // Reload user's monitored trips after add/update. - await dispatch(fetchUserMonitoredTrips(accessToken)) - } else { - alert(`An error was encountered:\n${JSON.stringify(result)}`) - } + // Reload user's monitored trips after add/update. + await dispatch(fetchUserMonitoredTrips(accessToken)) + } else { + alert(`An error was encountered:\n${JSON.stringify(result)}`) } - } + }) } /** @@ -164,20 +166,14 @@ export function createOrUpdateUserMonitoredTrip (tripData, isNew) { * then, if that was successful, refreshes the redux monitoredTrips state. */ export function deleteUserMonitoredTrip (id) { - return async function (dispatch, getState) { - const { otp, user } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence - - if (otpMiddleware) { - const { accessToken } = user - const deleteResult = await deleteTrip(otpMiddleware, accessToken, id) - - if (deleteResult.status === 'success') { - // Reload user's monitored trips after deletion. - await dispatch(fetchUserMonitoredTrips(accessToken)) - } else { - alert(`An error was encountered:\n${JSON.stringify(deleteResult)}`) - } + return executeWithMiddleware(async (dispatch, otpMiddleware, accessToken) => { + const deleteResult = await deleteTrip(otpMiddleware, accessToken, id) + + if (deleteResult.status === 'success') { + // Reload user's monitored trips after deletion. + await dispatch(fetchUserMonitoredTrips(accessToken)) + } else { + alert(`An error was encountered:\n${JSON.stringify(deleteResult)}`) } - } + }) } From 495bf043875e1ff2ebea35c50e5d04545701c00c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 2 Oct 2020 18:08:27 -0400 Subject: [PATCH 16/55] refactor(actions/user): Refactor middleware methods. --- lib/actions/user.js | 146 ++++++++++++++++++++++++++--------------- lib/util/middleware.js | 84 ------------------------ 2 files changed, 92 insertions(+), 138 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 0a47f8e4c..06b7d64eb 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -1,16 +1,12 @@ import { createAction } from 'redux-actions' -import { - addTrip, - addUser, - deleteTrip, - fetchUser, - getTrips, - updateTrip, - updateUser -} from '../util/middleware' +import { secureFetch } from '../util/middleware' import { isNewUser } from '../util/user' +// Middleware API paths. +const API_MONITORTRIP_PATH = '/api/secure/monitoredtrip' +const API_USER_PATH = '/api/secure/user' + const setCurrentUser = createAction('SET_CURRENT_USER') const setCurrentUserMonitoredTrips = createAction('SET_CURRENT_USER_MONITORED_TRIPS') export const setPathBeforeSignIn = createAction('SET_PATH_BEFORE_SIGNIN') @@ -29,11 +25,12 @@ function createNewUser (auth0User) { /** * Helper function that - * - extracts key variables from the state - * (state.otp.config.persistence.otp_middleware, state.user.accessToken, state.user.loggedIUnUser) + * - extracts key variables from the state and passes them to the code to execute: + * - apiKey and apiBaseUrl from state.otp.config.persistence.otp_middleware, + * - accessToken and loggedIUnUser from state.user. * - checks that otp_middleware is set, and throws an error if not. - * @param functionToExecute the code to execute, with parameters (dispatch, otpMiddleware, accessToken, loggedInUser) - * @return an action of type () + * @param functionToExecute the code to execute, with parameters (dispatch, arguments) + * @return a redux action that will run if a middleware s configured. */ function executeWithMiddleware (functionToExecute) { return async function (dispatch, getState) { @@ -42,7 +39,13 @@ function executeWithMiddleware (functionToExecute) { if (otpMiddleware) { const { accessToken, loggedInUser } = user - functionToExecute(dispatch, otpMiddleware, accessToken, loggedInUser) + const { apiBaseUrl, apiKey } = otpMiddleware + await functionToExecute(dispatch, { + accessToken, + apiBaseUrl, + apiKey, + loggedInUser + }) } else { throw new Error('This action requires a valid middleware configuration.') } @@ -55,21 +58,26 @@ function executeWithMiddleware (functionToExecute) { * whether the process to populate state.user is completed or not. */ export function fetchUserMonitoredTrips (accessToken) { - return executeWithMiddleware(async (dispatch, otpMiddleware) => { - const { data: trips, status: fetchStatus } = await getTrips(otpMiddleware, accessToken) - if (fetchStatus === 'success') { + return executeWithMiddleware(async (dispatch, { apiBaseUrl, apiKey }) => { + const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` + + const { data: trips, status } = await secureFetch(requestUrl, accessToken, apiKey, 'GET') + if (status === 'success') { dispatch(setCurrentUserMonitoredTrips(trips)) } }) } /** - * Fetches user preferences to state.user, or set initial values under state.user if no user has been loaded. + * Fetches user preferences to state.user, + * or set initial values under state.user if no user has been loaded. */ export function fetchOrInitializeUser (auth) { - return executeWithMiddleware(async (dispatch, otpMiddleware) => { + return executeWithMiddleware(async (dispatch, { apiBaseUrl, apiKey }) => { const { accessToken, user: authUser } = auth - const { data: user, status: fetchUserStatus } = await fetchUser(otpMiddleware, accessToken) + const requestUrl = `${apiBaseUrl}${API_USER_PATH}/fromtoken` + + const { data: user, status } = await secureFetch(requestUrl, accessToken, apiKey) // Beware! On AWS API gateway, if a user is not found in the middleware // (e.g. they just created their Auth0 password but have not completed the account setup form yet), @@ -92,7 +100,7 @@ export function fetchOrInitializeUser (auth) { // } // TODO: Improve AWS response. - const isNewAccount = fetchUserStatus === 'error' || (user && user.result === 'ERR') + const isNewAccount = status === 'error' || (user && user.result === 'ERR') if (!isNewAccount) { // Load user's monitored trips before setting the user state. await dispatch(fetchUserMonitoredTrips(accessToken)) @@ -111,26 +119,38 @@ export function fetchOrInitializeUser (auth) { * @param silentOnSuccess true to suppress the confirmation if the operation is successful (e.g. immediately after user accepts the terms). */ export function createOrUpdateUser (userData, silentOnSuccess = false) { - return executeWithMiddleware(async (dispatch, otpMiddleware, accessToken, loggedInUser) => { - let result + return executeWithMiddleware(async (dispatch, { accessToken, apiBaseUrl, apiKey, loggedInUser }) => { + const { id } = userData // Middleware ID, NOT auth0 (or similar) id. + let requestUrl, method + + // Determine URL and method to use. if (isNewUser(loggedInUser)) { - result = await addUser(otpMiddleware, accessToken, userData) - } else { - result = await updateUser(otpMiddleware, accessToken, userData) + requestUrl = `${apiBaseUrl}${API_USER_PATH}` + method = 'POST' + } else if (id) { + requestUrl = `${apiBaseUrl}${API_USER_PATH}/${id}` + method = 'PUT' } - // TODO: improve the UI feedback messages for this. - if (result.status === 'success' && result.data) { - if (!silentOnSuccess) { - alert('Your preferences have been saved.') + if (requestUrl) { + const result = await secureFetch(requestUrl, accessToken, apiKey, method, { + body: JSON.stringify(userData) + }) + + // TODO: improve the UI feedback messages for this. + if (result.status === 'success' && result.data) { + if (!silentOnSuccess) { + alert('Your preferences have been saved.') + } + + // Update application state with the user entry as saved + // (as returned) by the middleware. + dispatch(setCurrentUser({ accessToken, user: result.data })) + } else { + alert(`An error was encountered:\n${JSON.stringify(result)}`) } - - // Update application state with the user entry as saved - // (as returned) by the middleware. - const newUserData = result.data - dispatch(setCurrentUser({ accessToken, user: newUserData })) } else { - alert(`An error was encountered:\n${JSON.stringify(result)}`) + alert('Corrupted state: User ID not available for exiting user.') } }) } @@ -141,22 +161,35 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) { * with the updated trip. */ export function createOrUpdateUserMonitoredTrip (tripData, isNew) { - return executeWithMiddleware(async (dispatch, otpMiddleware, accessToken) => { - let result + return executeWithMiddleware(async (dispatch, { accessToken, apiBaseUrl, apiKey }) => { + const { id } = tripData + let requestUrl, method + + // Determine URL and method to use. if (isNew) { - result = await addTrip(otpMiddleware, accessToken, tripData) - } else { - result = await updateTrip(otpMiddleware, accessToken, tripData) + requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` + method = 'POST' + } else if (id) { + requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${id}` + method = 'PUT' } - // TODO: improve the UI feedback messages for this. - if (result.status === 'success' && result.data) { - alert('Your preferences have been saved.') + if (requestUrl) { + const result = await secureFetch(requestUrl, accessToken, apiKey, method, { + body: JSON.stringify(tripData) + }) - // Reload user's monitored trips after add/update. - await dispatch(fetchUserMonitoredTrips(accessToken)) + // TODO: improve the UI feedback messages for this. + if (result.status === 'success' && result.data) { + alert('Your preferences have been saved.') + + // Reload user's monitored trips after add/update. + await dispatch(fetchUserMonitoredTrips(accessToken)) + } else { + alert(`An error was encountered:\n${JSON.stringify(result)}`) + } } else { - alert(`An error was encountered:\n${JSON.stringify(result)}`) + alert('Corrupted state: Trip ID not available for exiting trip.') } }) } @@ -166,14 +199,19 @@ export function createOrUpdateUserMonitoredTrip (tripData, isNew) { * then, if that was successful, refreshes the redux monitoredTrips state. */ export function deleteUserMonitoredTrip (id) { - return executeWithMiddleware(async (dispatch, otpMiddleware, accessToken) => { - const deleteResult = await deleteTrip(otpMiddleware, accessToken, id) - - if (deleteResult.status === 'success') { - // Reload user's monitored trips after deletion. - await dispatch(fetchUserMonitoredTrips(accessToken)) + return executeWithMiddleware(async (dispatch, { accessToken, apiBaseUrl, apiKey }) => { + const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${id}` + + if (id) { + const deleteResult = secureFetch(requestUrl, accessToken, apiKey, 'DELETE') + if (deleteResult.status === 'success') { + // Reload user's monitored trips after deletion. + await dispatch(fetchUserMonitoredTrips(accessToken)) + } else { + alert(`An error was encountered:\n${JSON.stringify(deleteResult)}`) + } } else { - alert(`An error was encountered:\n${JSON.stringify(deleteResult)}`) + alert('Corrupted state: Monitored Trip ID not available for exiting user.') } }) } diff --git a/lib/util/middleware.js b/lib/util/middleware.js index 10403b465..5db426133 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -1,8 +1,5 @@ if (typeof (fetch) === 'undefined') require('isomorphic-fetch') -const API_USER_PATH = '/api/secure/user' -const API_MONITORTRIP_PATH = '/api/secure/monitoredtrip' - /** * This method builds the options object for call to the fetch method. * @param {string} accessToken If non-null, a bearer Authorization header will be added with the specified token. @@ -58,84 +55,3 @@ export async function secureFetch (url, accessToken, apiKey, method = 'get', opt data: await res.json() } } - -// TODO: Move methods below to user/entity-specific files? -export async function fetchUser (middlewareConfig, token) { - const { apiBaseUrl, apiKey } = middlewareConfig - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/fromtoken` - - return secureFetch(requestUrl, token, apiKey) -} - -export async function addUser (middlewareConfig, token, data) { - const { apiBaseUrl, apiKey } = middlewareConfig - const requestUrl = `${apiBaseUrl}${API_USER_PATH}` - - return secureFetch(requestUrl, token, apiKey, 'POST', { - body: JSON.stringify(data) - }) -} - -export async function updateUser (middlewareConfig, token, data) { - const { apiBaseUrl, apiKey } = middlewareConfig - const { id } = data // Middleware ID, NOT auth0 (or similar) id. - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${id}` - - if (id) { - return secureFetch(requestUrl, token, apiKey, 'PUT', { - body: JSON.stringify(data) - }) - } else { - return { - status: 'error', - message: 'Corrupted state: User ID not available for exiting user.' - } - } -} - -export async function getTrips (middlewareConfig, token) { - const { apiBaseUrl, apiKey } = middlewareConfig - const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` - - return secureFetch(requestUrl, token, apiKey, 'GET') -} - -export async function addTrip (middlewareConfig, token, data) { - const { apiBaseUrl, apiKey } = middlewareConfig - const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` - - return secureFetch(requestUrl, token, apiKey, 'POST', { - body: JSON.stringify(data) - }) -} - -export async function updateTrip (middlewareConfig, token, data) { - const { apiBaseUrl, apiKey } = middlewareConfig - const { id } = data - const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${id}` - - if (id) { - return secureFetch(requestUrl, token, apiKey, 'PUT', { - body: JSON.stringify(data) - }) - } else { - return { - status: 'error', - message: 'Corrupted state: Monitored Trip ID not available for exiting user.' - } - } -} - -export async function deleteTrip (middlewareConfig, token, id) { - const { apiBaseUrl, apiKey } = middlewareConfig - const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${id}` - - if (id) { - return secureFetch(requestUrl, token, apiKey, 'DELETE') - } else { - return { - status: 'error', - message: 'Corrupted state: Monitored Trip ID not available for exiting user.' - } - } -} From 2154b287da71f57d043b8ef1152c0c6ab897dff6 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 5 Oct 2020 10:01:43 -0400 Subject: [PATCH 17/55] refactor(actions/user): Tweak comments per PR feedback. --- lib/actions/user.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 06b7d64eb..0bd30024a 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -29,8 +29,10 @@ function createNewUser (auth0User) { * - apiKey and apiBaseUrl from state.otp.config.persistence.otp_middleware, * - accessToken and loggedIUnUser from state.user. * - checks that otp_middleware is set, and throws an error if not. - * @param functionToExecute the code to execute, with parameters (dispatch, arguments) - * @return a redux action that will run if a middleware s configured. + * @param functionToExecute a function that can be waited upon, + * with parameters (dispatch, arguments), that contains the code to be + * executed if the OTP middleware is configured. + * @return a redux action for the code to be executed. */ function executeWithMiddleware (functionToExecute) { return async function (dispatch, getState) { From e8f9d39fc66f01d75d6c6eb4da2359dbf0b0165e Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 8 Oct 2020 11:24:36 -0400 Subject: [PATCH 18/55] fix(actions/user): Do not persist user before sending sms req/after phone validation. --- lib/actions/user.js | 59 ++++--------------- .../user/notification-prefs-pane.js | 2 +- lib/util/middleware.js | 7 +-- 3 files changed, 14 insertions(+), 54 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index ad471a9a0..4dc91aab7 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -1,4 +1,3 @@ -import clone from 'clone' import { createAction } from 'redux-actions' import { @@ -192,7 +191,7 @@ export function deleteUserMonitoredTrip (id) { /** * Requests a verification code via SMS for the logged-in user. */ -export function requestPhoneVerificationSms (originalUserData, newPhoneNumber) { +export function requestPhoneVerificationSms (userData, newPhoneNumber) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence @@ -200,37 +199,14 @@ export function requestPhoneVerificationSms (originalUserData, newPhoneNumber) { if (otpMiddleware) { const { accessToken } = user - // FIXME: (required by middleware) Only temporarily update the user's phone number - // in the database with the pending one, - // and cache the previous value and verification state so they can be reverted. - // The full user record will be updated upon the user clicking Finish/Save preferences. - // TODO: Figure out what should happen if the user refreshes the browser at this stage. - const previousPhoneNumber = originalUserData.phoneNumber - const previousIsPhoneNumberVerified = originalUserData.isPhoneNumberVerified - - // Make a clone of the original userData object and persist it (temporarily). - const newUserData = clone(originalUserData) - newUserData.phoneNumber = newPhoneNumber - newUserData.isPhoneNumberVerified = false - const userUpdateResult = await updateUser(otpMiddleware, accessToken, newUserData) - - if (userUpdateResult.status === 'success' && userUpdateResult.data) { - // With the user's record updated, send the SMS request. - const sendSmsResult = await sendPhoneVerificationSms(otpMiddleware, accessToken, newUserData.id) - if (sendSmsResult.status === 'success') { - // Update application state on success. - dispatch(setCurrentUser({ accessToken, user: userUpdateResult.data })) - } else { - alert(`An error was encountered:\n${JSON.stringify(sendSmsResult)}`) - - // FIXME: Also if there was an error in sending the verificaton request, - // revert the phone number and verification status in the database. - newUserData.phoneNumber = previousPhoneNumber - newUserData.isPhoneNumberVerified = previousIsPhoneNumberVerified - await updateUser(otpMiddleware, accessToken, newUserData) - } + // Send the SMS request with the phone number to update. + const sendSmsResult = await sendPhoneVerificationSms(otpMiddleware, accessToken, userData.id, newPhoneNumber) + if (sendSmsResult.status === 'success') { + // Refetch user and update application state with new phone number and verification status. + // (This also refetches the user's monitored trip, and that's ok.) + await dispatch(fetchOrInitializeUser({ accessToken })) } else { - alert(`An error was encountered:\n${JSON.stringify(userUpdateResult)}`) + alert(`An error was encountered:\n${JSON.stringify(sendSmsResult)}`) } } } @@ -251,22 +227,9 @@ export function verifyPhoneNumber (originalUserData, code) { // If the check is successful, status in the returned data will be "approved". if (sendResult.status === 'success' && sendResult.data) { if (sendResult.data.status === 'approved') { - // Make a clone of the original userData object. - const newUserData = clone(originalUserData) - - // Update phone number and verification in database record. - // FIXME: The call to updateUser below assumes the middleware requires saving the user's phone number prior to verification. - newUserData.isPhoneNumberVerified = true - newUserData.notificationChannel = 'sms' - const userUpdateResult = await updateUser(otpMiddleware, accessToken, newUserData) - - if (userUpdateResult.status === 'success' && userUpdateResult.data) { - // Update application state. - // The new phone verification status will be shown underneath the phone number. - dispatch(setCurrentUser({ accessToken, user: userUpdateResult.data })) - } else { - alert(`Error updating your phone's verified status:\n${JSON.stringify(sendResult)}`) - } + // Refetch user and update application state with new phone number and verification status. + // (This also refetches the user's monitored trip, and that's ok.) + await dispatch(fetchOrInitializeUser({ accessToken })) } else { alert('You entered in incorrect validation code. Please try again.') } diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index f6924e921..8c262b662 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -163,7 +163,7 @@ class NotificationPrefsPane extends Component { // - Viewing a verified phone number (phoneNumber non-blank, same as initial, verified) // => green, "[V] Verified" indication. // - Editing a phone number (phoneNumber non-blank, different than initial or not verified) - // => red, "[X] Verification required" indication. + // => yellow, "[!] Verification required" indication. const isPhoneNumberBlank = !(phoneNumber && phoneNumber.length) const isPhoneNumberSameAsInitial = phoneNumber === initialPhoneNumber diff --git a/lib/util/middleware.js b/lib/util/middleware.js index f89b4749f..8ed02bd53 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -141,12 +141,9 @@ export async function deleteTrip (middlewareConfig, token, id) { } } -export async function sendPhoneVerificationSms (middlewareConfig, token, userId) { - // TODO: There is potential to combine the variable extraction/assignments - // in this method and its peers - // with the variable extraction/assignments in lib/actions/user.js. +export async function sendPhoneVerificationSms (middlewareConfig, token, userId, phoneNumber) { const { apiBaseUrl, apiKey } = middlewareConfig - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}` + const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${phoneNumber}` return secureFetch(requestUrl, token, apiKey, 'GET') } From a328c77a33ff7286907ea2e97d843f33fd021254 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 8 Oct 2020 16:20:37 -0400 Subject: [PATCH 19/55] refactor(NotificationPrefsPane): Tweak phone number revert functionality after middleware change. --- lib/actions/user.js | 4 +- .../user/notification-prefs-pane.js | 15 ++--- lib/components/user/user-account-screen.js | 59 ++++++++++++++----- 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 4dc91aab7..2a077e804 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -191,7 +191,7 @@ export function deleteUserMonitoredTrip (id) { /** * Requests a verification code via SMS for the logged-in user. */ -export function requestPhoneVerificationSms (userData, newPhoneNumber) { +export function requestPhoneVerificationSms (id, newPhoneNumber) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence @@ -200,7 +200,7 @@ export function requestPhoneVerificationSms (userData, newPhoneNumber) { const { accessToken } = user // Send the SMS request with the phone number to update. - const sendSmsResult = await sendPhoneVerificationSms(otpMiddleware, accessToken, userData.id, newPhoneNumber) + const sendSmsResult = await sendPhoneVerificationSms(otpMiddleware, accessToken, id, newPhoneNumber) if (sendSmsResult.status === 'success') { // Refetch user and update application state with new phone number and verification status. // (This also refetches the user's monitored trip, and that's ok.) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 8c262b662..baed60360 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -45,9 +45,10 @@ const ButtonStrip = styled.div` * User notification preferences pane. */ class NotificationPrefsPane extends Component { - static propTypes = { - onRequestPhoneVerificationCode: PropTypes.func, - onSendPhoneVerificationCode: PropTypes.func, + static propTypes = { // TODO: update prop types. + onRequestPhoneVerificationCode: PropTypes.func.isRequired, + onRevertUserPhoneNumber: PropTypes.func.isRequired, + onSendPhoneVerificationCode: PropTypes.func.isRequired, onUserDataChange: PropTypes.func.isRequired, userData: PropTypes.object.isRequired } @@ -98,17 +99,13 @@ class NotificationPrefsPane extends Component { } _handleRevertPhoneNumber = () => { - // Revert entered phone number to the one from the user record. // Reset the modified and pending states. this.setState({ isPhoneFieldModified: false, isVerificationPending: false }) - const { onRevertUserPhoneNumber } = this.props - if (onRevertUserPhoneNumber) { - onRevertUserPhoneNumber(this.state.userData) - } + this.props.onRevertUserPhoneNumber() } _handleRequestPhoneVerificationCode = () => { @@ -256,7 +253,7 @@ class NotificationPrefsPane extends Component { {/* The dialog prompt for validation code. */} - + Enter verification code diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 1861cf6c5..3f3f441ad 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -44,6 +44,15 @@ class UserAccountScreen extends Component { } } + /** + * Clears the previous phone number/state so that there is nothing to revert to. + */ + _clearRevertNumber = () => { + this.setState({ + previousIsPhoneNumberVerified: null, + previousPhoneNumber: null + }) + } /** * Updates state.userData with new data (can be just one prop or the entire user record). */ @@ -100,17 +109,39 @@ class UserAccountScreen extends Component { /** * Reverts the user phone number and verification state in the database. */ - _handleRevertPhoneNumber = () => { + _handleRevertPhoneNumber = async () => { const { createOrUpdateUser, loggedInUser } = this.props + const { lastPhoneNumberRequested, previousIsPhoneNumberVerified, previousPhoneNumber, userData } = this.state + const { phoneNumber } = userData - // Make a clone of the original userData object. - const userData = clone(loggedInUser) - userData.phoneNumber = this.state.previousPhoneNumber - userData.isPhoneNumberVerified = this.state.previousIsPhoneNumberVerified + // If an SMS request has been sent and a previous number was recorded, + // then revert the phone number in the database too. + if (lastPhoneNumberRequested === phoneNumber && previousPhoneNumber) { + // Make a clone of the original userData object + // and just change the phone and phone verification status. + // (The other changes have not been submitted by the user yet.) + const clonedLoggedInUser = clone(loggedInUser) + clonedLoggedInUser.isPhoneNumberVerified = previousIsPhoneNumberVerified + clonedLoggedInUser.phoneNumber = previousPhoneNumber + + await createOrUpdateUser(clonedLoggedInUser, true) + } + + // Restore unsaved changes to the userData state. + this._updateUserState({ + ...clone(userData), + isPhoneNumberVerified: previousIsPhoneNumberVerified, + phoneNumber: previousPhoneNumber + }) + + // Update state so there is nothing to revert to after this operation. + this._clearRevertNumber() - // FIXME: Silently update the user record. - // (This assumes the pending phone number must be saved in the database prior to verification.) - createOrUpdateUser(userData, true) + // Clear lastPhoneNumberRequested so that if the user requests again, + // we can update the record without worrying about the throttle. + this.setState({ + lastPhoneNumberRequested: null + }) } /** @@ -120,7 +151,7 @@ class UserAccountScreen extends Component { */ _handleRequestPhoneVerificationCode = async () => { const { lastPhoneNumberRequested, lastPhoneRequestTime, userData } = this.state - const { phoneNumber } = userData + const { id, phoneNumber } = userData const timestamp = new Date() // Request a new verification code if we are requesting a different number. @@ -132,8 +163,7 @@ class UserAccountScreen extends Component { lastPhoneRequestTime: timestamp }) - // Use the original user data to avoid persisting any other pending edits. - await this.props.requestPhoneVerificationSms(this.props.loggedInUser, phoneNumber) + await this.props.requestPhoneVerificationSms(id, phoneNumber) } } @@ -147,7 +177,7 @@ class UserAccountScreen extends Component { // state.user.isPhoneNumberVerified will be set to true on success. // Clear previous phone number and state if phone is verified. if (this.props.loggedInUser.isPhoneNumberVerified) { - this.setState({ previousPhoneNumber: null, previousIsPhoneNumberVerified: null }) + this._clearRevertNumber() } } @@ -193,10 +223,7 @@ class UserAccountScreen extends Component { // Clear previous phone states if values are identical to the ones set above. if (phoneNumber === this.state.previousPhoneNumber) { - this.setState({ - previousIsPhoneNumberVerified: null, - previousPhoneNumber: null - }) + this._clearRevertNumber() } } } From 5d5c8a5637625544e86ac1a27a6dc96b0754a077 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 8 Oct 2020 16:26:20 -0400 Subject: [PATCH 20/55] refactor(NotificationPrefsPane): Hide Revert Number if no phone number was saved before. --- lib/components/user/notification-prefs-pane.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index baed60360..7c0dbd11f 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -45,7 +45,7 @@ const ButtonStrip = styled.div` * User notification preferences pane. */ class NotificationPrefsPane extends Component { - static propTypes = { // TODO: update prop types. + static propTypes = { onRequestPhoneVerificationCode: PropTypes.func.isRequired, onRevertUserPhoneNumber: PropTypes.func.isRequired, onSendPhoneVerificationCode: PropTypes.func.isRequired, @@ -245,7 +245,7 @@ class NotificationPrefsPane extends Component { {shouldVerifyPhone && ( - + {initialPhoneNumber.length !== 0 && } )} From 90009cca293035928087573cb438b96c77d4fbce Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 12 Oct 2020 22:41:13 -0400 Subject: [PATCH 21/55] refactor(UserAccountScreen): Adapt initial user persistence to Formik. Remove revert ph number. --- lib/components/user/new-account-wizard.js | 103 ++++++++------- .../user/notification-prefs-pane.js | 18 +-- .../user/sequential-pane-display.js | 6 +- lib/components/user/user-account-screen.js | 124 ++++-------------- 4 files changed, 88 insertions(+), 163 deletions(-) diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 723c9e1f3..e705034e7 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -1,60 +1,73 @@ -import React from 'react' +import React, { Component } from 'react' import SequentialPaneDisplay from './sequential-pane-display' /** * This component is the new account wizard. */ -const NewAccountWizard = props => { +class NewAccountWizard extends Component { + _handleCreateNewUser = () => { + const { + onCreate, // provided by UserAccountScreen + setFieldValue, // provided by Formik + values: userData // provided by Formik + } = this.props + + onCreate(userData, setFieldValue) + } + + render () { // The props include Formik props that provide access to the current user data (stored in props.values) // and to its own blur/change/submit event handlers that automate the state. // We forward the props to each pane so that their individual controls // can be wired to be managed by Formik. - const { onCreate, panes, values: userData } = props - - const { - hasConsentedToTerms, - notificationChannel = 'email' - } = userData - - const paneSequence = { - terms: { - disableNext: !hasConsentedToTerms, - nextId: 'notifications', - onNext: onCreate, - pane: panes.terms, - props, - title: 'Create a new account' - }, - notifications: { - disableNext: notificationChannel === 'sms' && !userData.isPhoneNumberVerified, - nextId: 'places', - pane: panes.notifications, - prevId: 'terms', - props, - title: 'Notification preferences' - }, - places: { - nextId: 'finish', - pane: panes.locations, - prevId: 'notifications', - props, - title: 'Add your locations' - }, - finish: { - pane: panes.finish, - prevId: 'places', - props, - title: 'Account setup complete!' + const props = this.props + const { panes, values: userData } = props + + const { + hasConsentedToTerms, + notificationChannel = 'email' + } = userData + + const paneSequence = { + terms: { + disableNext: !hasConsentedToTerms, + nextId: 'notifications', + onNext: this._handleCreateNewUser, + pane: panes.terms, + props, + title: 'Create a new account' + }, + notifications: { + disableNext: notificationChannel === 'sms' && !userData.isPhoneNumberVerified, + nextId: 'places', + pane: panes.notifications, + prevId: 'terms', + props, + title: 'Notification preferences' + }, + places: { + nextId: 'finish', + pane: panes.locations, + prevId: 'notifications', + props, + title: 'Add your locations' + }, + finish: { + pane: panes.finish, + prevId: 'places', + props, + title: 'Account setup complete!' + } } - } - return ( - - ) + return ( + + ) + } } export default NewAccountWizard diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 098401353..9c2417e58 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -47,7 +47,6 @@ const ButtonStrip = styled.div` class NotificationPrefsPane extends Component { static propTypes = { onRequestPhoneVerificationCode: PropTypes.func.isRequired, - onRevertUserPhoneNumber: PropTypes.func.isRequired, onSendPhoneVerificationCode: PropTypes.func.isRequired } @@ -55,8 +54,6 @@ class NotificationPrefsPane extends Component { super(props) this.state = { - // Holds the initial phone number or the last confirmed phone number - initialPhoneNumber: props.values.phoneNumber, // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. isVerificationPending: false, // Holds the validation code. @@ -79,15 +76,6 @@ class NotificationPrefsPane extends Component { }) } - _handleRevertPhoneNumber = () => { - // Reset the modified and pending states. - this.setState({ - isVerificationPending: false - }) - - this.props.onRevertUserPhoneNumber() - } - _handleRequestPhoneVerificationCode = () => { // Send phone verification request. this.props.onRequestPhoneVerificationCode() @@ -102,11 +90,9 @@ class NotificationPrefsPane extends Component { _handleSendPhoneVerificationCode = async () => { await this.props.onSendPhoneVerificationCode(this.state.phoneValidationCode) - // Exit verification modal and update "initialPhoneNumber" if isPhoneNumberVerified was changed to true. - // Erase the verification code in all cases. + // Exit verification modal (erase the verification code). if (this.props.userData.isPhoneNumberVerified) { this.setState({ - initialPhoneNumber: this.props.userData.phoneNumber, isVerificationPending: false, phoneValidationCode: '' }) @@ -128,7 +114,6 @@ class NotificationPrefsPane extends Component { } = this.props const { - initialPhoneNumber, isVerificationPending, phoneValidationCode } = this.state @@ -237,7 +222,6 @@ class NotificationPrefsPane extends Component { {shouldVerifyPhone && ( - {initialPhoneNumber.length !== 0 && } )} diff --git a/lib/components/user/sequential-pane-display.js b/lib/components/user/sequential-pane-display.js index 782cf1c49..0bcf3e564 100644 --- a/lib/components/user/sequential-pane-display.js +++ b/lib/components/user/sequential-pane-display.js @@ -34,6 +34,9 @@ class SequentialPaneDisplay extends Component { const nextId = currentPane.nextId if (nextId) { + // Don't submit the form if there are more steps to complete. + e.preventDefault() + // Execute pane-specific action, if any (e.g. save a user account) // when clicking next. if (typeof currentPane.onNext === 'function') { @@ -43,9 +46,6 @@ class SequentialPaneDisplay extends Component { this.setState({ activePaneId: nextId }) - - // Don't submit the form if there are more steps to complete. - e.preventDefault() } } diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index f8d0412fa..77c588463 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -1,6 +1,5 @@ import clone from 'clone' import { Form, Formik } from 'formik' -import isEqual from 'lodash.isequal' import React, { Component } from 'react' import { connect } from 'react-redux' import { withLoginRequired } from 'use-auth0-hooks' @@ -83,87 +82,46 @@ class UserAccountScreen extends Component { // Last number and last time we requested a code for (to avoid repeat SMS and not waste SMS quota). lastPhoneNumberRequested: null, - lastPhoneRequestTime: null, - - // Previous phone number and verification status, in case the - // user needs to revert their phone number after a code has been requested. - previousIsPhoneNumberVerified: null, - previousPhoneNumber: null, - - // Work on a copy of the logged-in user data captured when this component is created. - userData: clone(props.loggedInUser) + lastPhoneRequestTime: null } } - /** - * Clears the previous phone number/state so that there is nothing to revert to. - */ - _clearRevertNumber = () => { - this.setState({ - previousIsPhoneNumberVerified: null, - previousPhoneNumber: null - }) - } - - _updateUserPrefs = async userData => { + _updateUserPrefs = async (userData, silentOnSucceed = false) => { // TODO: Change state of Save button while the update action takes place. // In userData.savedLocations, filter out entries with blank addresses. const newUserData = clone(userData) newUserData.savedLocations = newUserData.savedLocations.filter(({ address }) => address && address.length) - await this.props.createOrUpdateUser(newUserData) + await this.props.createOrUpdateUser(newUserData, silentOnSucceed) // TODO: Handle UI feedback (currently an alert() dialog inside createOrUpdateUser). } - _handleCreateNewUser = async () => { - // Silently create the user record upon user accepting terms. - // Creating the user record before the user finishes the account creation steps - // is required by the middleware in order to perform phone verification. - await this._updateUserPrefs(true) + /** + * Silently persists the user data upon accepting terms, + * and updates the Formik state with the user's id from the database, + * so that when the user finishes the new account wizard, + * we update that user record instead of creating another one in the database. + * + * Creating the user record before the user finishes the account creation steps + * is required by the middleware in order to perform phone verification. + * + * @param {*} userData The user data state to persist. + * @param {*} setFieldValue Helper function provided by Formik to update Formik's state. + */ + _handleCreateNewUser = async (userData, setFieldValue) => { + // Silently persist the user. + await this._updateUserPrefs(userData, true) + + // After user is initially persisted and reloaded to the redux state, + // update the 'id' in the Formik state. + setFieldValue('id', this.props.loggedInUser.id) } _handleExit = () => { // On exit, route to default search route. this.props.routeTo('/') } - /** - * Reverts the user phone number and verification state in the database. - */ - _handleRevertPhoneNumber = async () => { - const { createOrUpdateUser, loggedInUser } = this.props - const { lastPhoneNumberRequested, previousIsPhoneNumberVerified, previousPhoneNumber, userData } = this.state - const { phoneNumber } = userData - - // If an SMS request has been sent and a previous number was recorded, - // then revert the phone number in the database too. - if (lastPhoneNumberRequested === phoneNumber && previousPhoneNumber) { - // Make a clone of the original userData object - // and just change the phone and phone verification status. - // (The other changes have not been submitted by the user yet.) - const clonedLoggedInUser = clone(loggedInUser) - clonedLoggedInUser.isPhoneNumberVerified = previousIsPhoneNumberVerified - clonedLoggedInUser.phoneNumber = previousPhoneNumber - - await createOrUpdateUser(clonedLoggedInUser, true) - } - - // Restore unsaved changes to the userData state. - this._updateUserState({ - ...clone(userData), - isPhoneNumberVerified: previousIsPhoneNumberVerified, - phoneNumber: previousPhoneNumber - }) - - // Update state so there is nothing to revert to after this operation. - this._clearRevertNumber() - - // Clear lastPhoneNumberRequested so that if the user requests again, - // we can update the record without worrying about the throttle. - this.setState({ - lastPhoneNumberRequested: null - }) - } /** * Requests a phone verification code. @@ -172,6 +130,7 @@ class UserAccountScreen extends Component { */ _handleRequestPhoneVerificationCode = async () => { const { lastPhoneNumberRequested, lastPhoneRequestTime, userData } = this.state + // FIXME: get user data from Formik. const { id, phoneNumber } = userData const timestamp = new Date() @@ -194,12 +153,6 @@ class UserAccountScreen extends Component { _handleSendPhoneVerificationCode = async code => { // Use the original user data to avoid persisting any other pending edits. await this.props.verifyPhoneNumber(this.props.loggedInUser, code) - - // state.user.isPhoneNumberVerified will be set to true on success. - // Clear previous phone number and state if phone is verified. - if (this.props.loggedInUser.isPhoneNumberVerified) { - this._clearRevertNumber() - } } /** @@ -219,33 +172,10 @@ class UserAccountScreen extends Component { finish: AccountSetupFinishPane } - componentDidUpdate (prevProps) { - // We need to update some fields, but not erase the user's other pending changes - // when the loggedInUser record has been updated while this screen is shown, e.g.: - // - when a new user clicks "Next" after agreeing on terms, - // - when the phone verification status changes (this is a middleware constraint). - const { loggedInUser } = this.props - if (!isEqual(prevProps.loggedInUser, loggedInUser)) { - const { id, isPhoneNumberVerified, notificationChannel, phoneNumber } = loggedInUser - this._updateUserState({ - id, - isPhoneNumberVerified, - notificationChannel, - phoneNumber - }) - - // Clear previous phone states if values are identical to the ones set above. - if (phoneNumber === this.state.previousPhoneNumber) { - this._clearRevertNumber() - } - } - } - // TODO: Update title bar during componentDidMount. render () { const { auth, loggedInUser } = this.props - const handleExit = this._handleExit return (
@@ -267,7 +197,7 @@ class UserAccountScreen extends Component { // can be wired to be managed by Formik. props => { let formContents - if (isNewUser(loggedInUser)) { + if (this.state.isNewUser) { if (!auth.user.email_verified) { // Check and prompt for email verification first to avoid extra user wait. formContents = @@ -277,11 +207,10 @@ class UserAccountScreen extends Component { formContents = ( @@ -292,9 +221,8 @@ class UserAccountScreen extends Component { formContents = ( From bdbd8576a16d055a585c3dad5973496ed668f3fe Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 13 Oct 2020 10:54:10 -0400 Subject: [PATCH 22/55] refactor(config.yml): Make phone number regex configurable. --- example-config.yml | 6 ++++ lib/components/user/user-account-screen.js | 39 ++++++++++------------ 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/example-config.yml b/example-config.yml index 719aaf23b..846c4c92c 100644 --- a/example-config.yml +++ b/example-config.yml @@ -140,3 +140,9 @@ itinerary: # modes: # - WALK # - BICYCLE + +### If using OTP Middleware, users can enable phone notifications. +### The phone number RegEx needs to be set depending on the target country. +# notifications: +# # For US numbers: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html +# phoneNumberRegex: /^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$/ diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 77c588463..4e453b9c7 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -18,25 +18,6 @@ import TermsOfUsePane from './terms-of-use-pane' import VerifyEmailScreen from './verify-email-screen' import withLoggedInUserSupport from './with-logged-in-user-support' -// Regex for phone numbers from https://stackoverflow.com/questions/52483260/validate-phone-number-with-yup/53210158#53210158 -// https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html -// FIXME: On merging with PR #224, remember to strip the non-numbers out and add +1 if there are only 10 digits. -const phoneRegExp = /^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$/ // /^((\\+[1-9]{1,4}[ \\-]*)|(\\([0-9]{2,3}\\)[ \\-]*)|([0-9]{2,4})[ \\-]*)*?[0-9]{3,4}?[ \\-]*[0-9]{3,4}?$/ - -// The validation schema for the form fields. -const validationSchema = yup.object({ - email: yup.string().email(), - hasConsentedToTerms: yup.boolean().oneOf([true], 'You must agree to the terms to continue.'), - notificationChannel: yup.string().oneOf(['email', 'sms', 'none']), - phoneNumber: yup.string().matches(phoneRegExp, 'Phone number is not valid'), - savedLocations: yup.array().of(yup.object({ - address: yup.string(), - icon: yup.string(), - type: yup.string() - })), - storeTripHistory: yup.boolean() -}) - /** * Makes a copy of the logged-in user data for the Formik initial state, * with the 'home' and 'work' locations at the top of the savedLocations list @@ -74,6 +55,20 @@ class UserAccountScreen extends Component { constructor (props) { super(props) + // The validation schema for the form fields (uses props to obtain the configured phone number regex). + this.validationSchema = yup.object({ + email: yup.string().email(), + hasConsentedToTerms: yup.boolean().oneOf([true], 'You must agree to the terms to continue.'), + notificationChannel: yup.string().oneOf(['email', 'sms', 'none']), + phoneNumber: yup.string().matches(props.phoneNumberRegEx, 'Phone number is not valid'), + savedLocations: yup.array().of(yup.object({ + address: yup.string(), + icon: yup.string(), + type: yup.string() + })), + storeTripHistory: yup.boolean() + }) + this.state = { // Capture whether user is a new user at this stage, and retain that value as long as this screen is active. // Reminder: When a new user progresses through the account steps, @@ -185,7 +180,7 @@ class UserAccountScreen extends Component { // Avoid validating on change as it is annoying. Validating on blur is enough. validateOnChange={false} validateOnBlur - validationSchema={validationSchema} + validationSchema={this.validationSchema} onSubmit={this._handleSaveAndExit} initialValues={cloneWithHomeAndWorkAsTopLocations(loggedInUser)} > @@ -245,8 +240,10 @@ class UserAccountScreen extends Component { // connect to the redux store const mapStateToProps = (state, ownProps) => { + const { notificatons: notificationsConfig } = state.otp.config return { - loggedInUser: state.user.loggedInUser + loggedInUser: state.user.loggedInUser, + phoneNumberRegEx: notificationsConfig && notificationsConfig.phoneNumberRegEx } } From 084f6c769f426da00443bf62c0ac4ae5ea9350f3 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 13 Oct 2020 17:06:07 -0400 Subject: [PATCH 23/55] refactor(NotificationsPrefsPane): Implement new phone verif. flow --- example-config.yml | 6 +- lib/actions/user.js | 4 +- .../user/notification-prefs-pane.js | 228 ++++++++++-------- lib/components/user/user-account-screen.js | 33 +-- 4 files changed, 155 insertions(+), 116 deletions(-) diff --git a/example-config.yml b/example-config.yml index 846c4c92c..09a87d288 100644 --- a/example-config.yml +++ b/example-config.yml @@ -142,7 +142,9 @@ itinerary: # - BICYCLE ### If using OTP Middleware, users can enable phone notifications. -### The phone number RegEx needs to be set depending on the target country. +### - phoneNumberRegExp should be set for the target locale. +### - phoneNumberPlaceholder is optional. # notifications: # # For US numbers: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html -# phoneNumberRegex: /^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$/ +# phoneNumberRegExp: ^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$ +# phoneNumberPlaceholder: (555) 555-0123 diff --git a/lib/actions/user.js b/lib/actions/user.js index 698335a5b..491639068 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -217,14 +217,14 @@ export function requestPhoneVerificationSms (id, newPhoneNumber) { /** * Validate the phone number verification code for the logged-in user. */ -export function verifyPhoneNumber (originalUserData, code) { +export function verifyPhoneNumber (id, code) { return async function (dispatch, getState) { const { otp, user } = getState() const { otp_middleware: otpMiddleware = null } = otp.config.persistence if (otpMiddleware) { const { accessToken } = user - const sendResult = await validatePhoneVerificationCode(otpMiddleware, accessToken, originalUserData.id, code) + const sendResult = await validatePhoneVerificationCode(otpMiddleware, accessToken, id, code) // If the check is successful, status in the returned data will be "approved". if (sendResult.status === 'success' && sendResult.data) { diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 9c2417e58..71e123888 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -6,13 +6,11 @@ import { ControlLabel, FormControl, FormGroup, - Glyphicon, HelpBlock, - Modal, ToggleButton, ToggleButtonGroup } from 'react-bootstrap' -import styled from 'styled-components' +import styled, { css } from 'styled-components' const allowedNotificationChannels = [ { @@ -35,11 +33,34 @@ const Details = styled.div` min-height: 150px; margin-bottom: 15px; ` -const ButtonStrip = styled.div` +const ControlStrip = styled.div` > * { margin-right: 4px; } ` +const noBorderOrShadow = css` + background: transparent; + border: none; + box-shadow: none; +` +const InlineTextInput = styled(FormControl)` + display: inline-block; + vertical-align: middle; + width: 12em; +` +const BorderlessInlineTextInput = styled(InlineTextInput)` + ${noBorderOrShadow} + &:focus { + ${noBorderOrShadow} + } + &[readonly] { + ${noBorderOrShadow} + } +` +const FlushLink = styled(Button)` + padding-left: 0; + padding-right: 0; +` /** * User notification preferences pane. @@ -54,110 +75,107 @@ class NotificationPrefsPane extends Component { super(props) this.state = { + // Whether the phone number is being edited. + isEditingPhoneNumber: false, + // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. - isVerificationPending: false, - // Holds the validation code. - phoneValidationCode: '' + isVerificationPending: false } } - _handlePhoneValidationCodeChange = e => { - // Update validation code state. + _handleEditNumber = () => { + const { setFieldValue } = this.props + + // Initialize Formik state's pending phone number to blank. + setFieldValue('pendingPhoneNumberFormatted', '') + this.setState({ - phoneValidationCode: e.target.value + isEditingPhoneNumber: true, + isVerificationPending: false }) } - _handlePhoneValidationCancel = () => { - // Exit the phone verification process. + _handleCancelEditNumber = () => { this.setState({ - isVerificationPending: false, - phoneValidationCode: '' + isEditingPhoneNumber: false }) } _handleRequestPhoneVerificationCode = () => { - // Send phone verification request. - this.props.onRequestPhoneVerificationCode() + // Send phone verification request with the entered values. + const { setFieldValue, values: userData } = this.props + const { pendingPhoneNumberFormatted } = userData + this.props.onRequestPhoneVerificationCode(pendingPhoneNumberFormatted) + + // Set empty code field in Formik's state + setFieldValue('phoneValidationCode', '') // Prompt for code. this.setState({ - isVerificationPending: true, - phoneValidationCode: '' + isEditingPhoneNumber: false, + isVerificationPending: true }) } _handleSendPhoneVerificationCode = async () => { - await this.props.onSendPhoneVerificationCode(this.state.phoneValidationCode) + const { values: userData } = this.props - // Exit verification modal (erase the verification code). - if (this.props.userData.isPhoneNumberVerified) { - this.setState({ - isVerificationPending: false, - phoneValidationCode: '' - }) - } else { + await this.props.onSendPhoneVerificationCode(userData.phoneValidationCode) + + // When the pending phone number is erased by the backend, + // this means the phone number has been verified. + if (!this.props.values.pendingPhoneNumber) { this.setState({ - phoneValidationCode: '' + isVerificationPending: false }) } } render () { // All props below are Formik props (https://formik.org/docs/api/formik#props-1) + // except where indicated. const { errors, handleBlur, handleChange, + phoneNumberPlaceholder, // provided by UserAccountScreen touched, - values: userData + values: userDataWithValidationFields } = this.props const { - isVerificationPending, - phoneValidationCode + isEditingPhoneNumber, + isVerificationPending } = this.state const { email, - isPhoneNumberVerified, notificationChannel, - phoneNumber - } = userData + phoneNumber, + phoneNumberFormatted, + pendingPhoneNumberFormatted, + phoneValidationCode + } = userDataWithValidationFields // Here are the states we are dealing with: // - First time entering a phone number (phoneNumber blank, not modified) // => no color, no feedback indication. // - Typing backspace all the way to erase a number (phoneNumber blank, modified) - // => red, "[X] Please provide a number" indication. - // - Viewing a verified phone number (phoneNumber non-blank, same as initial, verified) - // => green, "[V] Verified" indication. - // - Editing a phone number (phoneNumber non-blank, different than initial or not verified) - // => yellow, "[!] Verification required" indication. + // => red error. + // - Typing a phone number that doesn't match the configured phoneNumberRegEx + // => red error. const isPhoneNumberBlank = !(phoneNumber && phoneNumber.length) - let phoneStatusGlyph // one of the Bootstrap glyphs. - let phoneStatusText let phoneFieldValidationState // one of the Bootstrap validationState values. - let shouldVerifyPhone = false if (isPhoneNumberBlank) { - if (!touched.phoneNumber) { + if (!touched.pendingPhoneNumberFormatted) { // Do not show an indication in initial state. } else { - phoneStatusGlyph = 'remove' - phoneStatusText = 'Please provide a number' phoneFieldValidationState = 'error' } - } else if (!touched.phoneNumber && isPhoneNumberVerified) { - phoneStatusGlyph = 'ok' - phoneStatusText = 'Verified' - phoneFieldValidationState = 'success' - } else { - phoneStatusGlyph = 'warning-sign' - phoneStatusText = 'Verification required' - phoneFieldValidationState = 'warning' - shouldVerifyPhone = true + } else if (touched.pendingPhoneNumberFormatted && errors.pendingPhoneNumberFormatted) { + phoneFieldValidationState = 'error' } return ( @@ -202,55 +220,71 @@ class NotificationPrefsPane extends Component { {/* FIXME: Merge the validation feedback upon approving PR #224. */} Enter your phone number for SMS notifications: - - - {errors.phoneNumber && {errors.phoneNumber}} - {/* Show glyphs underneath the input control with - (there are some alignment issues with in mobile view). */} - - {phoneStatusGlyph && } {phoneStatusText} - + + {/* Borderless input control for original phone number */} + {!isEditingPhoneNumber && ( + <> + + {isVerificationPending && '(pending) '} + + + )} - {shouldVerifyPhone && ( - - - - )} + {/* Regular-styled input control for editing the pending phone number */} + {isEditingPhoneNumber && ( + <> + + + + {errors.pendingPhoneNumberFormatted && {errors.pendingPhoneNumberFormatted}} + + )} + - {/* The dialog prompt for validation code. */} - - - Enter verification code - - - + {isVerificationPending && ( +

- Please check the SMS messaging app on your mobile phone - for a text message with a verification code, and enter the code below. + Please check the SMS messaging app on your mobile phone + for a text message with a verification code, and enter the code below.

- - Verification code: - - -

- -

-
- - - - - -
+ Verification code: + + + + + Request a new code + + )}
)} diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 4e453b9c7..e4bfb50d8 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -60,7 +60,8 @@ class UserAccountScreen extends Component { email: yup.string().email(), hasConsentedToTerms: yup.boolean().oneOf([true], 'You must agree to the terms to continue.'), notificationChannel: yup.string().oneOf(['email', 'sms', 'none']), - phoneNumber: yup.string().matches(props.phoneNumberRegEx, 'Phone number is not valid'), + pendingPhoneNumberFormatted: yup.string().matches(new RegExp(props.phoneNumberRegExp), 'Phone number is not valid'), + phoneValidationCode: yup.string().required(), savedLocations: yup.array().of(yup.object({ address: yup.string(), icon: yup.string(), @@ -123,22 +124,21 @@ class UserAccountScreen extends Component { * This handler is called when the user clicks "Verify my phone" after entering a new number, * and also when the user clicks "Request a new code" from the verification modal. */ - _handleRequestPhoneVerificationCode = async () => { - const { lastPhoneNumberRequested, lastPhoneRequestTime, userData } = this.state - // FIXME: get user data from Formik. - const { id, phoneNumber } = userData + _handleRequestPhoneVerificationCode = async pendingPhoneNumberFormatted => { + const { id } = this.props.loggedInUser + const { lastPhoneNumberRequested, lastPhoneRequestTime } = this.state const timestamp = new Date() // Request a new verification code if we are requesting a different number. // or enough time has ellapsed since the last request (1 minute?). - if (lastPhoneNumberRequested !== phoneNumber || + if (lastPhoneNumberRequested !== pendingPhoneNumberFormatted || (lastPhoneRequestTime && lastPhoneRequestTime <= timestamp + 60000)) { this.setState({ - lastPhoneNumberRequested: phoneNumber, + lastPhoneNumberRequested: pendingPhoneNumberFormatted, lastPhoneRequestTime: timestamp }) - await this.props.requestPhoneVerificationSms(id, phoneNumber) + await this.props.requestPhoneVerificationSms(id, pendingPhoneNumberFormatted) } } @@ -146,8 +146,7 @@ class UserAccountScreen extends Component { * Sends the phone verification code. */ _handleSendPhoneVerificationCode = async code => { - // Use the original user data to avoid persisting any other pending edits. - await this.props.verifyPhoneNumber(this.props.loggedInUser, code) + await this.props.verifyPhoneNumber(this.props.loggedInUser.id, code) } /** @@ -170,15 +169,15 @@ class UserAccountScreen extends Component { // TODO: Update title bar during componentDidMount. render () { - const { auth, loggedInUser } = this.props + const { auth, loggedInUser, phoneNumberPlaceholder } = this.props return (
{/* TODO: Do mobile view. */} ) } @@ -220,6 +220,7 @@ class UserAccountScreen extends Component { onRequestPhoneVerificationCode={this._handleRequestPhoneVerificationCode} onSendPhoneVerificationCode={this._handleSendPhoneVerificationCode} panes={this._panes} + phoneNumberPlaceholder={phoneNumberPlaceholder} /> ) } @@ -240,10 +241,12 @@ class UserAccountScreen extends Component { // connect to the redux store const mapStateToProps = (state, ownProps) => { - const { notificatons: notificationsConfig } = state.otp.config + const { notifications: notificationsConfig = {} } = state.otp.config + const { phoneNumberPlaceholder, phoneNumberRegExp } = notificationsConfig return { loggedInUser: state.user.loggedInUser, - phoneNumberRegEx: notificationsConfig && notificationsConfig.phoneNumberRegEx + phoneNumberPlaceholder, + phoneNumberRegExp } } From eadf6358dc707ec4db24118a1161499ecbbfc932 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 13 Oct 2020 19:40:26 -0400 Subject: [PATCH 24/55] refactor(NotificationPrefsPane): Various refactors --- lib/components/user/new-account-wizard.js | 2 +- .../user/notification-prefs-pane.js | 94 +++++++++---------- lib/components/user/user-account-screen.js | 13 ++- lib/util/middleware.js | 2 +- 4 files changed, 60 insertions(+), 51 deletions(-) diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index e705034e7..3e4df818c 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -39,7 +39,7 @@ class NewAccountWizard extends Component { title: 'Create a new account' }, notifications: { - disableNext: notificationChannel === 'sms' && !userData.isPhoneNumberVerified, + disableNext: notificationChannel === 'sms' && !userData.phoneNumber, nextId: 'places', pane: panes.notifications, prevId: 'terms', diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 71e123888..3298fd5a2 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -1,3 +1,4 @@ +import { Field } from 'formik' import PropTypes from 'prop-types' import React, { Component } from 'react' import { @@ -62,6 +63,23 @@ const FlushLink = styled(Button)` padding-right: 0; ` +/** + * @param {*} props The props from which to extract the Formik state to test. + * @param {*} field THe field name to test. + * @returns One of the Bootstrao validationState values. + */ +function getValidationState (props, field) { + const { errors, touched, values } = props + const value = values[field] + const isBlank = !(value && value.length) + const isInvalid = isBlank || !!errors[field] + + // one of the Bootstrap validationState values. + return isInvalid && touched[field] + ? 'error' + : null +} + /** * User notification preferences pane. */ @@ -76,18 +94,21 @@ class NotificationPrefsPane extends Component { this.state = { // Whether the phone number is being edited. - isEditingPhoneNumber: false, + // Intially false if an existing user has a phone number already. + // For (new) users who don't have a number entered, we want to set that to true. + isEditingPhoneNumber: !props.values.phoneNumber, // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. isVerificationPending: false } + + // Initialize Formik state's pending phone number to blank. + props.setFieldValue('pendingPhoneNumberFormatted', '') } _handleEditNumber = () => { - const { setFieldValue } = this.props - // Initialize Formik state's pending phone number to blank. - setFieldValue('pendingPhoneNumberFormatted', '') + this.props.setFieldValue('pendingPhoneNumberFormatted', '') this.setState({ isEditingPhoneNumber: true, @@ -103,12 +124,13 @@ class NotificationPrefsPane extends Component { _handleRequestPhoneVerificationCode = () => { // Send phone verification request with the entered values. - const { setFieldValue, values: userData } = this.props + const { setFieldValue, setFieldTouched, values: userData } = this.props const { pendingPhoneNumberFormatted } = userData this.props.onRequestPhoneVerificationCode(pendingPhoneNumberFormatted) // Set empty code field in Formik's state setFieldValue('phoneValidationCode', '') + setFieldTouched('phoneValidationCode', false) // Prompt for code. this.setState({ @@ -140,7 +162,7 @@ class NotificationPrefsPane extends Component { handleChange, phoneNumberPlaceholder, // provided by UserAccountScreen touched, - values: userDataWithValidationFields + values: userDataWithValidationCode } = this.props const { @@ -148,40 +170,26 @@ class NotificationPrefsPane extends Component { isVerificationPending } = this.state const { - email, notificationChannel, - phoneNumber, phoneNumberFormatted, - pendingPhoneNumberFormatted, - phoneValidationCode - } = userDataWithValidationFields + pendingPhoneNumberFormatted + } = userDataWithValidationCode // Here are the states we are dealing with: - // - First time entering a phone number (phoneNumber blank, not modified) + // - First time entering a phone number/validation code (blank value, not modified) // => no color, no feedback indication. - // - Typing backspace all the way to erase a number (phoneNumber blank, modified) + // - Typing backspace all the way to erase a number/code (blank value, modified) // => red error. // - Typing a phone number that doesn't match the configured phoneNumberRegEx // => red error. - const isPhoneNumberBlank = !(phoneNumber && phoneNumber.length) - - let phoneFieldValidationState // one of the Bootstrap validationState values. - - if (isPhoneNumberBlank) { - if (!touched.pendingPhoneNumberFormatted) { - // Do not show an indication in initial state. - } else { - phoneFieldValidationState = 'error' - } - } else if (touched.pendingPhoneNumberFormatted && errors.pendingPhoneNumberFormatted) { - phoneFieldValidationState = 'error' - } + const phoneFieldValidationState = getValidationState(this.props, 'pendingPhoneNumberFormatted') + const codeFieldValidationState = getValidationState(this.props, 'phoneValidationCode') return (

- You can receive notifications about trips you frequently take. + You can receive notifications about trips you frequently take.

How would you like to receive notifications? @@ -211,76 +219,68 @@ class NotificationPrefsPane extends Component { {notificationChannel === 'email' && ( Notification emails will be sent out to: - + )} {notificationChannel === 'sms' && (
- {/* TODO: Add field validation. */} - {/* FIXME: Merge the validation feedback upon approving PR #224. */} Enter your phone number for SMS notifications: - {/* Borderless input control for original phone number */} {!isEditingPhoneNumber && ( <> - {isVerificationPending && '(pending) '} )} - {/* Regular-styled input control for editing the pending phone number */} {isEditingPhoneNumber && ( <> - - {errors.pendingPhoneNumberFormatted && {errors.pendingPhoneNumberFormatted}} + {touched.pendingPhoneNumberFormatted && {errors.pendingPhoneNumberFormatted}} )} {isVerificationPending && ( - +

Please check the SMS messaging app on your mobile phone for a text message with a verification code, and enter the code below.

Verification code: - + {touched.phoneValidationCode && {errors.phoneValidationCode}} Request a new code
diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index e4bfb50d8..4baa22987 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -45,9 +45,16 @@ function cloneWithHomeAndWorkAsTopLocations (loggedInUser) { ] clonedUser.savedLocations = reorderedLocations + + // Blank strings for validation code and phone numbers + clonedUser.pendingPhoneNumberFormatted = '' + clonedUser.phoneValidationCode = '' + return clonedUser } +const INVALID_PHONE_MSG = 'Please enter a valid phone number.' + /** * This screen handles creating/updating OTP user account settings. */ @@ -60,8 +67,10 @@ class UserAccountScreen extends Component { email: yup.string().email(), hasConsentedToTerms: yup.boolean().oneOf([true], 'You must agree to the terms to continue.'), notificationChannel: yup.string().oneOf(['email', 'sms', 'none']), - pendingPhoneNumberFormatted: yup.string().matches(new RegExp(props.phoneNumberRegExp), 'Phone number is not valid'), - phoneValidationCode: yup.string().required(), + pendingPhoneNumberFormatted: yup.string() + .required(INVALID_PHONE_MSG) + .matches(new RegExp(props.phoneNumberRegExp), INVALID_PHONE_MSG), + phoneValidationCode: yup.string().required('Please enter a validation code.'), savedLocations: yup.array().of(yup.object({ address: yup.string(), icon: yup.string(), diff --git a/lib/util/middleware.js b/lib/util/middleware.js index 8ed02bd53..2620592c6 100644 --- a/lib/util/middleware.js +++ b/lib/util/middleware.js @@ -143,7 +143,7 @@ export async function deleteTrip (middlewareConfig, token, id) { export async function sendPhoneVerificationSms (middlewareConfig, token, userId, phoneNumber) { const { apiBaseUrl, apiKey } = middlewareConfig - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${phoneNumber}` + const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${encodeURIComponent(phoneNumber)}` return secureFetch(requestUrl, token, apiKey, 'GET') } From 7f5148d1b3498626cb62c133e5457ebad5584d22 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 14 Oct 2020 11:59:34 -0400 Subject: [PATCH 25/55] refactor(NotificationPrefsPane): Update phone numbers following API calls. --- lib/components/user/new-account-wizard.js | 16 +- .../user/notification-prefs-pane.js | 141 +++++++++++------- lib/components/user/user-account-screen.js | 83 +++++++---- 3 files changed, 153 insertions(+), 87 deletions(-) diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 3e4df818c..976db8b54 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -6,21 +6,25 @@ import SequentialPaneDisplay from './sequential-pane-display' * This component is the new account wizard. */ class NewAccountWizard extends Component { - _handleCreateNewUser = () => { + _handleCreateNewUser = async () => { const { onCreate, // provided by UserAccountScreen setFieldValue, // provided by Formik values: userData // provided by Formik } = this.props - onCreate(userData, setFieldValue) + const newId = await onCreate(userData) + + // After user is initially persisted and reloaded to the redux state, + // update the 'id' in the Formik state. + setFieldValue('id', newId) } render () { - // The props include Formik props that provide access to the current user data (stored in props.values) - // and to its own blur/change/submit event handlers that automate the state. - // We forward the props to each pane so that their individual controls - // can be wired to be managed by Formik. + // The props include Formik props that provide access to the current user data (stored in props.values) + // and to its own blur/change/submit event handlers that automate the state. + // We forward the props to each pane so that their individual controls + // can be wired to be managed by Formik. const props = this.props const { panes, values: userData } = props diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 3298fd5a2..8461d99ca 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -39,24 +39,16 @@ const ControlStrip = styled.div` margin-right: 4px; } ` -const noBorderOrShadow = css` - background: transparent; - border: none; - box-shadow: none; -` -const InlineTextInput = styled(FormControl)` +const phoneFieldCss = css` display: inline-block; vertical-align: middle; - width: 12em; + width: 14em; ` -const BorderlessInlineTextInput = styled(InlineTextInput)` - ${noBorderOrShadow} - &:focus { - ${noBorderOrShadow} - } - &[readonly] { - ${noBorderOrShadow} - } +const InlineTextInput = styled(FormControl)` + ${phoneFieldCss} +` +const InlineStatic = styled(FormControl.Static)` + ${phoneFieldCss} ` const FlushLink = styled(Button)` padding-left: 0; @@ -101,9 +93,6 @@ class NotificationPrefsPane extends Component { // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. isVerificationPending: false } - - // Initialize Formik state's pending phone number to blank. - props.setFieldValue('pendingPhoneNumberFormatted', '') } _handleEditNumber = () => { @@ -122,31 +111,69 @@ class NotificationPrefsPane extends Component { }) } - _handleRequestPhoneVerificationCode = () => { + _handlePhoneNumberKeyDown = e => { + if (e.keyCode === 13) { + // On the user pressing enter (keyCode 13) on the phone number field, + // prevent form submission and request the code. + e.preventDefault() + this._handleRequestPhoneVerificationCode() + } + } + + _handleValidationCodeKeyDown = e => { + if (e.keyCode === 13) { + // On the user pressing enter (keyCode 13) on the phone number field, + // prevent form submission and request the code. + e.preventDefault() + this._handleSendPhoneVerificationCode() + } + } + + _handleRequestPhoneVerificationCode = async () => { // Send phone verification request with the entered values. const { setFieldValue, setFieldTouched, values: userData } = this.props const { pendingPhoneNumberFormatted } = userData - this.props.onRequestPhoneVerificationCode(pendingPhoneNumberFormatted) - // Set empty code field in Formik's state - setFieldValue('phoneValidationCode', '') - setFieldTouched('phoneValidationCode', false) + // Exit editing first (hides the Send Text button). + this._handleCancelEditNumber() - // Prompt for code. - this.setState({ - isEditingPhoneNumber: false, - isVerificationPending: true - }) + const result = await this.props.onRequestPhoneVerificationCode(pendingPhoneNumberFormatted, setFieldValue) + + if (result) { + // On success, update the phone number in Formik's state. + setFieldValue('pendingPhoneNumber', result.pendingPhoneNumber) + setFieldValue('pendingPhoneNumberFormatted', result.pendingPhoneNumberFormatted) + + // Set empty code field in Formik's state and prompt for code. + setFieldValue('phoneValidationCode', '') + setFieldTouched('phoneValidationCode', false) + this.setState({ + isVerificationPending: true + }) + } else { + // Else discard the pending phone numbers. + setFieldValue('pendingPhoneNumber', '') + setFieldValue('pendingPhoneNumberFormatted', '') + } } _handleSendPhoneVerificationCode = async () => { - const { values: userData } = this.props + const { setFieldValue, values: userData } = this.props + const { phoneValidationCode } = userData + + // Clear the code field and disable the verify button. + // (The code to send is already captured in const above.) + setFieldValue('phoneValidationCode', '') - await this.props.onSendPhoneVerificationCode(userData.phoneValidationCode) + const result = await this.props.onSendPhoneVerificationCode(phoneValidationCode) + + if (result) { + // On success, update the phone number in Formik's state and hide the code field. + setFieldValue('phoneNumber', result.phoneNumber) + setFieldValue('phoneNumberFormatted', result.phoneNumberFormatted) + setFieldValue('pendingPhoneNumber', '') + setFieldValue('pendingPhoneNumberFormatted', '') - // When the pending phone number is erased by the backend, - // this means the phone number has been verified. - if (!this.props.values.pendingPhoneNumber) { this.setState({ isVerificationPending: false }) @@ -218,33 +245,23 @@ class NotificationPrefsPane extends Component {
{notificationChannel === 'email' && ( - Notification emails will be sent out to: + Notification emails will be sent to: )} {notificationChannel === 'sms' && (
- - Enter your phone number for SMS notifications: - - {!isEditingPhoneNumber && ( - <> - - - - )} - - {isEditingPhoneNumber && ( - <> + {isEditingPhoneNumber + ? ( + + Enter your phone number for SMS notifications: + {touched.pendingPhoneNumberFormatted && {errors.pendingPhoneNumberFormatted}} - - )} - - + + + ) : ( + + SMS notifications will be sent to: + + + {isVerificationPending ? `${pendingPhoneNumberFormatted} (pending)` : phoneNumberFormatted} + + + + + )} {isVerificationPending && ( @@ -270,6 +296,7 @@ class NotificationPrefsPane extends Component { @@ -280,7 +307,9 @@ class NotificationPrefsPane extends Component { > Verify - {touched.phoneValidationCode && {errors.phoneValidationCode}} + {touched.phoneValidationCode && errors.phoneValidationCode && ( + {errors.phoneValidationCode} + )} Request a new code diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 4baa22987..90cc4ea41 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -19,13 +19,14 @@ import VerifyEmailScreen from './verify-email-screen' import withLoggedInUserSupport from './with-logged-in-user-support' /** - * Makes a copy of the logged-in user data for the Formik initial state, - * with the 'home' and 'work' locations at the top of the savedLocations list - * so they are always shown and shown at the top of the FavoriteLocationsPane. - * Note: In the returned value, savedLocations is always a valid array. + * Makes a copy of the logged-in user data for the Formik initial state, with: + * - the 'home' and 'work' locations at the top of the savedLocations list + * so they are always shown and shown at the top of the FavoriteLocationsPane. + * Note: In the returned value, savedLocations is always a valid array. + * - initial values for phone number/code fields used by Formik. */ -function cloneWithHomeAndWorkAsTopLocations (loggedInUser) { - const clonedUser = clone(loggedInUser) +function cloneForFormik (userData) { + const clonedUser = clone(userData) const { savedLocations = [] } = clonedUser const homeLocation = savedLocations.find(isHome) || { @@ -47,12 +48,27 @@ function cloneWithHomeAndWorkAsTopLocations (loggedInUser) { clonedUser.savedLocations = reorderedLocations // Blank strings for validation code and phone numbers + // (to avoid React warnings). clonedUser.pendingPhoneNumberFormatted = '' clonedUser.phoneValidationCode = '' return clonedUser } +/** + * Returns a user data clone without extra fields we've set to use with Formik. + */ +function cloneForPersistence (userData) { + const persistedData = clone(userData) + delete persistedData.phoneValidationCode + + // Don't save empty strings used to avoid React warnings. + if (persistedData.pendingPhoneNumberFormatted === '') delete persistedData.pendingPhoneNumberFormatted + if (persistedData.pendingPhoneNumber === '') delete persistedData.pendingPhoneNumber + + return persistedData +} + const INVALID_PHONE_MSG = 'Please enter a valid phone number.' /** @@ -95,7 +111,7 @@ class UserAccountScreen extends Component { // TODO: Change state of Save button while the update action takes place. // In userData.savedLocations, filter out entries with blank addresses. - const newUserData = clone(userData) + const newUserData = cloneForPersistence(userData) newUserData.savedLocations = newUserData.savedLocations.filter(({ address }) => address && address.length) await this.props.createOrUpdateUser(newUserData, silentOnSucceed) @@ -103,24 +119,17 @@ class UserAccountScreen extends Component { } /** - * Silently persists the user data upon accepting terms, - * and updates the Formik state with the user's id from the database, - * so that when the user finishes the new account wizard, - * we update that user record instead of creating another one in the database. - * + * Silently persists the user data upon accepting terms. * Creating the user record before the user finishes the account creation steps * is required by the middleware in order to perform phone verification. * * @param {*} userData The user data state to persist. - * @param {*} setFieldValue Helper function provided by Formik to update Formik's state. + * @returns The new user id the the caller can use. */ - _handleCreateNewUser = async (userData, setFieldValue) => { - // Silently persist the user. - await this._updateUserPrefs(userData, true) + _handleCreateNewUser = async userData => { + await this._updateUserPrefs(cloneForPersistence(userData), true) - // After user is initially persisted and reloaded to the redux state, - // update the 'id' in the Formik state. - setFieldValue('id', this.props.loggedInUser.id) + return this.props.loggedInUser.id } _handleExit = () => { @@ -129,33 +138,57 @@ class UserAccountScreen extends Component { } /** - * Requests a phone verification code. + * Requests a phone verification code to the specified phone number. * This handler is called when the user clicks "Verify my phone" after entering a new number, * and also when the user clicks "Request a new code" from the verification modal. + * + * @returns The pending phone number (plain and formatted) if the request succeeded, null otherwise. */ - _handleRequestPhoneVerificationCode = async pendingPhoneNumberFormatted => { - const { id } = this.props.loggedInUser + _handleRequestPhoneVerificationCode = async numberToVerify => { const { lastPhoneNumberRequested, lastPhoneRequestTime } = this.state const timestamp = new Date() // Request a new verification code if we are requesting a different number. // or enough time has ellapsed since the last request (1 minute?). - if (lastPhoneNumberRequested !== pendingPhoneNumberFormatted || + // TODO: Should throttling be handled in the middleware? + if (lastPhoneNumberRequested !== numberToVerify || (lastPhoneRequestTime && lastPhoneRequestTime <= timestamp + 60000)) { + await this.props.requestPhoneVerificationSms(this.props.loggedInUser.id, numberToVerify) + + // (props have been updated are set at this point.) + const { pendingPhoneNumber, pendingPhoneNumberFormatted } = this.props.loggedInUser this.setState({ + // (The reformatted number may be different from the value initially passed, + // so the throttling condition is not a rigourous check.) lastPhoneNumberRequested: pendingPhoneNumberFormatted, lastPhoneRequestTime: timestamp }) - await this.props.requestPhoneVerificationSms(id, pendingPhoneNumberFormatted) + // When the pending phone number is populated by the backend, + // this means the phone number verification SMS has been sent. + if (pendingPhoneNumber) { + return { pendingPhoneNumber, pendingPhoneNumberFormatted } + } } + + return null } /** * Sends the phone verification code. + * @returns The new phone number (plain and formatted) if the verification succeeded, null otherwise. */ _handleSendPhoneVerificationCode = async code => { await this.props.verifyPhoneNumber(this.props.loggedInUser.id, code) + + // When the pending phone number is erased by the backend, + // this means the phone number has been verified. + const { pendingPhoneNumber, phoneNumber, phoneNumberFormatted } = this.props.loggedInUser + if (!pendingPhoneNumber) { + return { phoneNumber, phoneNumberFormatted } + } + + return null } /** @@ -190,7 +223,7 @@ class UserAccountScreen extends Component { validateOnBlur validationSchema={this.validationSchema} onSubmit={this._handleSaveAndExit} - initialValues={cloneWithHomeAndWorkAsTopLocations(loggedInUser)} + initialValues={cloneForFormik(loggedInUser)} > { // Formik props provide access to the current user data state and errors, From f79a3f769e2679713a7b26f4c017de296a316f9b Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 14 Oct 2020 23:51:28 -0400 Subject: [PATCH 26/55] refactor(PhoneNumberEditor): Extract component from NotificationPrefsPane --- lib/components/user/new-account-wizard.js | 1 - .../user/notification-prefs-pane.js | 393 ++++++++++-------- lib/components/user/user-account-screen.js | 42 +- lib/util/ui.js | 8 + 4 files changed, 246 insertions(+), 198 deletions(-) diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 976db8b54..50604b8fa 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -27,7 +27,6 @@ class NewAccountWizard extends Component { // can be wired to be managed by Formik. const props = this.props const { panes, values: userData } = props - const { hasConsentedToTerms, notificationChannel = 'email' diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 8461d99ca..3984c8822 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -1,5 +1,4 @@ -import { Field } from 'formik' -import PropTypes from 'prop-types' +import { Field, Formik } from 'formik' import React, { Component } from 'react' import { Button, @@ -12,6 +11,9 @@ import { ToggleButtonGroup } from 'react-bootstrap' import styled, { css } from 'styled-components' +import * as yup from 'yup' + +import { isBlank } from '../../util/ui' const allowedNotificationChannels = [ { @@ -55,16 +57,17 @@ const FlushLink = styled(Button)` padding-right: 0; ` +const INVALID_PHONE_MSG = 'Please enter a valid phone number.' + /** * @param {*} props The props from which to extract the Formik state to test. * @param {*} field THe field name to test. * @returns One of the Bootstrao validationState values. */ -function getValidationState (props, field) { +function getErrorState (props, field) { const { errors, touched, values } = props const value = values[field] - const isBlank = !(value && value.length) - const isInvalid = isBlank || !!errors[field] + const isInvalid = isBlank(value) || !!errors[field] // one of the Bootstrap validationState values. return isInvalid && touched[field] @@ -76,38 +79,150 @@ function getValidationState (props, field) { * User notification preferences pane. */ class NotificationPrefsPane extends Component { - static propTypes = { - onRequestPhoneVerificationCode: PropTypes.func.isRequired, - onSendPhoneVerificationCode: PropTypes.func.isRequired + constructor (props) { + super(props) + + this.phoneValidationSchema = yup.object({ + newPhoneNumber: yup.string() + .required(INVALID_PHONE_MSG) + .matches(props.phoneNumberRegExp, INVALID_PHONE_MSG), + validationCode: yup.string().required('Please enter a validation code.') + }) + } + + _handleOnPhoneVerified = ({ phoneNumber, phoneNumberFormatted }) => { + const { setFieldValue } = this.props + + // Update the parent's Formik state (so when saving, it sends the updated numbers) + setFieldValue('phoneNumber', phoneNumber) + setFieldValue('phoneNumberFormatted', phoneNumberFormatted) } + render () { + // All props below are Formik props (https://formik.org/docs/api/formik#props-1) + // except * below which indicates the prop is provided by UserAccountScreen. + const { + handleBlur, + handleChange, + onRequestPhoneVerificationCode, // * + onSendPhoneVerificationCode, // * + phoneNumberPlaceholder, // * + phoneNumberRegExp, // * + values: userData + } = this.props + + const { + notificationChannel, + phoneNumber + } = userData + + const initialFormikValues = { + newPhoneNumber: '', + validationCode: '' + } + + return ( +
+

+ You can receive notifications about trips you frequently take. +

+ + How would you like to receive notifications? + + + {allowedNotificationChannels.map(({ type, text }, index) => ( + + {text} + + ))} + + + +
+ {notificationChannel === 'email' && ( + + Notification emails will be sent to: + + + )} + {notificationChannel === 'sms' && ( + + { + // Pass Formik props to the component rendered so Formik can manage its validation. + // (The validation for this component is independent of the validation set in UserAccountScreen.) + innerProps => { + return ( + + ) + } + } + + )} +
+
+ ) + } +} + +export default NotificationPrefsPane + +/** + * Sub-component that handles phone number and validation code editing and validation intricacies. + */ +class PhoneNumberEditor extends Component { constructor (props) { super(props) + const { initialPhoneNumber } = props this.state = { - // Whether the phone number is being edited. - // Intially false if an existing user has a phone number already. - // For (new) users who don't have a number entered, we want to set that to true. - isEditingPhoneNumber: !props.values.phoneNumber, + // If true, phone number is being edited. + // For new users, render component in editing state. + isEditing: isBlank(initialPhoneNumber), // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. - isVerificationPending: false + isPending: false, + + // Holds the last known verified phone number. + verifiedNumber: initialPhoneNumber } } _handleEditNumber = () => { - // Initialize Formik state's pending phone number to blank. - this.props.setFieldValue('pendingPhoneNumberFormatted', '') - this.setState({ - isEditingPhoneNumber: true, - isVerificationPending: false + isEditing: true, + isPending: false }) } _handleCancelEditNumber = () => { this.setState({ - isEditingPhoneNumber: false + isEditing: false, + isPending: false }) } @@ -116,67 +231,62 @@ class NotificationPrefsPane extends Component { // On the user pressing enter (keyCode 13) on the phone number field, // prevent form submission and request the code. e.preventDefault() - this._handleRequestPhoneVerificationCode() + this._handleRequestCode() } } _handleValidationCodeKeyDown = e => { if (e.keyCode === 13) { - // On the user pressing enter (keyCode 13) on the phone number field, - // prevent form submission and request the code. + // On the user pressing enter (keyCode 13) on the validation code field, + // prevent form submission and send the validation code. e.preventDefault() - this._handleSendPhoneVerificationCode() + this._handleSubmitCode() } } - _handleRequestPhoneVerificationCode = async () => { + _handleRequestCode = async () => { // Send phone verification request with the entered values. - const { setFieldValue, setFieldTouched, values: userData } = this.props - const { pendingPhoneNumberFormatted } = userData + const { onRequestCode, setFieldValue, setFieldTouched, values } = this.props + const { newPhoneNumber } = values - // Exit editing first (hides the Send Text button). - this._handleCancelEditNumber() + // Empty the input box (and disable the Send text button). + setFieldValue('newPhoneNumber', '') - const result = await this.props.onRequestPhoneVerificationCode(pendingPhoneNumberFormatted, setFieldValue) + const result = await onRequestCode(newPhoneNumber, setFieldValue) if (result) { // On success, update the phone number in Formik's state. - setFieldValue('pendingPhoneNumber', result.pendingPhoneNumber) - setFieldValue('pendingPhoneNumberFormatted', result.pendingPhoneNumberFormatted) + setFieldValue('newPhoneNumber', result.pendingPhoneNumberFormatted) // Set empty code field in Formik's state and prompt for code. - setFieldValue('phoneValidationCode', '') - setFieldTouched('phoneValidationCode', false) + setFieldValue('validationCode', '') + setFieldTouched('validationCode', false) this.setState({ - isVerificationPending: true + isEditing: false, + isPending: true }) - } else { - // Else discard the pending phone numbers. - setFieldValue('pendingPhoneNumber', '') - setFieldValue('pendingPhoneNumberFormatted', '') } } - _handleSendPhoneVerificationCode = async () => { - const { setFieldValue, values: userData } = this.props - const { phoneValidationCode } = userData + _handleSubmitCode = async () => { + const { onSubmitCode, onVerified, setFieldValue, values } = this.props + const { validationCode } = values // Clear the code field and disable the verify button. // (The code to send is already captured in const above.) - setFieldValue('phoneValidationCode', '') - - const result = await this.props.onSendPhoneVerificationCode(phoneValidationCode) + setFieldValue('validationCode', '') + const result = await onSubmitCode(validationCode) if (result) { // On success, update the phone number in Formik's state and hide the code field. - setFieldValue('phoneNumber', result.phoneNumber) - setFieldValue('phoneNumberFormatted', result.phoneNumberFormatted) - setFieldValue('pendingPhoneNumber', '') - setFieldValue('pendingPhoneNumberFormatted', '') + setFieldValue('newPhoneNumber', '') this.setState({ - isVerificationPending: false + isPending: false, + verifiedNumber: result.phoneNumberFormatted }) + + onVerified(result) } } @@ -185,22 +295,14 @@ class NotificationPrefsPane extends Component { // except where indicated. const { errors, - handleBlur, - handleChange, - phoneNumberPlaceholder, // provided by UserAccountScreen + phoneNumberPlaceholder, // Provided by parent. touched, - values: userDataWithValidationCode + values } = this.props + const { isPending, verifiedNumber } = this.state - const { - isEditingPhoneNumber, - isVerificationPending - } = this.state - const { - notificationChannel, - phoneNumberFormatted, - pendingPhoneNumberFormatted - } = userDataWithValidationCode + const { newPhoneNumber } = values + const { isEditing } = this.state // Here are the states we are dealing with: // - First time entering a phone number/validation code (blank value, not modified) @@ -209,117 +311,76 @@ class NotificationPrefsPane extends Component { // => red error. // - Typing a phone number that doesn't match the configured phoneNumberRegEx // => red error. - - const phoneFieldValidationState = getValidationState(this.props, 'pendingPhoneNumberFormatted') - const codeFieldValidationState = getValidationState(this.props, 'phoneValidationCode') + const phoneErrorState = getErrorState(this.props, 'newPhoneNumber') + const codeErrorState = getErrorState(this.props, 'validationCode') return ( -
-

- You can receive notifications about trips you frequently take. -

- - How would you like to receive notifications? - - - {allowedNotificationChannels.map(({ type, text }, index) => ( - + {isEditing + ? ( + + Enter your phone number for SMS notifications: + + + + { // Show cancel button only if a phone number is already recorded. + verifiedNumber && } + {touched.newPhoneNumber && {errors.newPhoneNumber}} + + + ) : ( - Notification emails will be sent to: - + SMS notifications will be sent to: + + + {isPending ? `${newPhoneNumber} (pending)` : verifiedNumber} + + + )} - {notificationChannel === 'sms' && ( -
- {isEditingPhoneNumber - ? ( - - Enter your phone number for SMS notifications: - - - - - {touched.pendingPhoneNumberFormatted && {errors.pendingPhoneNumberFormatted}} - - - ) : ( - - SMS notifications will be sent to: - - - {isVerificationPending ? `${pendingPhoneNumberFormatted} (pending)` : phoneNumberFormatted} - - - - - )} - - {isVerificationPending && ( - -

- Please check the SMS messaging app on your mobile phone - for a text message with a verification code, and enter the code below. -

- Verification code: - - - - {touched.phoneValidationCode && errors.phoneValidationCode && ( - {errors.phoneValidationCode} - )} - - Request a new code -
+ + {isPending && ( + +

+ Please check the SMS messaging app on your mobile phone + for a text message with a verification code, and enter the code below. +

+ Verification code: + + + + {touched.validationCode && errors.validationCode && ( + {errors.validationCode} )} -
- )} -
-
+ + Request a new code +
+ )} + ) } } - -export default NotificationPrefsPane diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 90cc4ea41..a82fcaf7c 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -46,31 +46,9 @@ function cloneForFormik (userData) { ] clonedUser.savedLocations = reorderedLocations - - // Blank strings for validation code and phone numbers - // (to avoid React warnings). - clonedUser.pendingPhoneNumberFormatted = '' - clonedUser.phoneValidationCode = '' - return clonedUser } -/** - * Returns a user data clone without extra fields we've set to use with Formik. - */ -function cloneForPersistence (userData) { - const persistedData = clone(userData) - delete persistedData.phoneValidationCode - - // Don't save empty strings used to avoid React warnings. - if (persistedData.pendingPhoneNumberFormatted === '') delete persistedData.pendingPhoneNumberFormatted - if (persistedData.pendingPhoneNumber === '') delete persistedData.pendingPhoneNumber - - return persistedData -} - -const INVALID_PHONE_MSG = 'Please enter a valid phone number.' - /** * This screen handles creating/updating OTP user account settings. */ @@ -78,15 +56,15 @@ class UserAccountScreen extends Component { constructor (props) { super(props) - // The validation schema for the form fields (uses props to obtain the configured phone number regex). + // Use props to obtain the configured phone number regex. + this.phoneRegExp = new RegExp(props.phoneNumberRegExp) + + // The validation schema for the form fields. this.validationSchema = yup.object({ email: yup.string().email(), hasConsentedToTerms: yup.boolean().oneOf([true], 'You must agree to the terms to continue.'), + isEditingPhoneNumber: yup.boolean(), notificationChannel: yup.string().oneOf(['email', 'sms', 'none']), - pendingPhoneNumberFormatted: yup.string() - .required(INVALID_PHONE_MSG) - .matches(new RegExp(props.phoneNumberRegExp), INVALID_PHONE_MSG), - phoneValidationCode: yup.string().required('Please enter a validation code.'), savedLocations: yup.array().of(yup.object({ address: yup.string(), icon: yup.string(), @@ -111,7 +89,7 @@ class UserAccountScreen extends Component { // TODO: Change state of Save button while the update action takes place. // In userData.savedLocations, filter out entries with blank addresses. - const newUserData = cloneForPersistence(userData) + const newUserData = clone(userData) newUserData.savedLocations = newUserData.savedLocations.filter(({ address }) => address && address.length) await this.props.createOrUpdateUser(newUserData, silentOnSucceed) @@ -127,7 +105,7 @@ class UserAccountScreen extends Component { * @returns The new user id the the caller can use. */ _handleCreateNewUser = async userData => { - await this._updateUserPrefs(cloneForPersistence(userData), true) + await this._updateUserPrefs(userData, true) return this.props.loggedInUser.id } @@ -218,8 +196,8 @@ class UserAccountScreen extends Component { {/* TODO: Do mobile view. */} ) } @@ -263,6 +242,7 @@ class UserAccountScreen extends Component { onSendPhoneVerificationCode={this._handleSendPhoneVerificationCode} panes={this._panes} phoneNumberPlaceholder={phoneNumberPlaceholder} + phoneNumberRegExp={this.phoneRegExp} /> ) } diff --git a/lib/util/ui.js b/lib/util/ui.js index b9c4b3923..6760b051e 100644 --- a/lib/util/ui.js +++ b/lib/util/ui.js @@ -1,5 +1,13 @@ import { Children, isValidElement, cloneElement } from 'react' +/** + * @param {*} string the string to test. + * @returns true if the string is null or of zero length. + */ +export function isBlank (string) { + return !(string && string.length) +} + /** * Renders children with additional props. * Modified from From 79232ce511a218cea232c5f30bd395330a9a3cc2 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 15 Oct 2020 00:06:27 -0400 Subject: [PATCH 27/55] refactor: Make other light refactors. --- .../user/notification-prefs-pane.js | 4 +-- lib/components/user/user-account-screen.js | 27 +++++++------------ 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 3984c8822..9cf84307b 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -299,10 +299,8 @@ class PhoneNumberEditor extends Component { touched, values } = this.props - const { isPending, verifiedNumber } = this.state - const { newPhoneNumber } = values - const { isEditing } = this.state + const { isEditing, isPending, verifiedNumber } = this.state // Here are the states we are dealing with: // - First time entering a phone number/validation code (blank value, not modified) diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index a82fcaf7c..5f81f1993 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -116,13 +116,13 @@ class UserAccountScreen extends Component { } /** - * Requests a phone verification code to the specified phone number. + * Requests a phone verification SMS to the specified phone number. * This handler is called when the user clicks "Verify my phone" after entering a new number, * and also when the user clicks "Request a new code" from the verification modal. * * @returns The pending phone number (plain and formatted) if the request succeeded, null otherwise. */ - _handleRequestPhoneVerificationCode = async numberToVerify => { + _handleRequestPhoneVerificationSms = async numberToVerify => { const { lastPhoneNumberRequested, lastPhoneRequestTime } = this.state const timestamp = new Date() @@ -211,6 +211,7 @@ class UserAccountScreen extends Component { // can be wired to be managed by Formik. props => { let formContents + let DisplayComponent if (this.state.isNewUser) { if (!auth.user.email_verified) { // Check and prompt for email verification first to avoid extra user wait. @@ -218,27 +219,19 @@ class UserAccountScreen extends Component { } else { // New users are shown "wizard" (step-by-step) mode // (includes when a "new" user clicks "My Account" from the account menu in the nav bar). - formContents = ( - - ) + DisplayComponent = NewAccountWizard } } else { // Existing users are shown all panes together. + DisplayComponent = ExistingAccountDisplay + } + if (DisplayComponent) { formContents = ( - Date: Thu, 15 Oct 2020 09:40:09 -0400 Subject: [PATCH 28/55] refactor(UserAccountScreen): Move validation back to const. Add label for pending numbers. --- .../user/notification-prefs-pane.js | 7 ++++- lib/components/user/user-account-screen.js | 29 +++++++++---------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 9cf84307b..339f04b2a 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -7,6 +7,7 @@ import { FormControl, FormGroup, HelpBlock, + Label, ToggleButton, ToggleButtonGroup } from 'react-bootstrap' @@ -343,7 +344,11 @@ class PhoneNumberEditor extends Component { SMS notifications will be sent to: - {isPending ? `${newPhoneNumber} (pending)` : verifiedNumber} + {isPending + // eslint-disable-next-line jsx-a11y/label-has-for + ? <>{newPhoneNumber} + : verifiedNumber + } diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 5f81f1993..517264786 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -18,6 +18,19 @@ import TermsOfUsePane from './terms-of-use-pane' import VerifyEmailScreen from './verify-email-screen' import withLoggedInUserSupport from './with-logged-in-user-support' +// The validation schema for the form fields. +const validationSchema = yup.object({ + email: yup.string().email(), + hasConsentedToTerms: yup.boolean().oneOf([true], 'You must agree to the terms to continue.'), + notificationChannel: yup.string().oneOf(['email', 'sms', 'none']), + savedLocations: yup.array().of(yup.object({ + address: yup.string(), + icon: yup.string(), + type: yup.string() + })), + storeTripHistory: yup.boolean() +}) + /** * Makes a copy of the logged-in user data for the Formik initial state, with: * - the 'home' and 'work' locations at the top of the savedLocations list @@ -59,20 +72,6 @@ class UserAccountScreen extends Component { // Use props to obtain the configured phone number regex. this.phoneRegExp = new RegExp(props.phoneNumberRegExp) - // The validation schema for the form fields. - this.validationSchema = yup.object({ - email: yup.string().email(), - hasConsentedToTerms: yup.boolean().oneOf([true], 'You must agree to the terms to continue.'), - isEditingPhoneNumber: yup.boolean(), - notificationChannel: yup.string().oneOf(['email', 'sms', 'none']), - savedLocations: yup.array().of(yup.object({ - address: yup.string(), - icon: yup.string(), - type: yup.string() - })), - storeTripHistory: yup.boolean() - }) - this.state = { // Capture whether user is a new user at this stage, and retain that value as long as this screen is active. // Reminder: When a new user progresses through the account steps, @@ -199,7 +198,7 @@ class UserAccountScreen extends Component { // Avoid validating on change as it is annoying. Validating on blur is enough. validateOnChange={false} validateOnBlur - validationSchema={this.validationSchema} + validationSchema={validationSchema} onSubmit={this._handleSaveAndExit} initialValues={cloneForFormik(loggedInUser)} > From 920e0deb208ab021b57b2bfdb8a7c1cb0813a9b8 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 15 Oct 2020 11:20:49 -0400 Subject: [PATCH 29/55] refactor(actions/user): Finish refactor user actions per #246 comments. --- lib/actions/user.js | 129 +++++++++------------ lib/components/user/user-account-screen.js | 4 +- 2 files changed, 60 insertions(+), 73 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 4d222d95b..6905a7c7b 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -25,33 +25,25 @@ function createNewUser (auth0User) { } /** - * Helper function that - * - extracts key variables from the state and passes them to the code to execute: - * - apiKey and apiBaseUrl from state.otp.config.persistence.otp_middleware, - * - accessToken and loggedIUnUser from state.user. - * - checks that otp_middleware is set, and throws an error if not. - * @param functionToExecute a function that can be waited upon, - * with parameters (dispatch, arguments), that contains the code to be - * executed if the OTP middleware is configured. - * @return a redux action for the code to be executed. + * Extracts accessToken, loggedInUser from the redux state, + * and apiBaseUrl, apiKey from the middleware configuration. + * If the middleware configuration does not exist, throws an error. */ -function executeWithMiddleware (functionToExecute) { - return async function (dispatch, getState) { - const { otp, user } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence +function getMiddlewareVariables (state) { + const { otp, user } = state + const { otp_middleware: otpMiddleware = null } = otp.config.persistence - if (otpMiddleware) { - const { accessToken, loggedInUser } = user - const { apiBaseUrl, apiKey } = otpMiddleware - await functionToExecute(dispatch, { - accessToken, - apiBaseUrl, - apiKey, - loggedInUser - }) - } else { - throw new Error('This action requires a valid middleware configuration.') + if (otpMiddleware) { + const { accessToken, loggedInUser } = user + const { apiBaseUrl, apiKey } = otpMiddleware + return { + accessToken, + apiBaseUrl, + apiKey, + loggedInUser } + } else { + throw new Error('This action requires a valid middleware configuration.') } } @@ -61,14 +53,15 @@ function executeWithMiddleware (functionToExecute) { * whether the process to populate state.user is completed or not. */ export function fetchUserMonitoredTrips (accessToken) { - return executeWithMiddleware(async (dispatch, { apiBaseUrl, apiKey }) => { + return async function (dispatch, getState) { + const { apiBaseUrl, apiKey } = getMiddlewareVariables(getState()) const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` const { data: trips, status } = await secureFetch(requestUrl, accessToken, apiKey, 'GET') if (status === 'success') { dispatch(setCurrentUserMonitoredTrips(trips)) } - }) + } } /** @@ -76,7 +69,8 @@ export function fetchUserMonitoredTrips (accessToken) { * or set initial values under state.user if no user has been loaded. */ export function fetchOrInitializeUser (auth) { - return executeWithMiddleware(async (dispatch, { apiBaseUrl, apiKey }) => { + return async function (dispatch, getState) { + const { apiBaseUrl, apiKey } = getMiddlewareVariables(getState()) const { accessToken, user: authUser } = auth const requestUrl = `${apiBaseUrl}${API_USER_PATH}/fromtoken` @@ -112,7 +106,7 @@ export function fetchOrInitializeUser (auth) { } else { dispatch(setCurrentUser({ accessToken, user: createNewUser(authUser) })) } - }) + } } /** @@ -122,7 +116,8 @@ export function fetchOrInitializeUser (auth) { * @param silentOnSuccess true to suppress the confirmation if the operation is successful (e.g. immediately after user accepts the terms). */ export function createOrUpdateUser (userData, silentOnSuccess = false) { - return executeWithMiddleware(async (dispatch, { accessToken, apiBaseUrl, apiKey, loggedInUser }) => { + return async function (dispatch, getState) { + const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) const { id } = userData // Middleware ID, NOT auth0 (or similar) id. let requestUrl, method @@ -155,7 +150,7 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) { } else { alert('Corrupted state: User ID not available for exiting user.') } - }) + } } /** @@ -164,7 +159,8 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) { * and refreshes the redux monitoredTrips with the updated trip. */ export function createOrUpdateUserMonitoredTrip (tripData, isNew, silentOnSuccess) { - return executeWithMiddleware(async (dispatch, { accessToken, apiBaseUrl, apiKey }) => { + return async function (dispatch, getState) { + const { accessToken, apiBaseUrl, apiKey } = getMiddlewareVariables(getState()) const { id } = tripData let requestUrl, method @@ -196,18 +192,19 @@ export function createOrUpdateUserMonitoredTrip (tripData, isNew, silentOnSucces } else { alert('Corrupted state: Trip ID not available for exiting trip.') } - }) + } } /** * Deletes a logged-in user's monitored trip, * then, if that was successful, refreshes the redux monitoredTrips state. */ -export function deleteUserMonitoredTrip (id) { - return executeWithMiddleware(async (dispatch, { accessToken, apiBaseUrl, apiKey }) => { - const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${id}` +export function deleteUserMonitoredTrip (tripId) { + return async function (dispatch, getState) { + const { accessToken, apiBaseUrl, apiKey } = getMiddlewareVariables(getState()) + const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${tripId}` - if (id) { + if (tripId) { const deleteResult = secureFetch(requestUrl, accessToken, apiKey, 'DELETE') if (deleteResult.status === 'success') { // Reload user's monitored trips after deletion. @@ -218,31 +215,25 @@ export function deleteUserMonitoredTrip (id) { } else { alert('Corrupted state: Monitored Trip ID not available for exiting user.') } - }) + } } /** * Requests a verification code via SMS for the logged-in user. */ -export function requestPhoneVerificationSms (userId, newPhoneNumber) { +export function requestPhoneVerificationSms (newPhoneNumber) { return async function (dispatch, getState) { - const { otp, user } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence + const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) + const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${loggedInUser.id}${API_USER_VERIFYSMS_PATH}/${encodeURIComponent(newPhoneNumber)}` - if (otpMiddleware) { - const { accessToken } = user + const sendSmsResult = secureFetch(requestUrl, accessToken, apiKey, 'GET') - // Send the SMS request with the phone number to update. - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${encodeURIComponent(newPhoneNumber)}` - const sendSmsResult = secureFetch(requestUrl, accessToken, apiKey, 'GET') - - if (sendSmsResult.status === 'success') { - // Refetch user and update application state with new phone number and verification status. - // (This also refetches the user's monitored trip, and that's ok.) - await dispatch(fetchOrInitializeUser({ accessToken })) - } else { - alert(`An error was encountered:\n${JSON.stringify(sendSmsResult)}`) - } + if (sendSmsResult.status === 'success') { + // Refetch user and update application state with new phone number and verification status. + // (This also refetches the user's monitored trip, and that's ok.) + await dispatch(fetchOrInitializeUser({ accessToken })) + } else { + alert(`An error was encountered:\n${JSON.stringify(sendSmsResult)}`) } } } @@ -250,28 +241,24 @@ export function requestPhoneVerificationSms (userId, newPhoneNumber) { /** * Validate the phone number verification code for the logged-in user. */ -export function verifyPhoneNumber (userId, code) { +export function verifyPhoneNumber (code) { return async function (dispatch, getState) { - const { otp, user } = getState() - const { otp_middleware: otpMiddleware = null } = otp.config.persistence + const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) + const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${loggedInUser.id}${API_USER_VERIFYSMS_PATH}/${code}` - if (otpMiddleware) { - const { accessToken } = user - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${userId}${API_USER_VERIFYSMS_PATH}/${code}` - const sendResult = secureFetch(requestUrl, accessToken, apiKey, 'POST') - - // If the check is successful, status in the returned data will be "approved". - if (sendResult.status === 'success' && sendResult.data) { - if (sendResult.data.status === 'approved') { - // Refetch user and update application state with new phone number and verification status. - // (This also refetches the user's monitored trip, and that's ok.) - await dispatch(fetchOrInitializeUser({ accessToken })) - } else { - alert('You entered in incorrect validation code. Please try again.') - } + const sendResult = secureFetch(requestUrl, accessToken, apiKey, 'POST') + + // If the check is successful, status in the returned data will be "approved". + if (sendResult.status === 'success' && sendResult.data) { + if (sendResult.data.status === 'approved') { + // Refetch user and update application state with new phone number and verification status. + // (This also refetches the user's monitored trip, and that's ok.) + await dispatch(fetchOrInitializeUser({ accessToken })) } else { - alert(`Your phone could not be verified:\n${JSON.stringify(sendResult)}`) + alert('You entered in incorrect validation code. Please try again.') } + } else { + alert(`Your phone could not be verified:\n${JSON.stringify(sendResult)}`) } } } diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 517264786..01974ae97 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -130,7 +130,7 @@ class UserAccountScreen extends Component { // TODO: Should throttling be handled in the middleware? if (lastPhoneNumberRequested !== numberToVerify || (lastPhoneRequestTime && lastPhoneRequestTime <= timestamp + 60000)) { - await this.props.requestPhoneVerificationSms(this.props.loggedInUser.id, numberToVerify) + await this.props.requestPhoneVerificationSms(numberToVerify) // (props have been updated are set at this point.) const { pendingPhoneNumber, pendingPhoneNumberFormatted } = this.props.loggedInUser @@ -156,7 +156,7 @@ class UserAccountScreen extends Component { * @returns The new phone number (plain and formatted) if the verification succeeded, null otherwise. */ _handleSendPhoneVerificationCode = async code => { - await this.props.verifyPhoneNumber(this.props.loggedInUser.id, code) + await this.props.verifyPhoneNumber(code) // When the pending phone number is erased by the backend, // this means the phone number has been verified. From 7970429718b1ee31eb8be8a248dc639e9b954ec1 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 15 Oct 2020 11:28:13 -0400 Subject: [PATCH 30/55] refactor(user/actions): await fetches. --- lib/actions/user.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 6905a7c7b..6ddd8459c 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -205,7 +205,7 @@ export function deleteUserMonitoredTrip (tripId) { const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${tripId}` if (tripId) { - const deleteResult = secureFetch(requestUrl, accessToken, apiKey, 'DELETE') + const deleteResult = await secureFetch(requestUrl, accessToken, apiKey, 'DELETE') if (deleteResult.status === 'success') { // Reload user's monitored trips after deletion. await dispatch(fetchUserMonitoredTrips(accessToken)) @@ -226,7 +226,7 @@ export function requestPhoneVerificationSms (newPhoneNumber) { const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${loggedInUser.id}${API_USER_VERIFYSMS_PATH}/${encodeURIComponent(newPhoneNumber)}` - const sendSmsResult = secureFetch(requestUrl, accessToken, apiKey, 'GET') + const sendSmsResult = await secureFetch(requestUrl, accessToken, apiKey, 'GET') if (sendSmsResult.status === 'success') { // Refetch user and update application state with new phone number and verification status. @@ -246,7 +246,7 @@ export function verifyPhoneNumber (code) { const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${loggedInUser.id}${API_USER_VERIFYSMS_PATH}/${code}` - const sendResult = secureFetch(requestUrl, accessToken, apiKey, 'POST') + const sendResult = await secureFetch(requestUrl, accessToken, apiKey, 'POST') // If the check is successful, status in the returned data will be "approved". if (sendResult.status === 'success' && sendResult.data) { From 9fa9b3cdaf89f27ee4e9aa297d098155d0412e07 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 15 Oct 2020 19:42:11 -0400 Subject: [PATCH 31/55] refactor(PhoneNumberEditor): Add config for formatting numbers. --- example-config.yml | 21 ++++++++---- .../user/notification-prefs-pane.js | 34 +++++++++---------- lib/components/user/user-account-screen.js | 31 +++++++---------- lib/util/ui.js | 14 ++++++++ 4 files changed, 57 insertions(+), 43 deletions(-) diff --git a/example-config.yml b/example-config.yml index 09a87d288..6919f6012 100644 --- a/example-config.yml +++ b/example-config.yml @@ -141,10 +141,17 @@ itinerary: # - WALK # - BICYCLE -### If using OTP Middleware, users can enable phone notifications. -### - phoneNumberRegExp should be set for the target locale. -### - phoneNumberPlaceholder is optional. -# notifications: -# # For US numbers: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html -# phoneNumberRegExp: ^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$ -# phoneNumberPlaceholder: (555) 555-0123 +### If using OTP Middleware, you can define the optional phone number formats +### below for the target locale. +# phoneFormatOptions: +# # Regex for validating phone numbers entered by the user +# # For US numbers: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html +# validationRegExp: ^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$ +# # The placeholder displayed in phone number controls +# inputPlaceholder: (555) 555-0123 +# # Regex that defines the groups of digits (the structure) to extract from raw phone numbers shown in +15555550123 format. +# # If not specified, phone numbers will be displayed in raw format. +# rawStructureRegExp: \+1(\d{3})(\d{3})(\d{4}) +# # The format for displaying the regex capture groups ($1, $2, ...) of the above structure. +# # If not specified, phone numbers will be displayed in raw format. +# formattedStructure: ($1) $2-$3 diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 339f04b2a..428e5809b 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -14,7 +14,7 @@ import { import styled, { css } from 'styled-components' import * as yup from 'yup' -import { isBlank } from '../../util/ui' +import { formatPhoneNumber, isBlank } from '../../util/ui' const allowedNotificationChannels = [ { @@ -86,17 +86,17 @@ class NotificationPrefsPane extends Component { this.phoneValidationSchema = yup.object({ newPhoneNumber: yup.string() .required(INVALID_PHONE_MSG) - .matches(props.phoneNumberRegExp, INVALID_PHONE_MSG), + .matches(new RegExp(props.phoneFormatOptions.validationRegExp), INVALID_PHONE_MSG), validationCode: yup.string().required('Please enter a validation code.') }) } - _handleOnPhoneVerified = ({ phoneNumber, phoneNumberFormatted }) => { + _handleOnPhoneVerified = ({ isPhoneNumberVerified, phoneNumber }) => { const { setFieldValue } = this.props // Update the parent's Formik state (so when saving, it sends the updated numbers) + setFieldValue('isPhoneNumberVerified', isPhoneNumberVerified) setFieldValue('phoneNumber', phoneNumber) - setFieldValue('phoneNumberFormatted', phoneNumberFormatted) } render () { @@ -107,8 +107,7 @@ class NotificationPrefsPane extends Component { handleChange, onRequestPhoneVerificationCode, // * onSendPhoneVerificationCode, // * - phoneNumberPlaceholder, // * - phoneNumberRegExp, // * + phoneFormatOptions, // * values: userData } = this.props @@ -176,8 +175,7 @@ class NotificationPrefsPane extends Component { onRequestCode={onRequestPhoneVerificationCode} onSubmitCode={onSendPhoneVerificationCode} onVerified={this._handleOnPhoneVerified} - phoneNumberPlaceholder={phoneNumberPlaceholder} - phoneNumberRegExp={phoneNumberRegExp} + phoneFormatOptions={phoneFormatOptions} /> ) } @@ -209,7 +207,7 @@ class PhoneNumberEditor extends Component { isPending: false, // Holds the last known verified phone number. - verifiedNumber: initialPhoneNumber + verifiedNumber: formatPhoneNumber(initialPhoneNumber, props.phoneFormatOptions) } } @@ -247,17 +245,18 @@ class PhoneNumberEditor extends Component { _handleRequestCode = async () => { // Send phone verification request with the entered values. - const { onRequestCode, setFieldValue, setFieldTouched, values } = this.props + const { onRequestCode, phoneFormatOptions, setFieldValue, setFieldTouched, values } = this.props const { newPhoneNumber } = values // Empty the input box (and disable the Send text button). setFieldValue('newPhoneNumber', '') + setFieldTouched('newPhoneNumber', false) - const result = await onRequestCode(newPhoneNumber, setFieldValue) + const result = await onRequestCode(newPhoneNumber) if (result) { - // On success, update the phone number in Formik's state. - setFieldValue('newPhoneNumber', result.pendingPhoneNumberFormatted) + // On success, update the formatted phone number in Formik's state. + setFieldValue('newPhoneNumber', formatPhoneNumber(result.phoneNumber, phoneFormatOptions)) // Set empty code field in Formik's state and prompt for code. setFieldValue('validationCode', '') @@ -270,7 +269,7 @@ class PhoneNumberEditor extends Component { } _handleSubmitCode = async () => { - const { onSubmitCode, onVerified, setFieldValue, values } = this.props + const { onSubmitCode, onVerified, phoneFormatOptions, setFieldValue, values } = this.props const { validationCode } = values // Clear the code field and disable the verify button. @@ -278,13 +277,14 @@ class PhoneNumberEditor extends Component { setFieldValue('validationCode', '') const result = await onSubmitCode(validationCode) + if (result) { // On success, update the phone number in Formik's state and hide the code field. setFieldValue('newPhoneNumber', '') this.setState({ isPending: false, - verifiedNumber: result.phoneNumberFormatted + verifiedNumber: formatPhoneNumber(result.phoneNumber, phoneFormatOptions) }) onVerified(result) @@ -296,7 +296,7 @@ class PhoneNumberEditor extends Component { // except where indicated. const { errors, - phoneNumberPlaceholder, // Provided by parent. + phoneFormatOptions, // Provided by parent. touched, values } = this.props @@ -323,7 +323,7 @@ class PhoneNumberEditor extends Component { diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 01974ae97..8fc7b057b 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -69,9 +69,6 @@ class UserAccountScreen extends Component { constructor (props) { super(props) - // Use props to obtain the configured phone number regex. - this.phoneRegExp = new RegExp(props.phoneNumberRegExp) - this.state = { // Capture whether user is a new user at this stage, and retain that value as long as this screen is active. // Reminder: When a new user progresses through the account steps, @@ -133,18 +130,16 @@ class UserAccountScreen extends Component { await this.props.requestPhoneVerificationSms(numberToVerify) // (props have been updated are set at this point.) - const { pendingPhoneNumber, pendingPhoneNumberFormatted } = this.props.loggedInUser + const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser this.setState({ - // (The reformatted number may be different from the value initially passed, - // so the throttling condition is not a rigourous check.) - lastPhoneNumberRequested: pendingPhoneNumberFormatted, + lastPhoneNumberRequested: phoneNumber, lastPhoneRequestTime: timestamp }) // When the pending phone number is populated by the backend, // this means the phone number verification SMS has been sent. - if (pendingPhoneNumber) { - return { pendingPhoneNumber, pendingPhoneNumberFormatted } + if (!isPhoneNumberVerified) { + return { isPhoneNumberVerified, phoneNumber } } } @@ -160,9 +155,9 @@ class UserAccountScreen extends Component { // When the pending phone number is erased by the backend, // this means the phone number has been verified. - const { pendingPhoneNumber, phoneNumber, phoneNumberFormatted } = this.props.loggedInUser - if (!pendingPhoneNumber) { - return { phoneNumber, phoneNumberFormatted } + const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser + if (isPhoneNumberVerified) { + return { isPhoneNumberVerified, phoneNumber } } return null @@ -188,7 +183,7 @@ class UserAccountScreen extends Component { // TODO: Update title bar during componentDidMount. render () { - const { auth, loggedInUser, phoneNumberPlaceholder } = this.props + const { auth, loggedInUser, phoneFormatOptions } = this.props return (
@@ -233,8 +228,7 @@ class UserAccountScreen extends Component { onRequestPhoneVerificationCode={this._handleRequestPhoneVerificationSms} onSendPhoneVerificationCode={this._handleSendPhoneVerificationCode} panes={this._panes} - phoneNumberPlaceholder={phoneNumberPlaceholder} - phoneNumberRegExp={this.phoneRegExp} + phoneFormatOptions={phoneFormatOptions} /> ) } @@ -255,12 +249,11 @@ class UserAccountScreen extends Component { // connect to the redux store const mapStateToProps = (state, ownProps) => { - const { notifications: notificationsConfig = {} } = state.otp.config - const { phoneNumberPlaceholder, phoneNumberRegExp } = notificationsConfig + // Make phoneFormatOptions not null. + const { phoneFormatOptions = {} } = state.otp.config return { loggedInUser: state.user.loggedInUser, - phoneNumberPlaceholder, - phoneNumberRegExp + phoneFormatOptions } } diff --git a/lib/util/ui.js b/lib/util/ui.js index 6760b051e..ceac5bf10 100644 --- a/lib/util/ui.js +++ b/lib/util/ui.js @@ -8,6 +8,20 @@ export function isBlank (string) { return !(string && string.length) } +/** + * Formats a phone number according to the structure and format in phoneFormatOptions. + * @param {*} rawNumber The raw number (e.g. +155555555) to process + * @param {*} phoneFormatOptions The phoneFormatOptions from the configuration. + */ +export function formatPhoneNumber (rawNumber, phoneFormatOptions) { + const { rawStructureRegExp, formattedStructure } = phoneFormatOptions + + if (rawStructureRegExp && formattedStructure) { + return rawNumber.replace(new RegExp(rawStructureRegExp), formattedStructure) + } + return rawNumber +} + /** * Renders children with additional props. * Modified from From 5690ac83559ffaf8c4d355f6744480832f029147 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 16 Oct 2020 12:18:20 -0400 Subject: [PATCH 32/55] refactor(NotificationPrefsPane): Use awesome-phone library. --- example-config.yml | 21 +++++----- .../user/notification-prefs-pane.js | 39 ++++++++++++------- lib/components/user/user-account-screen.js | 6 ++- lib/util/ui.js | 8 ++-- package.json | 1 + yarn.lock | 5 +++ 6 files changed, 50 insertions(+), 30 deletions(-) diff --git a/example-config.yml b/example-config.yml index 6919f6012..9bec2cf35 100644 --- a/example-config.yml +++ b/example-config.yml @@ -144,14 +144,13 @@ itinerary: ### If using OTP Middleware, you can define the optional phone number formats ### below for the target locale. # phoneFormatOptions: -# # Regex for validating phone numbers entered by the user -# # For US numbers: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html -# validationRegExp: ^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$ -# # The placeholder displayed in phone number controls -# inputPlaceholder: (555) 555-0123 -# # Regex that defines the groups of digits (the structure) to extract from raw phone numbers shown in +15555550123 format. -# # If not specified, phone numbers will be displayed in raw format. -# rawStructureRegExp: \+1(\d{3})(\d{3})(\d{4}) -# # The format for displaying the regex capture groups ($1, $2, ...) of the above structure. -# # If not specified, phone numbers will be displayed in raw format. -# formattedStructure: ($1) $2-$3 +# # The placeholder displayed in phone number controls +# inputPlaceholder: (555) 555-0123 +# # Region code for converting between raw E.164 numbers and national format. +# # (defaults to 'US') +# regionCode: US +# # awesome-phonenumber is a little bit too strict on validating formatted numbers +# # (although converting to raw E.164 number still works fine in such cases), +# # so we provide our own regex for validating user input instead. +# # For US numbers: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html +# validationRegExp: ^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$ diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 428e5809b..f5c918dc9 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -1,3 +1,4 @@ +import PhoneNumber from 'awesome-phonenumber' import { Field, Formik } from 'formik' import React, { Component } from 'react' import { @@ -112,6 +113,7 @@ class NotificationPrefsPane extends Component { } = this.props const { + isPhoneNumberVerified, notificationChannel, phoneNumber } = userData @@ -172,6 +174,7 @@ class NotificationPrefsPane extends Component { { // Show cancel button only if a phone number is already recorded. - verifiedNumber && } + currentNumber && } {touched.newPhoneNumber && {errors.newPhoneNumber}} @@ -347,7 +355,8 @@ class PhoneNumberEditor extends Component { {isPending // eslint-disable-next-line jsx-a11y/label-has-for ? <>{newPhoneNumber} - : verifiedNumber + // eslint-disable-next-line jsx-a11y/label-has-for + : <>{currentNumber} {currentNumberVerified && } } diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 8fc7b057b..fb09e8d12 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -249,8 +249,12 @@ class UserAccountScreen extends Component { // connect to the redux store const mapStateToProps = (state, ownProps) => { - // Make phoneFormatOptions not null. + // Make phoneFormatOptions not null with default US region code if not specified. const { phoneFormatOptions = {} } = state.otp.config + if (!phoneFormatOptions.regionCode) { + phoneFormatOptions.regionCode = 'US' + } + return { loggedInUser: state.user.loggedInUser, phoneFormatOptions diff --git a/lib/util/ui.js b/lib/util/ui.js index ceac5bf10..d26fa59a4 100644 --- a/lib/util/ui.js +++ b/lib/util/ui.js @@ -1,3 +1,4 @@ +import PhoneNumber from 'awesome-phonenumber' import { Children, isValidElement, cloneElement } from 'react' /** @@ -12,12 +13,13 @@ export function isBlank (string) { * Formats a phone number according to the structure and format in phoneFormatOptions. * @param {*} rawNumber The raw number (e.g. +155555555) to process * @param {*} phoneFormatOptions The phoneFormatOptions from the configuration. + * @return The formatted number, or the raw number if no region code was provided. */ export function formatPhoneNumber (rawNumber, phoneFormatOptions) { - const { rawStructureRegExp, formattedStructure } = phoneFormatOptions + const { regionCode } = phoneFormatOptions - if (rawStructureRegExp && formattedStructure) { - return rawNumber.replace(new RegExp(rawStructureRegExp), formattedStructure) + if (regionCode) { + return new PhoneNumber(rawNumber, regionCode).getNumber('national') } return rawNumber } diff --git a/package.json b/package.json index 95872ad2c..4dba1d205 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "@opentripplanner/trip-form": "^1.0.2", "@opentripplanner/trip-viewer-overlay": "^1.0.1", "@opentripplanner/vehicle-rental-overlay": "^1.0.1", + "awesome-phonenumber": "^2.40.0", "bootstrap": "^3.3.7", "bowser": "^1.9.3", "clone": "^2.1.0", diff --git a/yarn.lock b/yarn.lock index 9257334c6..a9abe658e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2676,6 +2676,11 @@ autoprefixer@^9.6.1: postcss "^7.0.17" postcss-value-parser "^4.0.0" +awesome-phonenumber@^2.40.0: + version "2.40.0" + resolved "https://registry.yarnpkg.com/awesome-phonenumber/-/awesome-phonenumber-2.40.0.tgz#02963b7a1a372572e42fb827401eb78d9ac4e294" + integrity sha512-ViGH+iNPzFCQ1s0Lut8jQC2dP05SjnntlA10emT8ANXy3UtyWykeWvKaDG17eWIcSjXqPvdKkPCco9Nu+Egj1g== + aws-sdk@^2.414.0: version "2.502.0" resolved "https://registry.yarnpkg.com/aws-sdk/-/aws-sdk-2.502.0.tgz#b3192f82389db982605462c8394cc3fa8c475b3e" From 708f5b7b8eb3f91aec836eaf0391b1c442a20bbb Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Fri, 16 Oct 2020 12:21:42 -0400 Subject: [PATCH 33/55] refactor(actions/user): Tweak error message text. --- lib/actions/user.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 6ddd8459c..f7910ccdc 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -255,7 +255,8 @@ export function verifyPhoneNumber (code) { // (This also refetches the user's monitored trip, and that's ok.) await dispatch(fetchOrInitializeUser({ accessToken })) } else { - alert('You entered in incorrect validation code. Please try again.') + // 'invalid' captures cases when users enter wrong/incorrect codes or expired codes. + alert('You entered in invalid code. Please try again.') } } else { alert(`Your phone could not be verified:\n${JSON.stringify(sendResult)}`) From 6d2f94a9ab8f2cdb8ce90af749c2fd5e2ce3e026 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 19 Oct 2020 19:31:21 -0400 Subject: [PATCH 34/55] refactor(PhoneNumberEditor): Use react-phone-number-input --- .../user/notification-prefs-pane.js | 103 ++++++++++-------- lib/util/ui.js | 16 --- package.json | 2 +- yarn.lock | 49 ++++++++- 4 files changed, 102 insertions(+), 68 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index f5c918dc9..d016eb7d9 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -1,4 +1,3 @@ -import PhoneNumber from 'awesome-phonenumber' import { Field, Formik } from 'formik' import React, { Component } from 'react' import { @@ -12,10 +11,12 @@ import { ToggleButton, ToggleButtonGroup } from 'react-bootstrap' +import { formatPhoneNumber, isPossiblePhoneNumber } from 'react-phone-number-input' +import Input from 'react-phone-number-input/input' import styled, { css } from 'styled-components' import * as yup from 'yup' -import { formatPhoneNumber, isBlank } from '../../util/ui' +import { isBlank } from '../../util/ui' const allowedNotificationChannels = [ { @@ -54,6 +55,10 @@ const InlineTextInput = styled(FormControl)` const InlineStatic = styled(FormControl.Static)` ${phoneFieldCss} ` +const InlinePhoneInput = styled(Input)` + ${phoneFieldCss} +` + const FlushLink = styled(Button)` padding-left: 0; padding-right: 0; @@ -77,21 +82,14 @@ function getErrorState (props, field) { : null } +const codeValidationSchema = yup.object({ + validationCode: yup.string().required('Please enter a validation code.') +}) + /** * User notification preferences pane. */ class NotificationPrefsPane extends Component { - constructor (props) { - super(props) - - this.phoneValidationSchema = yup.object({ - newPhoneNumber: yup.string() - .required(INVALID_PHONE_MSG) - .matches(new RegExp(props.phoneFormatOptions.validationRegExp), INVALID_PHONE_MSG), - validationCode: yup.string().required('Please enter a validation code.') - }) - } - _handleOnPhoneVerified = ({ isPhoneNumberVerified, phoneNumber }) => { const { setFieldValue } = this.props @@ -119,7 +117,6 @@ class NotificationPrefsPane extends Component { } = userData const initialFormikValues = { - newPhoneNumber: '', validationCode: '' } @@ -163,7 +160,7 @@ class NotificationPrefsPane extends Component { { @@ -203,7 +200,7 @@ class PhoneNumberEditor extends Component { const { initialPhoneNumber, initialPhoneNumberVerified } = props this.state = { // Holds the last known verified phone number. - currentNumber: formatPhoneNumber(initialPhoneNumber, props.phoneFormatOptions), + currentNumber: initialPhoneNumber, // The verified status of the above number. currentNumberVerified: initialPhoneNumberVerified, @@ -213,14 +210,25 @@ class PhoneNumberEditor extends Component { isEditing: isBlank(initialPhoneNumber), // If true, a phone verification request has been sent and the UI is awaiting the user sending the code. - isPending: false + isPending: false, + + // Holds the new phone number (+15555550123 format) entered by the user + // (outside of Formik because (Phone)Input does not have a standard onChange event or simple valitity test). + newPhoneNumber: '' } } _handleEditNumber = () => { this.setState({ isEditing: true, - isPending: false + isPending: false, + newPhoneNumber: '' + }) + } + + _handleNewPhoneNumberChange = newPhoneNumber => { + this.setState({ + newPhoneNumber }) } @@ -251,19 +259,21 @@ class PhoneNumberEditor extends Component { _handleRequestCode = async () => { // Send phone verification request with the entered values. - const { onRequestCode, phoneFormatOptions, setFieldValue, setFieldTouched, values } = this.props - const { newPhoneNumber } = values + const { onRequestCode, setFieldValue, setFieldTouched } = this.props + const { newPhoneNumber } = this.state // Empty the input box (and disable the Send text button). - setFieldValue('newPhoneNumber', '') - setFieldTouched('newPhoneNumber', false) + this.setState({ + newPhoneNumber: '' + }) - const submittedNumber = new PhoneNumber(newPhoneNumber, phoneFormatOptions.regionCode).getNumber('e164') - const result = await onRequestCode(submittedNumber) + const result = await onRequestCode(newPhoneNumber) - if (result && result.phoneNumber === submittedNumber && !result.isPhoneNumberVerified) { - // On success (phoneNumber is updated and verified status is false), update the formatted phone number in Formik's state. - setFieldValue('newPhoneNumber', formatPhoneNumber(result.phoneNumber, phoneFormatOptions)) + if (result && result.phoneNumber === newPhoneNumber && !result.isPhoneNumberVerified) { + // On success (phoneNumber is updated and verified status is false), update the phone number in component state. + this.setState({ + newPhoneNumber: result.phoneNumber + }) // Set empty code field in Formik's state and prompt for code. setFieldValue('validationCode', '') @@ -276,7 +286,7 @@ class PhoneNumberEditor extends Component { } _handleSubmitCode = async () => { - const { onSubmitCode, onVerified, phoneFormatOptions, setFieldValue, values } = this.props + const { onSubmitCode, onVerified, setFieldValue, values } = this.props const { validationCode } = values // Clear the code field and disable the verify button. @@ -286,13 +296,12 @@ class PhoneNumberEditor extends Component { const result = await onSubmitCode(validationCode) if (result && result.isPhoneNumberVerified) { - // On success (status is changed to verified), update the phone number in Formik's state and hide the code field. - setFieldValue('newPhoneNumber', '') - + // On success (status is changed to verified), update the phone number in component state and hide the code field. this.setState({ - currentNumber: formatPhoneNumber(result.phoneNumber, phoneFormatOptions), + currentNumber: result.phoneNumber, currentNumberVerified: true, - isPending: false + isPending: false, + newPhoneNumber: '' }) onVerified(result) @@ -305,11 +314,9 @@ class PhoneNumberEditor extends Component { const { errors, phoneFormatOptions, // Provided by parent. - touched, - values + touched } = this.props - const { newPhoneNumber } = values - const { currentNumber, currentNumberVerified, isEditing, isPending } = this.state + const { currentNumber, currentNumberVerified, isEditing, isPending, newPhoneNumber } = this.state // Here are the states we are dealing with: // - First time entering a phone number/validation code (blank value, not modified) @@ -318,7 +325,8 @@ class PhoneNumberEditor extends Component { // => red error. // - Typing a phone number that doesn't match the configured phoneNumberRegEx // => red error. - const phoneErrorState = getErrorState(this.props, 'newPhoneNumber') + const isPhoneInvalid = !isBlank(newPhoneNumber) && !isPossiblePhoneNumber(newPhoneNumber) + const phoneErrorState = isPhoneInvalid ? 'error' : null const codeErrorState = getErrorState(this.props, 'validationCode') return ( @@ -328,23 +336,26 @@ class PhoneNumberEditor extends Component { Enter your phone number for SMS notifications: - { // Show cancel button only if a phone number is already recorded. currentNumber && } - {touched.newPhoneNumber && {errors.newPhoneNumber}} + {isPhoneInvalid && {INVALID_PHONE_MSG}} ) : ( @@ -354,9 +365,9 @@ class PhoneNumberEditor extends Component { {isPending // eslint-disable-next-line jsx-a11y/label-has-for - ? <>{newPhoneNumber} + ? <>{formatPhoneNumber(newPhoneNumber)} // eslint-disable-next-line jsx-a11y/label-has-for - : <>{currentNumber} {currentNumberVerified && } + : <>{formatPhoneNumber(currentNumber)} {currentNumberVerified && } } diff --git a/lib/util/ui.js b/lib/util/ui.js index d26fa59a4..6760b051e 100644 --- a/lib/util/ui.js +++ b/lib/util/ui.js @@ -1,4 +1,3 @@ -import PhoneNumber from 'awesome-phonenumber' import { Children, isValidElement, cloneElement } from 'react' /** @@ -9,21 +8,6 @@ export function isBlank (string) { return !(string && string.length) } -/** - * Formats a phone number according to the structure and format in phoneFormatOptions. - * @param {*} rawNumber The raw number (e.g. +155555555) to process - * @param {*} phoneFormatOptions The phoneFormatOptions from the configuration. - * @return The formatted number, or the raw number if no region code was provided. - */ -export function formatPhoneNumber (rawNumber, phoneFormatOptions) { - const { regionCode } = phoneFormatOptions - - if (regionCode) { - return new PhoneNumber(rawNumber, regionCode).getNumber('national') - } - return rawNumber -} - /** * Renders children with additional props. * Modified from diff --git a/package.json b/package.json index 4dba1d205..208c071d9 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,6 @@ "@opentripplanner/trip-form": "^1.0.2", "@opentripplanner/trip-viewer-overlay": "^1.0.1", "@opentripplanner/vehicle-rental-overlay": "^1.0.1", - "awesome-phonenumber": "^2.40.0", "bootstrap": "^3.3.7", "bowser": "^1.9.3", "clone": "^2.1.0", @@ -79,6 +78,7 @@ "react-draggable": "^4.4.3", "react-fontawesome": "^1.5.0", "react-loading-skeleton": "^2.1.1", + "react-phone-number-input": "^3.1.0", "react-redux": "^7.1.0", "react-resize-detector": "^2.1.0", "react-router": "^5.0.1", diff --git a/yarn.lock b/yarn.lock index a9abe658e..a5ffb79e9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2676,11 +2676,6 @@ autoprefixer@^9.6.1: postcss "^7.0.17" postcss-value-parser "^4.0.0" -awesome-phonenumber@^2.40.0: - version "2.40.0" - resolved "https://registry.yarnpkg.com/awesome-phonenumber/-/awesome-phonenumber-2.40.0.tgz#02963b7a1a372572e42fb827401eb78d9ac4e294" - integrity sha512-ViGH+iNPzFCQ1s0Lut8jQC2dP05SjnntlA10emT8ANXy3UtyWykeWvKaDG17eWIcSjXqPvdKkPCco9Nu+Egj1g== - aws-sdk@^2.414.0: version "2.502.0" resolved "https://registry.yarnpkg.com/aws-sdk/-/aws-sdk-2.502.0.tgz#b3192f82389db982605462c8394cc3fa8c475b3e" @@ -5016,6 +5011,11 @@ cosmiconfig@^6.0.0: path-type "^4.0.0" yaml "^1.7.2" +country-flag-icons@^1.0.2: + version "1.2.5" + resolved "https://registry.yarnpkg.com/country-flag-icons/-/country-flag-icons-1.2.5.tgz#784185503f3589e650b30402c93ef7cc2a3225a9" + integrity sha512-5V7GEpGLG+uyLUf0qs35Ub80/Nnjtymfax7wwv7DMJFeA9PZWVYIck7OAuBP2FSL8Xxaqm0qMNmpufVyHGEHlw== + create-ecdh@^4.0.0: version "4.0.3" resolved "https://registry.yarnpkg.com/create-ecdh/-/create-ecdh-4.0.3.tgz#c9111b6f33045c4697f144787f9254cdc77c45ff" @@ -8065,6 +8065,13 @@ inline-source-map@~0.6.0: dependencies: source-map "~0.5.3" +input-format@^0.3.0: + version "0.3.0" + resolved "https://registry.yarnpkg.com/input-format/-/input-format-0.3.0.tgz#f662b1667b6d067769f44c717bcfcc92554bee75" + integrity sha512-7ipaXJ5Hnd2o62IxLRTCMcl7AOPzxU2PTfDunPDIaaAjq+Q8DcjI4/nmPo3fXJUvdTCX97BlW8d+7ArUhVTxAA== + dependencies: + prop-types "^15.7.2" + inquirer@6.2.0: version "6.2.0" resolved "https://registry.yarnpkg.com/inquirer/-/inquirer-6.2.0.tgz#51adcd776f661369dc1e894859c2560a224abdd8" @@ -9570,6 +9577,14 @@ libnpx@^10.2.2: y18n "^4.0.0" yargs "^11.0.0" +libphonenumber-js@^1.8.1: + version "1.8.4" + resolved "https://registry.yarnpkg.com/libphonenumber-js/-/libphonenumber-js-1.8.4.tgz#e84eaa25b96bfebb2ac1a5c847b7f7f17da5bd59" + integrity sha512-s0fTPZRB4hcsfDL9p6wUNOLngVh4y3fBPhH33dL7OfkHA2RirI8p3rlR+4f4SMxdcng9jPNOweS2Z47BivHMYw== + dependencies: + minimist "^1.2.5" + xml2js "^0.4.17" + lines-and-columns@^1.1.6: version "1.1.6" resolved "https://registry.yarnpkg.com/lines-and-columns/-/lines-and-columns-1.1.6.tgz#1c00c743b433cd0a4e80758f7b64a57440d9ff00" @@ -13431,6 +13446,17 @@ react-overlays@^0.8.0: react-transition-group "^2.2.0" warning "^3.0.0" +react-phone-number-input@^3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/react-phone-number-input/-/react-phone-number-input-3.1.0.tgz#55e153945d8a3c3294c3469e3c58dfdfc2984961" + integrity sha512-xJQXmpRtpVmwOM59JNUj8iOOBCRTM6VvV+5PAek+H/R6D+TTKO2QSelu9WMK+C18h17RznHe77QXlBx1Q3S6NA== + dependencies: + classnames "^2.2.5" + country-flag-icons "^1.0.2" + input-format "^0.3.0" + libphonenumber-js "^1.8.1" + prop-types "^15.7.2" + react-portal@^3.0.0: version "3.2.0" resolved "https://registry.yarnpkg.com/react-portal/-/react-portal-3.2.0.tgz#4224e19b2b05d5cbe730a7ba0e34ec7585de0043" @@ -16924,6 +16950,19 @@ xml2js@0.4.19: sax ">=0.6.0" xmlbuilder "~9.0.1" +xml2js@^0.4.17: + version "0.4.23" + resolved "https://registry.yarnpkg.com/xml2js/-/xml2js-0.4.23.tgz#a0c69516752421eb2ac758ee4d4ccf58843eac66" + integrity sha512-ySPiMjM0+pLDftHgXY4By0uswI3SPKLDw/i3UXbnO8M/p28zqexCUoPmQFrYD+/1BzhGJSs2i1ERWKJAtiLrug== + dependencies: + sax ">=0.6.0" + xmlbuilder "~11.0.0" + +xmlbuilder@~11.0.0: + version "11.0.1" + resolved "https://registry.yarnpkg.com/xmlbuilder/-/xmlbuilder-11.0.1.tgz#be9bae1c8a046e76b31127726347d0ad7002beb3" + integrity sha512-fDlsI/kFEx7gLvbecc0/ohLG50fugQp8ryHzMTuW9vSa1GJ0XYWKnhsUx7oie3G98+r56aTQIUB4kht42R3JvA== + xmlbuilder@~9.0.1: version "9.0.7" resolved "https://registry.yarnpkg.com/xmlbuilder/-/xmlbuilder-9.0.7.tgz#132ee63d2ec5565c557e20f4c22df9aca686b10d" From 2b7d992175f0cf225f284ad72a369613da656800 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 19 Oct 2020 19:34:44 -0400 Subject: [PATCH 35/55] refactor(NotificationPrefsPane): Use static text for email (same as verified phone) --- lib/components/user/notification-prefs-pane.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index d016eb7d9..92f37dc3e 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -111,6 +111,7 @@ class NotificationPrefsPane extends Component { } = this.props const { + email, isPhoneNumberVerified, notificationChannel, phoneNumber @@ -153,7 +154,7 @@ class NotificationPrefsPane extends Component { {notificationChannel === 'email' && ( Notification emails will be sent to: - + {email} )} {notificationChannel === 'sms' && ( From 31345eaad3aebbda5a36b0a2baeac1720d5e2751 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 20 Oct 2020 11:54:53 -0400 Subject: [PATCH 36/55] refactor(OtpReducer): Move default phone country to OTP reducer. --- example-config.yml | 15 +++------------ lib/components/user/notification-prefs-pane.js | 2 +- lib/components/user/user-account-screen.js | 8 +------- lib/reducers/create-otp-reducer.js | 8 ++++++++ 4 files changed, 13 insertions(+), 20 deletions(-) diff --git a/example-config.yml b/example-config.yml index 9bec2cf35..85722e6d8 100644 --- a/example-config.yml +++ b/example-config.yml @@ -141,16 +141,7 @@ itinerary: # - WALK # - BICYCLE -### If using OTP Middleware, you can define the optional phone number formats -### below for the target locale. +### If using OTP Middleware, you can define the optional phone number options below. # phoneFormatOptions: -# # The placeholder displayed in phone number controls -# inputPlaceholder: (555) 555-0123 -# # Region code for converting between raw E.164 numbers and national format. -# # (defaults to 'US') -# regionCode: US -# # awesome-phonenumber is a little bit too strict on validating formatted numbers -# # (although converting to raw E.164 number still works fine in such cases), -# # so we provide our own regex for validating user input instead. -# # For US numbers: https://www.oreilly.com/library/view/regular-expressions-cookbook/9781449327453/ch04s02.html -# validationRegExp: ^\(?([0-9]{3})\)?[-. ]?([0-9]{3})[-. ]?([0-9]{4})$ +# # ISO 2-letter country code for phone number formats (defaults to 'US') +# countryCode: US diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 92f37dc3e..bf61239c7 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -339,7 +339,7 @@ class PhoneNumberEditor extends Component { { - // Make phoneFormatOptions not null with default US region code if not specified. - const { phoneFormatOptions = {} } = state.otp.config - if (!phoneFormatOptions.regionCode) { - phoneFormatOptions.regionCode = 'US' - } - return { loggedInUser: state.user.loggedInUser, - phoneFormatOptions + phoneFormatOptions: state.otp.config.phoneFormatOptions } } diff --git a/lib/reducers/create-otp-reducer.js b/lib/reducers/create-otp-reducer.js index 22c5d2a3e..71a303a42 100644 --- a/lib/reducers/create-otp-reducer.js +++ b/lib/reducers/create-otp-reducer.js @@ -92,6 +92,14 @@ export function getInitialState (userDefinedConfig, initialQuery) { ) } + // Phone format options fall back to US region if not provided. + if (!config.phoneFormatOptions) { + config.phoneFormatOptions = {} + } + if (!config.phoneFormatOptions.countryCode) { + config.phoneFormatOptions.countryCode = 'US' + } + // Load user settings from local storage. // TODO: Make this work with settings fetched from alternative storage system // (e.g., OTP backend middleware containing user profile system). From f8425635b3583337624f116a7f81a602ca9d57f2 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 20 Oct 2020 14:57:51 -0400 Subject: [PATCH 37/55] test(otp-reducer): Update snapshot --- __tests__/reducers/__snapshots__/create-otp-reducer.js.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/__tests__/reducers/__snapshots__/create-otp-reducer.js.snap b/__tests__/reducers/__snapshots__/create-otp-reducer.js.snap index 93507dd74..704184c29 100644 --- a/__tests__/reducers/__snapshots__/create-otp-reducer.js.snap +++ b/__tests__/reducers/__snapshots__/create-otp-reducer.js.snap @@ -8,6 +8,9 @@ Object { "debouncePlanTimeMs": 0, "homeTimezone": "America/Los_Angeles", "language": Object {}, + "phoneFormatOptions": Object { + "countryCode": "US", + }, "realtimeEffectsDisplayThreshold": 120, "routingTypes": Array [], "stopViewer": Object { From 643ae432449eaecd2cf8985bd34542aad1bdeba3 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 20 Oct 2020 14:58:19 -0400 Subject: [PATCH 38/55] refactor: Address PR comments --- lib/actions/user.js | 18 +++++++++--------- lib/components/user/notification-prefs-pane.js | 6 ++++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index b0edbf187..727c8c771 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -6,8 +6,8 @@ import { isNewUser } from '../util/user' // Middleware API paths. const API_MONITORTRIP_PATH = '/api/secure/monitoredtrip' -const API_USER_PATH = '/api/secure/user' -const API_USER_VERIFYSMS_PATH = '/verify_sms' +const API_OTPUSER_PATH = '/api/secure/user' +const API_OTPUSER_VERIFYSMS_PATH = '/verify_sms' const setCurrentUser = createAction('SET_CURRENT_USER') const setCurrentUserMonitoredTrips = createAction('SET_CURRENT_USER_MONITORED_TRIPS') @@ -44,7 +44,7 @@ function getMiddlewareVariables (state) { loggedInUser } } else { - throw new Error('This action requires a valid middleware configuration.') + throw new Error('This action requires config.yml#persistence#otp_middleware to be defined.') } } @@ -60,7 +60,7 @@ export function fetchUserMonitoredTrips (accessToken) { const { data: trips, status } = await secureFetch(requestUrl, accessToken, apiKey, 'GET') if (status === 'success') { - dispatch(setCurrentUserMonitoredTrips(trips)) + dispatch(setCurrentUserMonitoredTrips(trips.data)) } } } @@ -73,7 +73,7 @@ export function fetchOrInitializeUser (auth) { return async function (dispatch, getState) { const { apiBaseUrl, apiKey } = getMiddlewareVariables(getState()) const { accessToken, user: authUser } = auth - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/fromtoken` + const requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/fromtoken` const { data: user, status } = await secureFetch(requestUrl, accessToken, apiKey) @@ -124,10 +124,10 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) { // Determine URL and method to use. if (isNewUser(loggedInUser)) { - requestUrl = `${apiBaseUrl}${API_USER_PATH}` + requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}` method = 'POST' } else if (id) { - requestUrl = `${apiBaseUrl}${API_USER_PATH}/${id}` + requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/${id}` method = 'PUT' } @@ -228,7 +228,7 @@ export function deleteUserMonitoredTrip (tripId) { export function requestPhoneVerificationSms (newPhoneNumber) { return async function (dispatch, getState) { const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${loggedInUser.id}${API_USER_VERIFYSMS_PATH}/${encodeURIComponent(newPhoneNumber)}` + const requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/${loggedInUser.id}${API_OTPUSER_VERIFYSMS_PATH}/${encodeURIComponent(newPhoneNumber)}` const sendSmsResult = await secureFetch(requestUrl, accessToken, apiKey, 'GET') @@ -248,7 +248,7 @@ export function requestPhoneVerificationSms (newPhoneNumber) { export function verifyPhoneNumber (code) { return async function (dispatch, getState) { const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) - const requestUrl = `${apiBaseUrl}${API_USER_PATH}/${loggedInUser.id}${API_USER_VERIFYSMS_PATH}/${code}` + const requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/${loggedInUser.id}${API_OTPUSER_VERIFYSMS_PATH}/${code}` const sendResult = await secureFetch(requestUrl, accessToken, apiKey, 'POST') diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index bf61239c7..a73535ff0 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -135,6 +135,8 @@ class NotificationPrefsPane extends Component { defaultValue={notificationChannel} > {allowedNotificationChannels.map(({ type, text }, index) => ( + // FIXME: If removing the Save/Cancel buttons on the account screen, + // persist changes immediately when onChange is triggered. {isEditing ? ( + // FIXME: If removing the Save/Cancel buttons on the account screen, + // make this a
and remove onKeyDown handler. Enter your phone number for SMS notifications: @@ -377,6 +381,8 @@ class PhoneNumberEditor extends Component { )} {isPending && ( + // FIXME: If removing the Save/Cancel buttons on the account screen, + // make this a and remove onKeyDown handler.

Please check the SMS messaging app on your mobile phone From 2c4705c66277b87b6475ec2f895e22c41963e5e2 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 20 Oct 2020 18:18:25 -0400 Subject: [PATCH 39/55] refactor(PhoneNumberEditor): Exit edit mode if user enters same number as current. --- .../user/notification-prefs-pane.js | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index a73535ff0..e40c3eaf4 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -263,28 +263,38 @@ class PhoneNumberEditor extends Component { _handleRequestCode = async () => { // Send phone verification request with the entered values. const { onRequestCode, setFieldValue, setFieldTouched } = this.props - const { newPhoneNumber } = this.state + const { currentNumber, currentNumberVerified, newPhoneNumber } = this.state - // Empty the input box (and disable the Send text button). - this.setState({ - newPhoneNumber: '' - }) - - const result = await onRequestCode(newPhoneNumber) - - if (result && result.phoneNumber === newPhoneNumber && !result.isPhoneNumberVerified) { - // On success (phoneNumber is updated and verified status is false), update the phone number in component state. + // If user enters the same phone number as their current verified number, + // just cancel editing without sending a verification SMS. + if (newPhoneNumber === currentNumber && currentNumberVerified) { this.setState({ - newPhoneNumber: result.phoneNumber + isEditing: false, + isPending: false, + newPhoneNumber: '' }) - - // Set empty code field in Formik's state and prompt for code. - setFieldValue('validationCode', '') - setFieldTouched('validationCode', false) + } else { + // Empty the input box (and disable the Send text button). this.setState({ - isEditing: false, - isPending: true + newPhoneNumber: '' }) + + const result = await onRequestCode(newPhoneNumber) + + if (result && result.phoneNumber === newPhoneNumber && !result.isPhoneNumberVerified) { + // On success (phoneNumber is updated and verified status is false), update the phone number in component state. + this.setState({ + newPhoneNumber: result.phoneNumber + }) + + // Set empty code field in Formik's state and prompt for code. + setFieldValue('validationCode', '') + setFieldTouched('validationCode', false) + this.setState({ + isEditing: false, + isPending: true + }) + } } } @@ -328,8 +338,9 @@ class PhoneNumberEditor extends Component { // => red error. // - Typing a phone number that doesn't match the configured phoneNumberRegEx // => red error. - const isPhoneInvalid = !isBlank(newPhoneNumber) && !isPossiblePhoneNumber(newPhoneNumber) - const phoneErrorState = isPhoneInvalid ? 'error' : null + const isPhoneInvalid = !isPossiblePhoneNumber(newPhoneNumber) + const showPhoneError = isPhoneInvalid && !isBlank(newPhoneNumber) + const phoneErrorState = showPhoneError ? 'error' : null const codeErrorState = getErrorState(this.props, 'validationCode') return ( @@ -360,7 +371,7 @@ class PhoneNumberEditor extends Component { { // Show cancel button only if a phone number is already recorded. currentNumber && } - {isPhoneInvalid && {INVALID_PHONE_MSG}} + {showPhoneError && {INVALID_PHONE_MSG}} ) : ( From 8e093486edbcd81d9f4adfdf973dbe0babe03fac Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 20 Oct 2020 19:27:15 -0400 Subject: [PATCH 40/55] refactor(UserAccountScreen): Handle SMS request error. --- .../user/notification-prefs-pane.js | 9 ++++----- lib/components/user/user-account-screen.js | 19 +++++++------------ 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index e40c3eaf4..289a7ef76 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -268,11 +268,7 @@ class PhoneNumberEditor extends Component { // If user enters the same phone number as their current verified number, // just cancel editing without sending a verification SMS. if (newPhoneNumber === currentNumber && currentNumberVerified) { - this.setState({ - isEditing: false, - isPending: false, - newPhoneNumber: '' - }) + this._handleCancelEditNumber() } else { // Empty the input box (and disable the Send text button). this.setState({ @@ -294,6 +290,9 @@ class PhoneNumberEditor extends Component { isEditing: false, isPending: true }) + } else { + // On failure, just exit editing state. + this._handleCancelEditNumber() } } } diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index a3a85871d..b2526cf99 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -127,23 +127,18 @@ class UserAccountScreen extends Component { // TODO: Should throttling be handled in the middleware? if (lastPhoneNumberRequested !== numberToVerify || (lastPhoneRequestTime && lastPhoneRequestTime <= timestamp + 60000)) { - await this.props.requestPhoneVerificationSms(numberToVerify) - - // (props have been updated are set at this point.) - const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser this.setState({ - lastPhoneNumberRequested: phoneNumber, + lastPhoneNumberRequested: numberToVerify, lastPhoneRequestTime: timestamp }) - - // When the pending phone number is populated by the backend, - // this means the phone number verification SMS has been sent. - if (!isPhoneNumberVerified) { - return { isPhoneNumberVerified, phoneNumber } - } + await this.props.requestPhoneVerificationSms(numberToVerify) } - return null + // Return the phone number and state from state.user.loggedInUser. + // If the SMS request was successful, the pending phone number and unverified status + // will be reflected in the state, otherwise they will be unchanged. + const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser + return { isPhoneNumberVerified, phoneNumber } } /** From d078e927d6bf0e7398e8aac3a01c5d4750f19c76 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 21 Oct 2020 10:52:05 -0400 Subject: [PATCH 41/55] refactor(PhoneNumberEditor): Refactor, keep editing after SMS req error and no current number. --- lib/components/user/notification-prefs-pane.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 289a7ef76..d9c76d936 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -275,12 +275,12 @@ class PhoneNumberEditor extends Component { newPhoneNumber: '' }) - const result = await onRequestCode(newPhoneNumber) + const { isPhoneNumberVerified, phoneNumber } = await onRequestCode(newPhoneNumber) - if (result && result.phoneNumber === newPhoneNumber && !result.isPhoneNumberVerified) { + if (phoneNumber === newPhoneNumber && !isPhoneNumberVerified) { // On success (phoneNumber is updated and verified status is false), update the phone number in component state. this.setState({ - newPhoneNumber: result.phoneNumber + newPhoneNumber: phoneNumber }) // Set empty code field in Formik's state and prompt for code. @@ -290,8 +290,9 @@ class PhoneNumberEditor extends Component { isEditing: false, isPending: true }) - } else { - // On failure, just exit editing state. + } else if (currentNumber) { + // On failure, exit the editing state if there is a current number. + // (Stay in edit mode if there is no current number.) this._handleCancelEditNumber() } } From 4d5fcc84620e4da38fbf924a2e36da37a46e144a Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 21 Oct 2020 11:04:18 -0400 Subject: [PATCH 42/55] refactor(PhoneNumberEditor): Refactor more --- .../user/notification-prefs-pane.js | 5 +++-- lib/components/user/user-account-screen.js | 19 +++++++------------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index d9c76d936..6fc7b6191 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -307,11 +307,12 @@ class PhoneNumberEditor extends Component { setFieldValue('validationCode', '') const result = await onSubmitCode(validationCode) + const { isPhoneNumberVerified, phoneNumber } = result - if (result && result.isPhoneNumberVerified) { + if (isPhoneNumberVerified) { // On success (status is changed to verified), update the phone number in component state and hide the code field. this.setState({ - currentNumber: result.phoneNumber, + currentNumber: phoneNumber, currentNumberVerified: true, isPending: false, newPhoneNumber: '' diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index b2526cf99..bda5457a7 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -113,10 +113,8 @@ class UserAccountScreen extends Component { /** * Requests a phone verification SMS to the specified phone number. - * This handler is called when the user clicks "Verify my phone" after entering a new number, - * and also when the user clicks "Request a new code" from the verification modal. - * - * @returns The pending phone number (plain and formatted) if the request succeeded, null otherwise. + * This handler is called when the user clicks "Verify my phone" after entering a new number. + * @returns The effective phone number and verification status. */ _handleRequestPhoneVerificationSms = async numberToVerify => { const { lastPhoneNumberRequested, lastPhoneRequestTime } = this.state @@ -143,19 +141,16 @@ class UserAccountScreen extends Component { /** * Sends the phone verification code. - * @returns The new phone number (plain and formatted) if the verification succeeded, null otherwise. + * @returns The effective phone number and verification status. */ _handleSendPhoneVerificationCode = async code => { await this.props.verifyPhoneNumber(code) - // When the pending phone number is erased by the backend, - // this means the phone number has been verified. + // Return the phone number and state from state.user.loggedInUser. + // If the verification was successful, isPhoneNumberVerified will be true, + // otherwise the phone information will remain unchanged. const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser - if (isPhoneNumberVerified) { - return { isPhoneNumberVerified, phoneNumber } - } - - return null + return { isPhoneNumberVerified, phoneNumber } } /** From a9dc87ae6cf80fe03b4ab246cf75473ba7484321 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Wed, 21 Oct 2020 11:04:59 -0400 Subject: [PATCH 43/55] refactor(actions/user): Remove unencoutered id checks. --- lib/actions/user.js | 90 ++++++++++++++++++++------------------------- 1 file changed, 39 insertions(+), 51 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index 727c8c771..22369f9af 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -126,30 +126,26 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) { if (isNewUser(loggedInUser)) { requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}` method = 'POST' - } else if (id) { + } else { requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/${id}` method = 'PUT' } - if (requestUrl) { - const result = await secureFetch(requestUrl, accessToken, apiKey, method, { - body: JSON.stringify(userData) - }) - - // TODO: improve the UI feedback messages for this. - if (result.status === 'success' && result.data) { - if (!silentOnSuccess) { - alert('Your preferences have been saved.') - } + const { data, message, status } = await secureFetch(requestUrl, accessToken, apiKey, method, { + body: JSON.stringify(userData) + }) - // Update application state with the user entry as saved - // (as returned) by the middleware. - dispatch(setCurrentUser({ accessToken, user: result.data })) - } else { - alert(`An error was encountered:\n${JSON.stringify(result)}`) + // TODO: improve the UI feedback messages for this. + if (status === 'success' && data) { + if (!silentOnSuccess) { + alert('Your preferences have been saved.') } + + // Update application state with the user entry as saved + // (as returned) by the middleware. + dispatch(setCurrentUser({ accessToken, user: data })) } else { - alert('Corrupted state: User ID not available for exiting user.') + alert(`An error was encountered:\n${JSON.stringify(message)}`) } } } @@ -169,32 +165,28 @@ export function createOrUpdateUserMonitoredTrip (tripData, isNew, silentOnSucces if (isNew) { requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}` method = 'POST' - } else if (id) { + } else { requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${id}` method = 'PUT' } - if (requestUrl) { - const result = await secureFetch(requestUrl, accessToken, apiKey, method, { - body: JSON.stringify(tripData) - }) + const { data, message, status } = await secureFetch(requestUrl, accessToken, apiKey, method, { + body: JSON.stringify(tripData) + }) - // TODO: improve the UI feedback messages for this. - if (result.status === 'success' && result.data) { - if (!silentOnSuccess) { - alert('Your preferences have been saved.') - } + // TODO: improve the UI feedback messages for this. + if (status === 'success' && data) { + if (!silentOnSuccess) { + alert('Your preferences have been saved.') + } - // Reload user's monitored trips after add/update. - await dispatch(fetchUserMonitoredTrips(accessToken)) + // Reload user's monitored trips after add/update. + await dispatch(fetchUserMonitoredTrips(accessToken)) - // Finally, navigate to the saved trips page. - dispatch(routeTo('/savedtrips')) - } else { - alert(`An error was encountered:\n${JSON.stringify(result)}`) - } + // Finally, navigate to the saved trips page. + dispatch(routeTo('/savedtrips')) } else { - alert('Corrupted state: Trip ID not available for exiting trip.') + alert(`An error was encountered:\n${JSON.stringify(message)}`) } } } @@ -208,16 +200,12 @@ export function deleteUserMonitoredTrip (tripId) { const { accessToken, apiBaseUrl, apiKey } = getMiddlewareVariables(getState()) const requestUrl = `${apiBaseUrl}${API_MONITORTRIP_PATH}/${tripId}` - if (tripId) { - const deleteResult = await secureFetch(requestUrl, accessToken, apiKey, 'DELETE') - if (deleteResult.status === 'success') { - // Reload user's monitored trips after deletion. - await dispatch(fetchUserMonitoredTrips(accessToken)) - } else { - alert(`An error was encountered:\n${JSON.stringify(deleteResult)}`) - } + const { message, status } = await secureFetch(requestUrl, accessToken, apiKey, 'DELETE') + if (status === 'success') { + // Reload user's monitored trips after deletion. + await dispatch(fetchUserMonitoredTrips(accessToken)) } else { - alert('Corrupted state: Monitored Trip ID not available for exiting user.') + alert(`An error was encountered:\n${JSON.stringify(message)}`) } } } @@ -230,14 +218,14 @@ export function requestPhoneVerificationSms (newPhoneNumber) { const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) const requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/${loggedInUser.id}${API_OTPUSER_VERIFYSMS_PATH}/${encodeURIComponent(newPhoneNumber)}` - const sendSmsResult = await secureFetch(requestUrl, accessToken, apiKey, 'GET') + const { message, status } = await secureFetch(requestUrl, accessToken, apiKey, 'GET') - if (sendSmsResult.status === 'success') { + if (status === 'success') { // Refetch user and update application state with new phone number and verification status. // (This also refetches the user's monitored trip, and that's ok.) await dispatch(fetchOrInitializeUser({ accessToken })) } else { - alert(`An error was encountered:\n${JSON.stringify(sendSmsResult)}`) + alert(`An error was encountered:\n${JSON.stringify(message)}`) } } } @@ -250,11 +238,11 @@ export function verifyPhoneNumber (code) { const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) const requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/${loggedInUser.id}${API_OTPUSER_VERIFYSMS_PATH}/${code}` - const sendResult = await secureFetch(requestUrl, accessToken, apiKey, 'POST') + const { data, message, status } = await secureFetch(requestUrl, accessToken, apiKey, 'POST') // If the check is successful, status in the returned data will be "approved". - if (sendResult.status === 'success' && sendResult.data) { - if (sendResult.data.status === 'approved') { + if (status === 'success' && data) { + if (data.status === 'approved') { // Refetch user and update application state with new phone number and verification status. // (This also refetches the user's monitored trip, and that's ok.) await dispatch(fetchOrInitializeUser({ accessToken })) @@ -263,7 +251,7 @@ export function verifyPhoneNumber (code) { alert('You entered in invalid code. Please try again.') } } else { - alert(`Your phone could not be verified:\n${JSON.stringify(sendResult)}`) + alert(`Your phone could not be verified:\n${JSON.stringify(message)}`) } } } From aab453065a721588fb90219d5e9d99cbab0fb4d7 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Thu, 22 Oct 2020 16:11:58 -0400 Subject: [PATCH 44/55] fix(PhoneNumberEditor): Update pending number in Formik state. --- .../user/notification-prefs-pane.js | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 6fc7b6191..7d068df81 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -90,10 +90,11 @@ const codeValidationSchema = yup.object({ * User notification preferences pane. */ class NotificationPrefsPane extends Component { - _handleOnPhoneVerified = ({ isPhoneNumberVerified, phoneNumber }) => { + _handleOnPhoneUpdated = ({ isPhoneNumberVerified, phoneNumber }) => { const { setFieldValue } = this.props - // Update the parent's Formik state (so when saving, it sends the updated numbers) + // Update the parent's Formik state, so that the updated numbers are rendered, + // even when changing the notification channel back and forth. setFieldValue('isPhoneNumberVerified', isPhoneNumberVerified) setFieldValue('phoneNumber', phoneNumber) } @@ -175,9 +176,10 @@ class NotificationPrefsPane extends Component { {...innerProps} initialPhoneNumber={phoneNumber} initialPhoneNumberVerified={isPhoneNumberVerified} + onCodeRequested={this._handleOnPhoneUpdated} onRequestCode={onRequestPhoneVerificationCode} onSubmitCode={onSendPhoneVerificationCode} - onVerified={this._handleOnPhoneVerified} + onVerified={this._handleOnPhoneUpdated} phoneFormatOptions={phoneFormatOptions} /> ) @@ -262,7 +264,7 @@ class PhoneNumberEditor extends Component { _handleRequestCode = async () => { // Send phone verification request with the entered values. - const { onRequestCode, setFieldValue, setFieldTouched } = this.props + const { onCodeRequested, onRequestCode, setFieldValue, setFieldTouched } = this.props const { currentNumber, currentNumberVerified, newPhoneNumber } = this.state // If user enters the same phone number as their current verified number, @@ -275,7 +277,8 @@ class PhoneNumberEditor extends Component { newPhoneNumber: '' }) - const { isPhoneNumberVerified, phoneNumber } = await onRequestCode(newPhoneNumber) + const result = await onRequestCode(newPhoneNumber) + const { isPhoneNumberVerified, phoneNumber } = result if (phoneNumber === newPhoneNumber && !isPhoneNumberVerified) { // On success (phoneNumber is updated and verified status is false), update the phone number in component state. @@ -290,6 +293,8 @@ class PhoneNumberEditor extends Component { isEditing: false, isPending: true }) + + onCodeRequested(result) } else if (currentNumber) { // On failure, exit the editing state if there is a current number. // (Stay in edit mode if there is no current number.) @@ -380,11 +385,13 @@ class PhoneNumberEditor extends Component { SMS notifications will be sent to: - {isPending + {formatPhoneNumber(isPending ? newPhoneNumber : currentNumber)} + {' '} + {isPending || !currentNumberVerified // eslint-disable-next-line jsx-a11y/label-has-for - ? <>{formatPhoneNumber(newPhoneNumber)} + ? // eslint-disable-next-line jsx-a11y/label-has-for - : <>{formatPhoneNumber(currentNumber)} {currentNumberVerified && } + : } @@ -403,9 +410,11 @@ class PhoneNumberEditor extends Component { Verification code: { // Show cancel button only if a phone number is already recorded. - currentNumber && } + initialPhoneNumber && } {showPhoneError && {INVALID_PHONE_MSG}} @@ -385,9 +329,9 @@ class PhoneNumberEditor extends Component { SMS notifications will be sent to: - {formatPhoneNumber(isPending ? newPhoneNumber : currentNumber)} + {formatPhoneNumber(initialPhoneNumber)} {' '} - {isPending || !currentNumberVerified + {isPending // eslint-disable-next-line jsx-a11y/label-has-for ? // eslint-disable-next-line jsx-a11y/label-has-for @@ -399,7 +343,7 @@ class PhoneNumberEditor extends Component { )} - {isPending && ( + {isPending && !isEditing && ( // FIXME: If removing the Save/Cancel buttons on the account screen, // make this a and remove onKeyDown handler. @@ -410,7 +354,7 @@ class PhoneNumberEditor extends Component { Verification code: { - const { lastPhoneNumberRequested, lastPhoneRequestTime } = this.state - const timestamp = new Date() - - // Request a new verification code if we are requesting a different number. - // or enough time has ellapsed since the last request (1 minute?). - // TODO: Should throttling be handled in the middleware? - if (lastPhoneNumberRequested !== numberToVerify || - (lastPhoneRequestTime && lastPhoneRequestTime <= timestamp + 60000)) { - this.setState({ - lastPhoneNumberRequested: numberToVerify, - lastPhoneRequestTime: timestamp - }) - await this.props.requestPhoneVerificationSms(numberToVerify) - } - - // Return the phone number and state from state.user.loggedInUser. - // If the SMS request was successful, the pending phone number and unverified status - // will be reflected in the state, otherwise they will be unchanged. - const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser - return { isPhoneNumberVerified, phoneNumber } - } - - /** - * Sends the phone verification code. - * @returns The effective phone number and verification status. - */ - _handleSendPhoneVerificationCode = async code => { - await this.props.verifyPhoneNumber(code) + const newUserData = clone(this.props.loggedInUser) + newUserData.phoneNumber = '+15555550222' + newUserData.isPhoneNumberVerified = false + await this._updateUserPrefs(newUserData) - // Return the phone number and state from state.user.loggedInUser. - // If the verification was successful, isPhoneNumberVerified will be true, - // otherwise the phone information will remain unchanged. - const { isPhoneNumberVerified, phoneNumber } = this.props.loggedInUser - return { isPhoneNumberVerified, phoneNumber } + //await this.props.requestPhoneVerificationSms(numberToVerify) } /** @@ -173,7 +140,7 @@ class UserAccountScreen extends Component { // TODO: Update title bar during componentDidMount. render () { - const { auth, loggedInUser, phoneFormatOptions } = this.props + const { auth, loggedInUser, phoneFormatOptions, requestPhoneVerificationSms, verifyPhoneNumber } = this.props return (

@@ -213,10 +180,11 @@ class UserAccountScreen extends Component { formContents = ( diff --git a/lib/reducers/create-user-reducer.js b/lib/reducers/create-user-reducer.js index 2dc9afbb6..e7d578d32 100644 --- a/lib/reducers/create-user-reducer.js +++ b/lib/reducers/create-user-reducer.js @@ -2,7 +2,13 @@ import update from 'immutability-helper' // TODO: port user-specific code from the otp reducer. function createUserReducer () { - const initialState = {} + const initialState = { + lastPhoneSmsRequest: { + number: null, + status: null, + timestamp: new Date(0) + } + } return (state = initialState, action) => { switch (action.type) { From 3c4802751a090c2ad4e95a887b7497476a12ba22 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 26 Oct 2020 10:53:40 -0400 Subject: [PATCH 46/55] refactor(UserAccountScreen): Update Formik if new props are passed. --- lib/components/user/new-account-wizard.js | 12 +++++------ lib/components/user/user-account-screen.js | 23 +++++++--------------- 2 files changed, 12 insertions(+), 23 deletions(-) diff --git a/lib/components/user/new-account-wizard.js b/lib/components/user/new-account-wizard.js index 50604b8fa..3eb0e906b 100644 --- a/lib/components/user/new-account-wizard.js +++ b/lib/components/user/new-account-wizard.js @@ -6,18 +6,16 @@ import SequentialPaneDisplay from './sequential-pane-display' * This component is the new account wizard. */ class NewAccountWizard extends Component { - _handleCreateNewUser = async () => { + _handleCreateNewUser = () => { const { onCreate, // provided by UserAccountScreen - setFieldValue, // provided by Formik values: userData // provided by Formik } = this.props - const newId = await onCreate(userData) - - // After user is initially persisted and reloaded to the redux state, - // update the 'id' in the Formik state. - setFieldValue('id', newId) + // Create a user record only if an id is not assigned. + if (!userData.id) { + onCreate(userData) + } } render () { diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index e97a7ddbf..8c29e362f 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -100,10 +100,8 @@ class UserAccountScreen extends Component { * @param {*} userData The user data state to persist. * @returns The new user id the the caller can use. */ - _handleCreateNewUser = async userData => { - await this._updateUserPrefs(userData, true) - - return this.props.loggedInUser.id + _handleCreateNewUser = userData => { + this._updateUserPrefs(userData, true) } _handleExit = () => { @@ -111,15 +109,6 @@ class UserAccountScreen extends Component { this.props.routeTo('/') } - _handleRequestPhoneVerificationSms = async numberToVerify => { - const newUserData = clone(this.props.loggedInUser) - newUserData.phoneNumber = '+15555550222' - newUserData.isPhoneNumberVerified = false - await this._updateUserPrefs(newUserData) - - //await this.props.requestPhoneVerificationSms(numberToVerify) - } - /** * Save changes and return to the planner. * @param {*} userData The user edited state to be saved, provided by Formik. @@ -147,12 +136,14 @@ class UserAccountScreen extends Component { {/* TODO: Do mobile view. */} { // Formik props provide access to the current user data state and errors, @@ -183,7 +174,7 @@ class UserAccountScreen extends Component { loggedInUser={loggedInUser} onCancel={this._handleExit} onCreate={this._handleCreateNewUser} - onRequestPhoneVerificationCode={this._handleRequestPhoneVerificationSms} + onRequestPhoneVerificationCode={requestPhoneVerificationSms} onSendPhoneVerificationCode={verifyPhoneNumber} panes={this._panes} phoneFormatOptions={phoneFormatOptions} From b1d3d7c06753a056c14bdee4c5289b222462a7e6 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Mon, 26 Oct 2020 11:49:15 -0400 Subject: [PATCH 47/55] refactor: Perofrm light refactor. --- lib/components/user/user-account-screen.js | 4 ++-- lib/util/ui.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/components/user/user-account-screen.js b/lib/components/user/user-account-screen.js index 8c29e362f..d11bcc001 100644 --- a/lib/components/user/user-account-screen.js +++ b/lib/components/user/user-account-screen.js @@ -151,7 +151,7 @@ class UserAccountScreen extends Component { // and to its own blur/change/submit event handlers that automate the state. // We pass the Formik props below to the components rendered so that individual controls // can be wired to be managed by Formik. - props => { + formikProps => { let formContents let DisplayComponent if (this.state.isNewUser) { @@ -170,7 +170,7 @@ class UserAccountScreen extends Component { if (DisplayComponent) { formContents = ( Date: Tue, 27 Oct 2020 15:05:12 -0400 Subject: [PATCH 48/55] refactor(actions/user): Address throttle and expired code alerts. --- lib/actions/user.js | 16 +++++++++++----- lib/components/user/notification-prefs-pane.js | 4 ++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index c38864904..f5fec14c8 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -242,6 +242,10 @@ export function requestPhoneVerificationSms (newPhoneNumber) { } else { alert(`An error was encountered:\n${JSON.stringify(message)}`) } + } else { + // Alert user if they have been throttled. + // TODO: improve this alert. + alert('A verification SMS was sent to the indicated phone number less than a minute ago. Please try again in a few moments.') } } } @@ -254,20 +258,22 @@ export function verifyPhoneNumber (code) { const { accessToken, apiBaseUrl, apiKey, loggedInUser } = getMiddlewareVariables(getState()) const requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}/${loggedInUser.id}${API_OTPUSER_VERIFYSMS_PATH}/${code}` - const { data, message, status } = await secureFetch(requestUrl, accessToken, apiKey, 'POST') + const { data, status } = await secureFetch(requestUrl, accessToken, apiKey, 'POST') // If the check is successful, status in the returned data will be "approved". if (status === 'success' && data) { if (data.status === 'approved') { // Refetch user and update application state with new phone number and verification status. // (This also refetches the user's monitored trip, and that's ok.) - await dispatch(fetchOrInitializeUser({ accessToken })) + dispatch(fetchOrInitializeUser({ accessToken })) } else { - // 'invalid' captures cases when users enter wrong/incorrect codes or expired codes. - alert('You entered in invalid code. Please try again.') + // Otherwise, the user entered a wrong/incorrect code. + alert('The code you entered is invalid. Please try again.') } } else { - alert(`Your phone could not be verified:\n${JSON.stringify(message)}`) + // This happens when an error occurs on backend side, especially + // when the code has expired (or was cancelled by Twilio after too many attempts). + alert(`Your phone could not be verified. Perhaps the code you entered has expired. Please request a new code and try again.`) } } } diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 98112bd04..a15999b12 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -246,8 +246,8 @@ class PhoneNumberEditor extends Component { this._handleCancelEditNumber() - // Send the sms request, unless the user - // entered the same phone number as their current verified number. + // Send the sms request, unless the user entered + // the same phone number as their current verified number. if (!(newPhoneNumber === initialPhoneNumber && initialPhoneNumberVerified)) { onRequestCode(newPhoneNumber) } From 705919a63981a453c4eceb10efd5211c547d19c1 Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 27 Oct 2020 15:48:36 -0400 Subject: [PATCH 49/55] fix(user reducer): Add handling code for last SMS request. --- lib/actions/user.js | 4 +-- .../user/notification-prefs-pane.js | 26 ++++++++++++++----- lib/reducers/create-user-reducer.js | 6 +++++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/lib/actions/user.js b/lib/actions/user.js index f5fec14c8..8adcdd454 100644 --- a/lib/actions/user.js +++ b/lib/actions/user.js @@ -11,7 +11,7 @@ const API_OTPUSER_VERIFYSMS_PATH = '/verify_sms' const setCurrentUser = createAction('SET_CURRENT_USER') const setCurrentUserMonitoredTrips = createAction('SET_CURRENT_USER_MONITORED_TRIPS') -const setLastPhoneSmsRequestInfo = createAction('SET_LAST_PHONE_SMS_REQUEST_INFO') +const setLastPhoneSmsRequest = createAction('SET_LAST_PHONE_SMS_REQUEST') export const setPathBeforeSignIn = createAction('SET_PATH_BEFORE_SIGNIN') function createNewUser (auth0User) { @@ -229,7 +229,7 @@ export function requestPhoneVerificationSms (newPhoneNumber) { const { message, status } = await secureFetch(requestUrl, accessToken, apiKey, 'GET') - dispatch(setLastPhoneSmsRequestInfo({ + dispatch(setLastPhoneSmsRequest({ number: newPhoneNumber, status, timestamp: now diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index a15999b12..7a83b0e9f 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -246,10 +246,20 @@ class PhoneNumberEditor extends Component { this._handleCancelEditNumber() - // Send the sms request, unless the user entered - // the same phone number as their current verified number. - if (!(newPhoneNumber === initialPhoneNumber && initialPhoneNumberVerified)) { - onRequestCode(newPhoneNumber) + // Send the SMS request if one of these conditions apply: + // - the user entered a phone number different than their current verified number, + // - the user clicks 'Request new code' for an already pending number + // (they could have refreshed the page in between). + let submittedNumber + + if (newPhoneNumber && !(newPhoneNumber === initialPhoneNumber && initialPhoneNumberVerified)) { + submittedNumber = newPhoneNumber + } else if (this._isPhoneNumberPending()) { + submittedNumber = initialPhoneNumber + } + + if (submittedNumber) { + onRequestCode(submittedNumber) } } @@ -260,6 +270,11 @@ class PhoneNumberEditor extends Component { onSubmitCode(validationCode) } + _isPhoneNumberPending = () => { + const { initialPhoneNumber, initialPhoneNumberVerified } = this.props + return !isBlank(initialPhoneNumber) && !initialPhoneNumberVerified + } + componentDidUpdate (prevProps) { // If new phone number and verified status are received, // then reset/clear the inputs. @@ -275,12 +290,11 @@ class PhoneNumberEditor extends Component { const { errors, // Formik prop initialPhoneNumber, - initialPhoneNumberVerified, phoneFormatOptions, touched // Formik prop } = this.props const { isEditing, newPhoneNumber } = this.state - const isPending = !isBlank(initialPhoneNumber) && !initialPhoneNumberVerified + const isPending = this._isPhoneNumberPending() // Here are the states we are dealing with: // - First time entering a phone number/validation code (blank value, not modified) diff --git a/lib/reducers/create-user-reducer.js b/lib/reducers/create-user-reducer.js index e7d578d32..952adc120 100644 --- a/lib/reducers/create-user-reducer.js +++ b/lib/reducers/create-user-reducer.js @@ -31,6 +31,12 @@ function createUserReducer () { }) } + case 'SET_LAST_PHONE_SMS_REQUEST': { + return update(state, { + lastPhoneSmsRequest: { $set: action.payload } + }) + } + default: return state } From 08e40f45b14042004da04e39b4807fb91e2bab8c Mon Sep 17 00:00:00 2001 From: binh-dam-ibigroup <56846598+binh-dam-ibigroup@users.noreply.github.com> Date: Tue, 27 Oct 2020 15:59:17 -0400 Subject: [PATCH 50/55] refactor(PhoneNumberEditor): Change code input type to 'text'. --- lib/components/user/notification-prefs-pane.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/components/user/notification-prefs-pane.js b/lib/components/user/notification-prefs-pane.js index 7a83b0e9f..46d8a6a62 100644 --- a/lib/components/user/notification-prefs-pane.js +++ b/lib/components/user/notification-prefs-pane.js @@ -368,11 +368,10 @@ class PhoneNumberEditor extends Component { Verification code: