fix(actions/user): Do not persist user before sending sms req/after p…

…hone validation.
opentripplanner · Oct 8, 2020 · e8f9d39 · e8f9d39
1 parent 45a2057
commit e8f9d39
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 54 deletions.
diff --git 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,45 +191,22 @@ 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
 
     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
@@ -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
@@ -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')
 }