Various fixes from PR comments

6 files changed, 162 insertions, 119 deletions

diff --git a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js
index 1a399a464..842b34d2e 100644
--- a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js
+++ b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.component.js
@@ -1,11 +1,13 @@
 import React, { Component } from 'react'
 import PropTypes from 'prop-types'
 import ConfirmPageContainer, { ConfirmDetailRow } from '../../confirm-page-container'
-import { formatCurrency, getHexGasTotal } from '../../../helpers/confirm-transaction/util'
+import { formatCurrency } from '../../../helpers/confirm-transaction/util'
 import { isBalanceSufficient } from '../../send_/send.utils'
 import { DEFAULT_ROUTE } from '../../../routes'
-import { conversionGreaterThan } from '../../../conversion-util'
-import { MIN_GAS_LIMIT_DEC } from '../../send_/send.constants'
+import {
+  INSUFFICIENT_FUNDS_ERROR_KEY,
+  TRANSACTION_ERROR_KEY,
+} from '../../../constants/error-keys'
 
 export default class ConfirmTransactionBase extends Component {
   static contextTypes = {
@@ -13,57 +15,58 @@ export default class ConfirmTransactionBase extends Component {
   }
 
   static propTypes = {
+    // react-router props
     match: PropTypes.object,
     history: PropTypes.object,
     // Redux props
-    txData: PropTypes.object,
-    tokenData: PropTypes.object,
-    methodData: PropTypes.object,
-    tokenProps: PropTypes.object,
-    isTxReprice: PropTypes.bool,
-    nonce: PropTypes.string,
-    fromName: PropTypes.string,
-    fromAddress: PropTypes.string,
-    toName: PropTypes.string,
-    toAddress: PropTypes.string,
-    transactionStatus: PropTypes.string,
+    balance: PropTypes.string,
+    cancelTransaction: PropTypes.func,
+    clearConfirmTransaction: PropTypes.func,
+    clearSend: PropTypes.func,
+    conversionRate: PropTypes.number,
+    currentCurrency: PropTypes.string,
+    editTransaction: PropTypes.func,
     ethTransactionAmount: PropTypes.string,
     ethTransactionFee: PropTypes.string,
     ethTransactionTotal: PropTypes.string,
     fiatTransactionAmount: PropTypes.string,
     fiatTransactionFee: PropTypes.string,
     fiatTransactionTotal: PropTypes.string,
+    fromAddress: PropTypes.string,
+    fromName: PropTypes.string,
     hexGasTotal: PropTypes.string,
-    balance: PropTypes.string,
-    currentCurrency: PropTypes.string,
-    conversionRate: PropTypes.number,
-    clearConfirmTransaction: PropTypes.func,
-    cancelTransaction: PropTypes.func,
-    clearSend: PropTypes.func,
+    isTxReprice: PropTypes.bool,
+    methodData: PropTypes.object,
+    nonce: PropTypes.string,
     sendTransaction: PropTypes.func,
-    editTransaction: PropTypes.func,
     showCustomizeGasModal: PropTypes.func,
-    updateGasAndCalculate: PropTypes.func,
     showTransactionConfirmedModal: PropTypes.func,
+    toAddress: PropTypes.string,
+    tokenData: PropTypes.object,
+    tokenProps: PropTypes.object,
+    toName: PropTypes.string,
+    transactionStatus: PropTypes.string,
+    txData: PropTypes.object,
     // Component props
     action: PropTypes.string,
-    hideDetails: PropTypes.bool,
-    hideData: PropTypes.bool,
-    detailsComponent: PropTypes.node,
-    dataComponent: PropTypes.node,
-    summaryComponent: PropTypes.node,
     contentComponent: PropTypes.node,
-    title: PropTypes.string,
-    subtitle: PropTypes.string,
-    hideSubtitle: PropTypes.bool,
-    valid: PropTypes.bool,
+    dataComponent: PropTypes.node,
+    detailsComponent: PropTypes.node,
+    errorKey: PropTypes.string,
     errorMessage: PropTypes.string,
-    warning: PropTypes.string,
+    hideData: PropTypes.bool,
+    hideDetails: PropTypes.bool,
+    hideSubtitle: PropTypes.bool,
     identiconAddress: PropTypes.string,
+    onCancel: PropTypes.func,
     onEdit: PropTypes.func,
     onEditGas: PropTypes.func,
-    onCancel: PropTypes.func,
     onSubmit: PropTypes.func,
+    subtitle: PropTypes.string,
+    summaryComponent: PropTypes.node,
+    title: PropTypes.string,
+    valid: PropTypes.bool,
+    warning: PropTypes.string,
   }
 
   componentDidUpdate () {
@@ -86,9 +89,7 @@ export default class ConfirmTransactionBase extends Component {
     }
   }
 
-  getError () {
-    const INSUFFICIENT_FUNDS_ERROR = this.context.t('insufficientFunds')
-    const TRANSACTION_ERROR = this.context.t('transactionError')
+  getErrorKey () {
     const {
       balance,
       conversionRate,
@@ -111,68 +112,14 @@ export default class ConfirmTransactionBase extends Component {
     if (insufficientBalance) {
       return {
         valid: false,
-        errorMessage: INSUFFICIENT_FUNDS_ERROR,
+        errorKey: INSUFFICIENT_FUNDS_ERROR_KEY,
       }
     }
 
     if (simulationFails) {
       return {
         valid: false,
-        errorMessage: TRANSACTION_ERROR,
-      }
-    }
-
-    return {
-      valid: true,
-    }
-  }
-
-  validateEditGas ({ gasLimit, gasPrice }) {
-    const { t } = this.context
-    const {
-      balance,
-      conversionRate,
-      txData: {
-        txParams: {
-          value: amount,
-        } = {},
-      } = {},
-    } = this.props
-
-    const INSUFFICIENT_FUNDS_ERROR = t('insufficientFunds')
-    const GAS_LIMIT_TOO_LOW_ERROR = t('gasLimitTooLow')
-
-    const gasTotal = getHexGasTotal({ gasLimit, gasPrice })
-    const hasSufficientBalance = isBalanceSufficient({
-      amount,
-      gasTotal,
-      balance,
-      conversionRate,
-    })
-
-    if (!hasSufficientBalance) {
-      return {
-        valid: false,
-        errorMessage: INSUFFICIENT_FUNDS_ERROR,
-      }
-    }
-
-    const gasLimitTooLow = gasLimit && conversionGreaterThan(
-      {
-        value: MIN_GAS_LIMIT_DEC,
-        fromNumericBase: 'dec',
-        conversionRate,
-      },
-      {
-        value: gasLimit,
-        fromNumericBase: 'hex',
-      },
-    )
-
-    if (gasLimitTooLow) {
-      return {
-        valid: false,
-        errorMessage: GAS_LIMIT_TOO_LOW_ERROR,
+        errorKey: TRANSACTION_ERROR_KEY,
       }
     }
 
@@ -182,16 +129,12 @@ export default class ConfirmTransactionBase extends Component {
   }
 
   handleEditGas () {
-    const { onEditGas, showCustomizeGasModal, txData, updateGasAndCalculate } = this.props
+    const { onEditGas, showCustomizeGasModal } = this.props
 
     if (onEditGas) {
       onEditGas()
     } else {
-      showCustomizeGasModal({
-        txData,
-        onSubmit: txData => updateGasAndCalculate(txData),
-        validate: ({ gasLimit, gasPrice }) => this.validateEditGas({ gasLimit, gasPrice }),
-      })
+      showCustomizeGasModal()
     }
   }
 
@@ -328,7 +271,8 @@ export default class ConfirmTransactionBase extends Component {
       ethTransactionAmount,
       fiatTransactionAmount,
       valid: propsValid,
-      errorMessage: propsErrorMessage,
+      errorMessage,
+      errorKey: propsErrorKey,
       currentCurrency,
       action,
       title,
@@ -344,7 +288,7 @@ export default class ConfirmTransactionBase extends Component {
 
     const { name } = methodData
     const fiatConvertedAmount = formatCurrency(fiatTransactionAmount, currentCurrency)
-    const { valid, errorMessage } = this.getError()
+    const { valid, errorKey } = this.getErrorKey()
 
     return (
       <ConfirmPageContainer
@@ -363,7 +307,8 @@ export default class ConfirmTransactionBase extends Component {
         contentComponent={contentComponent}
         nonce={nonce}
         identiconAddress={identiconAddress}
-        errorMessage={propsErrorMessage || errorMessage}
+        errorMessage={errorMessage}
+        errorKey={propsErrorKey || errorKey}
         warning={warning}
         valid={propsValid || valid}
         onEdit={() => this.handleEdit()}
diff --git a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js
index 4f1fb86c1..31108bbd0 100644
--- a/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js
+++ b/ui/app/components/pages/confirm-transaction-base/confirm-transaction-base.container.js
@@ -8,6 +8,14 @@ import {
   updateGasAndCalculate,
 } from '../../../ducks/confirm-transaction.duck'
 import { clearSend, cancelTx, updateAndApproveTx, showModal } from '../../../actions'
+import {
+  INSUFFICIENT_FUNDS_ERROR_KEY,
+  GAS_LIMIT_TOO_LOW_ERROR_KEY,
+} from '../../../constants/error-keys'
+import { getHexGasTotal } from '../../../helpers/confirm-transaction/util'
+import { isBalanceSufficient } from '../../send_/send.utils'
+import { conversionGreaterThan } from '../../../conversion-util'
+import { MIN_GAS_LIMIT_DEC } from '../../send_/send.constants'
 
 const mapStateToProps = (state, props) => {
   const { toAddress: propsToAddress } = props
@@ -89,7 +97,73 @@ const mapDispatchToProps = dispatch => {
   }
 }
 
+const getValidateEditGas = ({ balance, conversionRate, txData }) => {
+  const { txParams: { value: amount } = {} } = txData
+
+  return ({ gasLimit, gasPrice }) => {
+    const gasTotal = getHexGasTotal({ gasLimit, gasPrice })
+    const hasSufficientBalance = isBalanceSufficient({
+      amount,
+      gasTotal,
+      balance,
+      conversionRate,
+    })
+
+    if (!hasSufficientBalance) {
+      return {
+        valid: false,
+        errorKey: INSUFFICIENT_FUNDS_ERROR_KEY,
+      }
+    }
+
+    const gasLimitTooLow = gasLimit && conversionGreaterThan(
+      {
+        value: MIN_GAS_LIMIT_DEC,
+        fromNumericBase: 'dec',
+        conversionRate,
+      },
+      {
+        value: gasLimit,
+        fromNumericBase: 'hex',
+      },
+    )
+
+    if (gasLimitTooLow) {
+      return {
+        valid: false,
+        errorKey: GAS_LIMIT_TOO_LOW_ERROR_KEY,
+      }
+    }
+
+    return {
+      valid: true,
+    }
+  }
+}
+
+const mergeProps = (stateProps, dispatchProps, ownProps) => {
+  const { balance, conversionRate, txData } = stateProps
+  const {
+    showCustomizeGasModal: dispatchShowCustomizeGasModal,
+    updateGasAndCalculate: dispatchUpdateGasAndCalculate,
+    ...otherDispatchProps
+  } = dispatchProps
+
+  const validateEditGas = getValidateEditGas({ balance, conversionRate, txData })
+
+  return {
+    ...stateProps,
+    ...otherDispatchProps,
+    ...ownProps,
+    showCustomizeGasModal: () => dispatchShowCustomizeGasModal({
+      txData,
+      onSubmit: txData => dispatchUpdateGasAndCalculate(txData),
+      validate: validateEditGas,
+    }),
+  }
+}
+
 export default compose(
   withRouter,
-  connect(mapStateToProps, mapDispatchToProps)
+  connect(mapStateToProps, mapDispatchToProps, mergeProps)
 )(ConfirmTransactionBase)
diff --git a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js
index db28d0bad..25259b98c 100644
--- a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js
+++ b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.component.js
@@ -16,19 +16,20 @@ import { TOKEN_METHOD_TRANSFER, TOKEN_METHOD_APPROVE } from './confirm-transacti
 
 export default class ConfirmTransactionSwitch extends Component {
   static propTypes = {
-    confirmTransaction: PropTypes.object,
+    txData: PropTypes.object,
+    methodData: PropTypes.object,
+    fetchingMethodData: PropTypes.bool,
   }
 
   redirectToTransaction () {
     const {
-      confirmTransaction: {
-        txData,
-        methodData: { name },
-        fetchingMethodData,
-      },
+      txData,
+      methodData: { name },
+      fetchingMethodData,
     } = this.props
     const { id } = txData
 
+
     if (isConfirmDeployContract(txData)) {
       const pathname = `${CONFIRM_TRANSACTION_ROUTE}/${id}${CONFIRM_DEPLOY_CONTRACT_PATH}`
       return <Redirect to={{ pathname }} />
@@ -62,7 +63,7 @@ export default class ConfirmTransactionSwitch extends Component {
   }
 
   render () {
-    const { confirmTransaction: { txData } } = this.props
+    const { txData } = this.props
 
     if (txData.txParams) {
       return this.redirectToTransaction()
diff --git a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js
index c04d388a9..3d7fc78cc 100644
--- a/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js
+++ b/ui/app/components/pages/confirm-transaction-switch/confirm-transaction-switch.container.js
@@ -2,10 +2,18 @@ import { connect } from 'react-redux'
 import ConfirmTransactionSwitch from './confirm-transaction-switch.component'
 
 const mapStateToProps = state => {
-  const { confirmTransaction } = state
+  const {
+    confirmTransaction: {
+      txData,
+      methodData,
+      fetchingMethodData,
+    },
+  } = state
 
   return {
-    confirmTransaction,
+    txData,
+    methodData,
+    fetchingMethodData,
   }
 }
 
diff --git a/ui/app/components/pages/confirm-transaction/confirm-transaction.component.js b/ui/app/components/pages/confirm-transaction/confirm-transaction.component.js
index 219f5b49a..874a89fd2 100644
--- a/ui/app/components/pages/confirm-transaction/confirm-transaction.component.js
+++ b/ui/app/components/pages/confirm-transaction/confirm-transaction.component.js
@@ -1,7 +1,6 @@
 import React, { Component } from 'react'
 import PropTypes from 'prop-types'
 import { Switch, Route } from 'react-router-dom'
-import R from 'ramda'
 import Loading from '../../loading-screen'
 import ConfirmTransactionSwitch from '../confirm-transaction-switch'
 import ConfirmTransactionBase from '../confirm-transaction-base'
@@ -30,6 +29,12 @@ export default class ConfirmTransaction extends Component {
     unconfirmedTransactions: PropTypes.array,
     setTransactionToConfirm: PropTypes.func,
     confirmTransaction: PropTypes.object,
+    clearConfirmTransaction: PropTypes.func,
+  }
+
+  getParamsTransactionId () {
+    const { match: { params: { id } = {} } } = this.props
+    return id || null
   }
 
   componentDidMount () {
@@ -52,13 +57,16 @@ export default class ConfirmTransaction extends Component {
 
   componentDidUpdate () {
     const {
-      match: { params: { id: paramsTransactionId } = {} },
       setTransactionToConfirm,
       confirmTransaction: { txData: { id: transactionId } = {} },
+      clearConfirmTransaction,
     } = this.props
+    const paramsTransactionId = this.getParamsTransactionId()
 
     if (paramsTransactionId && transactionId && paramsTransactionId !== transactionId + '') {
+      clearConfirmTransaction()
       setTransactionToConfirm(paramsTransactionId)
+      return
     }
 
     if (!transactionId) {
@@ -70,13 +78,13 @@ export default class ConfirmTransaction extends Component {
     const {
       history,
       unconfirmedTransactions,
-      match: { params: { id: paramsTransactionId } = {} },
       setTransactionToConfirm,
     } = this.props
+    const paramsTransactionId = this.getParamsTransactionId()
 
     if (paramsTransactionId) {
       // Check to make sure params ID is valid
-      const tx = R.find(({ id }) => id + '' === paramsTransactionId)(unconfirmedTransactions)
+      const tx = unconfirmedTransactions.find(({ id }) => id + '' === paramsTransactionId)
 
       if (!tx) {
         history.replace(DEFAULT_ROUTE)
@@ -96,8 +104,12 @@ export default class ConfirmTransaction extends Component {
 
   render () {
     const { confirmTransaction: { txData: { id } } = {} } = this.props
+    const paramsTransactionId = this.getParamsTransactionId()
 
-    return id
+    // Show routes when state.confirmTransaction has been set and when either the ID in the params
+    // isn't specified or is specified and matches the ID in state.confirmTransaction in order to
+    // support URLs of /confirm-transaction or /confirm-transaction/<transactionId>
+    return id && (!paramsTransactionId || paramsTransactionId === id + '')
       ? (
         <Switch>
           <Route
@@ -130,7 +142,7 @@ export default class ConfirmTransaction extends Component {
             path={`${CONFIRM_TRANSACTION_ROUTE}/:id?${SIGNATURE_REQUEST_PATH}`}
             component={ConfTx}
           />
-          <Route path={`${CONFIRM_TRANSACTION_ROUTE}/:id?`} component={ConfirmTransactionSwitch} />
+          <Route path="*" component={ConfirmTransactionSwitch} />
         </Switch>
       )
       : <Loading />
diff --git a/ui/app/components/pages/confirm-transaction/confirm-transaction.container.js b/ui/app/components/pages/confirm-transaction/confirm-transaction.container.js
index 1ad5aa355..1bc2f1efb 100644
--- a/ui/app/components/pages/confirm-transaction/confirm-transaction.container.js
+++ b/ui/app/components/pages/confirm-transaction/confirm-transaction.container.js
@@ -1,7 +1,10 @@
 import { connect } from 'react-redux'
 import { compose } from 'recompose'
 import { withRouter } from 'react-router-dom'
-import { setTransactionToConfirm } from '../../../ducks/confirm-transaction.duck'
+import {
+  setTransactionToConfirm,
+  clearConfirmTransaction,
+} from '../../../ducks/confirm-transaction.duck'
 import ConfirmTransaction from './confirm-transaction.component'
 import { getTotalUnapprovedCount } from '../../../selectors'
 import { unconfirmedTransactionsListSelector } from '../../../selectors/confirm-transaction'
@@ -20,7 +23,7 @@ const mapStateToProps = state => {
 const mapDispatchToProps = dispatch => {
   return {
     setTransactionToConfirm: transactionId => dispatch(setTransactionToConfirm(transactionId)),
-
+    clearConfirmTransaction: () => dispatch(clearConfirmTransaction()),
   }
 }