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 1/3] 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 2/3] 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 3/3] 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) {