Merge pull request #316 from opentripplanner/refactor-account-pages-qbd

Fixes to Save Trip routing, check terms

Author: landonreed

Diff for: lib/actions/user.js

@@ -17,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')
@@ -68,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.
@@ -111,11 +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(fetchMonitoredTrips())
-    }
+
+    // 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,10 +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 }))
-      // Re-fetch user's monitored trips.
-      dispatch(fetchMonitoredTrips())
+      // (as returned) by the middleware. (Fetch trips if creating user.)
+      dispatch(setUser(data, isCreatingUser))
     } else {
       alert(`An error was encountered:\n${JSON.stringify(message)}`)
     }

Diff for: lib/components/narrative/save-trip-button.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
   }
 }
 

Diff for: lib/components/user/account-page.js

@@ -1,26 +1,68 @@
-import React from 'react'
+import { withAuthenticationRequired } from '@auth0/auth0-react'
+import React, { Component } from 'react'
 import { Col, Row } from 'react-bootstrap'
+import { connect } from 'react-redux'
 
+import * as uiActions from '../../actions/ui'
+import { CREATE_ACCOUNT_PATH } from '../../util/constants'
+import { RETURN_TO_CURRENT_ROUTE } from '../../util/ui'
+import withLoggedInUserSupport from './with-logged-in-user-support'
 import DesktopNav from '../app/desktop-nav'
 import SubNav from './sub-nav'
 
 /**
  * This component contains common navigation elements and wrappers and should
  * wrap any user account page (e.g., SavedTripList or account settings).
  */
-const AccountPage = ({children, subnav = true}) => (
-  <div className='otp'>
-    {/* TODO: Do mobile view. */}
-    <DesktopNav />
-    {subnav && <SubNav />}
-    <div className='container'>
-      <Row xs={12}>
-        <Col xs={12} sm={10} smOffset={1} md={8} mdOffset={2}>
-          {children}
-        </Col>
-      </Row>
-    </div>
-  </div>
-)
+class AccountPage extends Component {
+  componentDidMount () {
+    const { loggedInUser, routeTo } = this.props
+
+    if (!loggedInUser.hasConsentedToTerms) {
+      // If a user signed up in Auth0 and did not complete the New Account wizard
+      // make the user finish set up their accounts first.
+      // monitoredTrips should not be null otherwise.
+      // NOTE: This check applies to any route that makes use of this component.
+      routeTo(CREATE_ACCOUNT_PATH)
+    }
+  }
+
+  render () {
+    const {children, subnav = true} = this.props
+    return (
+      <div className='otp'>
+        {/* TODO: Do mobile view. */}
+        <DesktopNav />
+        {subnav && <SubNav />}
+        <div className='container'>
+          <Row xs={12}>
+            <Col xs={12} sm={10} smOffset={1} md={8} mdOffset={2}>
+              {children}
+            </Col>
+          </Row>
+        </div>
+      </div>
+    )
+  }
+}
+
+// connect to the redux store
 
-export default AccountPage
+const mapStateToProps = (state, ownProps) => {
+  return {
+    loggedInUser: state.user.loggedInUser,
+    trips: state.user.loggedInUserMonitoredTrips
+  }
+}
+
+const mapDispatchToProps = {
+  routeTo: uiActions.routeTo
+}
+
+export default withLoggedInUserSupport(
+  withAuthenticationRequired(
+    connect(mapStateToProps, mapDispatchToProps)(AccountPage),
+    RETURN_TO_CURRENT_ROUTE
+  ),
+  true
+)

Diff for: lib/components/user/link-button.js

@@ -58,6 +58,7 @@ class SavedTripList extends Component {
 
 const mapStateToProps = (state, ownProps) => {
   return {
+    loggedInUser: state.user.loggedInUser,
     trips: state.user.loggedInUserMonitoredTrips
   }
 }

Diff for: lib/components/user/monitored-trip/saved-trip-list.js

@@ -101,12 +101,11 @@ class SavedTripScreen extends Component {
     summary: TripSummaryPane
   }
 
-  async componentDidMount () {
+  componentDidMount () {
     const { isCreating, monitoredTrips } = this.props
-
-    // There is a middleware limit of 5 saved trips,
-    // so if that limit is already reached, alert, then show editing mode.
     if (isCreating && hasMaxTripCount(monitoredTrips)) {
+      // There is a middleware limit of 5 saved trips,
+      // so if that limit is already reached, alert, then show editing mode.
       alert('You already have reached the maximum of five saved trips.\n' +
           'Please remove unused trips from your saved trips, and try again.')
 

Diff for: lib/components/user/monitored-trip/saved-trip-screen.js

@@ -17,10 +17,14 @@ function createUserReducer () {
 
   return (state = initialState, action) => {
     switch (action.type) {
+      case 'SET_ACCESS_TOKEN': {
+        return update(state, {
+          accessToken: { $set: action.payload }
+        })
+      }
       case 'SET_CURRENT_USER': {
         return update(state, {
-          accessToken: { $set: action.payload.accessToken },
-          loggedInUser: { $set: action.payload.user }
+          loggedInUser: { $set: action.payload }
         })
       }
 

Diff for: lib/reducers/create-user-reducer.js

@@ -8,6 +8,7 @@ export const ACCOUNT_PATH = '/account'
 export const ACCOUNT_SETTINGS_PATH = `${ACCOUNT_PATH}/settings`
 export const TRIPS_PATH = `${ACCOUNT_PATH}/trips`
 export const CREATE_ACCOUNT_PATH = `${ACCOUNT_PATH}/create`
+export const CREATE_TRIP_PATH = `${TRIPS_PATH}/new`
 
 // Gets the root URL, e.g. https://otp-instance.example.com:8080, computed once for all.
 // TODO: support root URLs that involve paths or subfolders, as in https://otp-ui.example.com/path-to-ui/