Merge pull request #308 from opentripplanner/refactor-account-pages

Refactor user account pages

Author: landonreed

Diff for: __tests__/components/viewers/__snapshots__/stop-viewer.js.snap

@@ -1,6 +1,7 @@
 .otp .navbar {
   border-radius: 0;
   background-color: #444;
+  margin: 0px;
 }
 
 .otp .navbar .container {

Diff for: example.css

@@ -8,6 +8,7 @@ import { createAction } from 'redux-actions'
 import { routingQuery } from './api'
 import { setQueryParam } from './form'
 import { routeTo } from './ui'
+import { TRIPS_PATH } from '../util/constants'
 import { secureFetch } from '../util/middleware'
 import { isNewUser } from '../util/user'
 
@@ -16,6 +17,7 @@ const API_MONITORED_TRIP_PATH = '/api/secure/monitoredtrip'
 const API_OTPUSER_PATH = '/api/secure/user'
 const API_OTPUSER_VERIFY_SMS_SUBPATH = '/verify_sms'
 
+const setAccessToken = createAction('SET_ACCESS_TOKEN')
 const setCurrentUser = createAction('SET_CURRENT_USER')
 const setCurrentUserMonitoredTrips = createAction('SET_CURRENT_USER_MONITORED_TRIPS')
 const setLastPhoneSmsRequest = createAction('SET_LAST_PHONE_SMS_REQUEST')
@@ -67,14 +69,30 @@ export function fetchAuth0Token (auth0) {
     try {
       const accessToken = await auth0.getAccessTokenSilently()
 
-      dispatch(setCurrentUser({ accessToken }))
+      dispatch(setAccessToken(accessToken))
     } catch (error) {
       // TODO: improve UI if there is an error.
       alert('Error obtaining an authorization token.')
     }
   }
 }
 
+/**
+ * Updates the redux state with the provided user data,
+ * and also fetches monitored trips if requested, i.e. when
+ * - initializing the user state with an existing persisted user, or
+ * - POST-ing a user for the first time.
+ */
+function setUser (user, fetchTrips) {
+  return function (dispatch, getState) {
+    dispatch(setCurrentUser(user))
+
+    if (fetchTrips) {
+      dispatch(fetchMonitoredTrips())
+    }
+  }
+}
+
 /**
  * Attempts to fetch user preferences (or set initial values if the user is being created)
  * into the redux state, under state.user.
@@ -110,12 +128,9 @@ export function fetchOrInitializeUser (auth0User) {
 
     const isNewAccount = status === 'error' || (user && user.result === 'ERR')
     const userData = isNewAccount ? createNewUser(auth0User) : user
-    dispatch(setCurrentUser({ accessToken, user: userData }))
 
-    // Also load monitored trips for existing users.
-    if (!isNewAccount) {
-      dispatch(fetchUserMonitoredTrips())
-    }
+    // Set uset in redux state. (Fetch trips for existing users.)
+    dispatch(setUser(userData, !isNewAccount))
   }
 }
 
@@ -132,7 +147,8 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) {
     let requestUrl, method
 
     // Determine URL and method to use.
-    if (isNewUser(loggedInUser)) {
+    const isCreatingUser = isNewUser(loggedInUser)
+    if (isCreatingUser) {
       requestUrl = `${apiBaseUrl}${API_OTPUSER_PATH}`
       method = 'POST'
     } else {
@@ -151,8 +167,8 @@ export function createOrUpdateUser (userData, silentOnSuccess = false) {
       }
 
       // Update application state with the user entry as saved
-      // (as returned) by the middleware.
-      dispatch(setCurrentUser({ accessToken, user: data }))
+      // (as returned) by the middleware. (Fetch trips if creating user.)
+      dispatch(setUser(data, isCreatingUser))
     } else {
       alert(`An error was encountered:\n${JSON.stringify(message)}`)
     }
@@ -182,7 +198,7 @@ export function resendVerificationEmail () {
  * We use the accessToken to fetch the data regardless of
  * whether the process to populate state.user is completed or not.
  */
-export function fetchUserMonitoredTrips () {
+export function fetchMonitoredTrips () {
   return async function (dispatch, getState) {
     const { accessToken, apiBaseUrl, apiKey } = getMiddlewareVariables(getState())
     const requestUrl = `${apiBaseUrl}${API_MONITORED_TRIP_PATH}`
@@ -230,12 +246,12 @@ export function createOrUpdateUserMonitoredTrip (
       }
 
       // Reload user's monitored trips after add/update.
-      await dispatch(fetchUserMonitoredTrips())
+      await dispatch(fetchMonitoredTrips())
 
       if (noRedirect) return
 
       // Finally, navigate to the saved trips page.
-      dispatch(routeTo('/savedtrips'))
+      dispatch(routeTo(TRIPS_PATH))
     } else {
       alert(`An error was encountered:\n${JSON.stringify(message)}`)
     }
@@ -281,7 +297,7 @@ export function confirmAndDeleteUserMonitoredTrip (tripId) {
     const { message, status } = await secureFetch(requestUrl, accessToken, apiKey, 'DELETE')
     if (status === 'success') {
       // Reload user's monitored trips after deletion.
-      dispatch(fetchUserMonitoredTrips())
+      dispatch(fetchMonitoredTrips())
     } else {
       alert(`An error was encountered:\n${JSON.stringify(message)}`)
     }

Diff for: lib/actions/user.js

@@ -373,7 +373,7 @@ class CallTakerAdvancedOptions extends Component {
 
   static contextType = ComponentContext
 
-  componentWillMount () {
+  componentDidMount () {
     // Fetch routes for banned/preferred routes selectors.
     this.props.findRoutes()
   }

Diff for: lib/components/app/call-taker-panel.js

@@ -6,7 +6,7 @@ import coreUtils from '@opentripplanner/core-utils'
 import PropTypes from 'prop-types'
 import React, { Component } from 'react'
 import { connect } from 'react-redux'
-import { Route, Switch, withRouter } from 'react-router'
+import { Redirect, Route, Switch, withRouter } from 'react-router'
 
 import PrintLayout from './print-layout'
 import * as authActions from '../../actions/auth'
@@ -17,7 +17,15 @@ import * as locationActions from '../../actions/location'
 import * as mapActions from '../../actions/map'
 import * as uiActions from '../../actions/ui'
 import { getAuth0Config } from '../../util/auth'
-import { AUTH0_AUDIENCE, AUTH0_SCOPE, URL_ROOT } from '../../util/constants'
+import {
+  ACCOUNT_PATH,
+  AUTH0_AUDIENCE,
+  AUTH0_SCOPE,
+  ACCOUNT_SETTINGS_PATH,
+  CREATE_ACCOUNT_PATH,
+  TRIPS_PATH,
+  URL_ROOT
+} from '../../util/constants'
 import { ComponentContext } from '../../util/contexts'
 import { getActiveItinerary, getTitle } from '../../util/state'
 import AfterSignInScreen from '../user/after-signin-screen'
@@ -137,12 +145,8 @@ class ResponsiveWebapp extends Component {
   }
 
   render () {
-    const { components, desktopView, mobileView } = this.props
-    return (
-      <ComponentContext.Provider value={components}>
-        {isMobile() ? mobileView : desktopView}
-      </ComponentContext.Provider>
-    )
+    const { desktopView, mobileView } = this.props
+    return isMobile() ? mobileView : desktopView
   }
 }
 
@@ -191,30 +195,21 @@ const WebappWithRouter = withRouter(
  * so that Auth0 services are available everywhere.
  */
 class RouterWrapperWithAuth0 extends Component {
-  /**
-   * Combine the router props with the other props that get
-   * passed to the exported component. This way, it is possible for
-   * the PrintLayout, UserAccountScreen, and other components to
-   * receive the LegIcon or other needed props.
-   */
-  _combineProps = routerProps => {
-    return { ...this.props, ...routerProps }
-  }
-
   render () {
     const {
       auth0Config,
+      components,
       processSignIn,
       routerConfig,
       showAccessTokenError,
       showLoginError
     } = this.props
 
     const router = (
-      <ConnectedRouter
-        basename={routerConfig && routerConfig.basename}
-        history={history}>
-        <div>
+      <ComponentContext.Provider value={components}>
+        <ConnectedRouter
+          basename={routerConfig && routerConfig.basename}
+          history={history}>
           <Switch>
             <Route
               exact
@@ -237,56 +232,40 @@ class RouterWrapperWithAuth0 extends Component {
               render={() => <WebappWithRouter {...this.props} />}
             />
             <Route
-              // This route lets new or existing users edit or set up their account.
-              path={'/account'}
-              component={routerProps => {
-                const props = this._combineProps(routerProps)
-                return <UserAccountScreen {...props} />
-              }}
-            />
-            <Route
-              path={'/savetrip'}
-              component={(routerProps) => {
-                const props = this._combineProps(routerProps)
-                return <SavedTripScreen isCreating {...props} />
-              }}
+              component={SavedTripScreen}
+              path={`${TRIPS_PATH}/:id`}
             />
+            <Route exact path={ACCOUNT_PATH}>
+              <Redirect to={TRIPS_PATH} />
+            </Route>
             <Route
-              path={'/savedtrips/:id'}
-              component={(routerProps) => {
-                const props = this._combineProps(routerProps)
-                return <SavedTripScreen {...props} />
-              }}
+              // This route lets new or existing users edit or set up their account.
+              component={UserAccountScreen}
+              path={[CREATE_ACCOUNT_PATH, ACCOUNT_SETTINGS_PATH]}
             />
             <Route
-              path={'/savedtrips'}
               component={SavedTripList}
+              path={TRIPS_PATH}
             />
             <Route
               // This route is called immediately after login by Auth0
               // and by the onRedirectCallback function from /lib/util/auth.js.
               // For new users, it displays the account setup form.
               // For existing users, it takes the browser back to the itinerary search prior to login.
-              path={'/signedin'}
-              component={routerProps => {
-                const props = this._combineProps(routerProps)
-                return <AfterSignInScreen {...props} />
-              }}
+              component={AfterSignInScreen}
+              path='/signedin'
             />
             <Route
+              component={PrintLayout}
               path='/print'
-              component={routerProps => {
-                const props = this._combineProps(routerProps)
-                return <PrintLayout {...props} />
-              }}
             />
             {/* For any other route, simply return the web app. */}
             <Route
               render={() => <WebappWithRouter {...this.props} />}
             />
           </Switch>
-        </div>
-      </ConnectedRouter>
+        </ConnectedRouter>
+      </ComponentContext.Provider>
     )
 
     return (

Diff for: lib/components/app/responsive-webapp.js

@@ -19,7 +19,7 @@ class DateTimePreview extends Component {
     date: PropTypes.string,
     departArrive: PropTypes.string,
     editButtonText: PropTypes.element,
-    hideButton: PropTypes.boolean,
+    hideButton: PropTypes.bool,
     onClick: PropTypes.func,
     routingType: PropTypes.string,
     time: PropTypes.string

Diff for: lib/components/form/date-time-preview.js

@@ -1,8 +1,9 @@
 import React from 'react'
 import { OverlayTrigger, Tooltip } from 'react-bootstrap'
 import { connect } from 'react-redux'
+import { LinkContainer } from 'react-router-bootstrap'
 
-import LinkButton from '../user/link-button'
+import { CREATE_TRIP_PATH } from '../../util/constants'
 import { itineraryCanBeMonitored } from '../../util/itinerary'
 import { getActiveItinerary } from '../../util/state'
 
@@ -13,7 +14,8 @@ import { getActiveItinerary } from '../../util/state'
 const SaveTripButton = ({
   itinerary,
   loggedInUser,
-  persistence
+  persistence,
+  queryParams
 }) => {
   // We are dealing with the following states:
   // 1. Persistence disabled => just return null
@@ -41,30 +43,35 @@ const SaveTripButton = ({
     buttonText = 'Save trip'
     icon = 'fa fa-plus-circle'
   }
-
+  const button = (
+    <button
+      // Apply pull-right style class so that the element is flush right,
+      // even with LineItineraries.
+      className='clear-button-formatting pull-right'
+      disabled={buttonDisabled}
+      style={buttonDisabled ? { pointerEvents: 'none' } : {}}
+    >
+      <i className={icon} /> {buttonText}
+    </button>
+  )
+  // Show tooltip with help text if button is disabled.
   if (buttonDisabled) {
-    const tooltip = <Tooltip id='disabled-save-tooltip'>{tooltipText}</Tooltip>
-
     return (
-      // Apply disabled bootstrap button styles to a non-input element
-      // to keep Tooltip and OverlayTrigger functional.
-      // Also apply pull-right style class so that the element is flush right, even with LineItineraries.
-      <OverlayTrigger placement='top' overlay={tooltip}>
-        <span className='btn disabled clear-button-formatting pull-right'>
-          <i className={icon} /> {buttonText}
-        </span>
+      <OverlayTrigger
+        overlay={<Tooltip id='disabled-save-tooltip'>{tooltipText}</Tooltip>}
+        placement='top'
+      >
+        <div style={{ cursor: 'not-allowed' }}>
+          {button}
+        </div>
       </OverlayTrigger>
     )
   }
 
   return (
-    <LinkButton
-      // Also apply pull-right style class so that the element is flush right, even with LineItineraries.
-      className='clear-button-formatting pull-right'
-      to='/savetrip'
-    >
-      <i className={icon} /> {buttonText}
-    </LinkButton>
+    <LinkContainer to={{ pathname: CREATE_TRIP_PATH, search: queryParams }}>
+      {button}
+    </LinkContainer>
   )
 }
 
@@ -75,7 +82,8 @@ const mapStateToProps = (state, ownProps) => {
   return {
     itinerary: getActiveItinerary(state.otp),
     loggedInUser: state.user.loggedInUser,
-    persistence
+    persistence,
+    queryParams: state.router.location.search
   }
 }