I#5956 fix2 dont overwrite existing rpc settings (#6044)

* mm-controller - dont overwrite existing rpc settings

* ui-networkDropdown - dont pass old network as chainId

* add methods preferencesController.updateRpc and metamaskController.updateAndSetCustomRpc

* use updateAndSetCustomRpc in settings to allow rpcs to be updated

* use new rpc as nickname if no nick name has been supplied

* fix update rpc method

7 files changed, 85 insertions, 9 deletions

diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js
index e82a69da2..d2ef987f2 100644
--- a/app/scripts/controllers/preferences.js
+++ b/app/scripts/controllers/preferences.js
@@ -407,6 +407,32 @@ class PreferencesController {
   }
 
   /**
+   * updates custom RPC details
+   *
+   * @param {string} url The RPC url to add to frequentRpcList.
+   * @param {number} chainId Optional chainId of the selected network.
+   * @param {string} ticker   Optional ticker symbol of the selected network.
+   * @param {string} nickname Optional nickname of the selected network.
+   * @returns {Promise<array>} Promise resolving to updated frequentRpcList.
+   *
+   */
+
+
+  updateRpc (newRpcDetails) {
+    const rpcList = this.getFrequentRpcListDetail()
+    const index = rpcList.findIndex((element) => { return element.rpcUrl === newRpcDetails.rpcUrl })
+    if (index > -1) {
+      const rpcDetail = rpcList[index]
+      const updatedRpc = extend(rpcDetail, newRpcDetails)
+      rpcList[index] = updatedRpc
+      this.store.updateState({ frequentRpcListDetail: rpcList })
+    } else {
+      const { rpcUrl, chainId, ticker, nickname } = newRpcDetails
+      return this.addToFrequentRpcList(rpcUrl, chainId, ticker, nickname)
+    }
+    return Promise.resolve(rpcList)
+  }
+  /**
    * Adds custom RPC url to state.
    *
    * @param {string} url The RPC url to add to frequentRpcList.
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 4189bdd10..942c6a62e 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -414,6 +414,7 @@ module.exports = class MetamaskController extends EventEmitter {
       // network management
       setProviderType: nodeify(networkController.setProviderType, networkController),
       setCustomRpc: nodeify(this.setCustomRpc, this),
+      updateAndSetCustomRpc: nodeify(this.updateAndSetCustomRpc, this),
       delCustomRpc: nodeify(this.delCustomRpc, this),
 
       // PreferencesController
@@ -1503,6 +1504,21 @@ module.exports = class MetamaskController extends EventEmitter {
   }
 
   // network
+  /**
+   * A method for selecting a custom URL for an ethereum RPC provider and updating it
+   * @param {string} rpcUrl - A URL for a valid Ethereum RPC API.
+   * @param {number} chainId - The chainId of the selected network.
+   * @param {string} ticker - The ticker symbol of the selected network.
+   * @param {string} nickname - Optional nickname of the selected network.
+   * @returns {Promise<String>} - The RPC Target URL confirmed.
+   */
+
+  async updateAndSetCustomRpc (rpcUrl, chainId, ticker = 'ETH', nickname) {
+    await this.preferencesController.updateRpc({ rpcUrl, chainId, ticker, nickname })
+    this.networkController.setRpcTarget(rpcUrl, chainId, ticker, nickname)
+    return rpcUrl
+  }
+
 
   /**
    * A method for selecting a custom URL for an ethereum RPC provider.
@@ -1513,8 +1529,15 @@ module.exports = class MetamaskController extends EventEmitter {
    * @returns {Promise<String>} - The RPC Target URL confirmed.
    */
   async setCustomRpc (rpcTarget, chainId, ticker = 'ETH', nickname = '') {
-    this.networkController.setRpcTarget(rpcTarget, chainId, ticker, nickname)
-    await this.preferencesController.addToFrequentRpcList(rpcTarget, chainId, ticker, nickname)
+    const frequentRpcListDetail = this.preferencesController.getFrequentRpcListDetail()
+    const rpcSettings = frequentRpcListDetail.find((rpc) => rpcTarget === rpc.rpcUrl)
+
+    if (rpcSettings) {
+      this.networkController.setRpcTarget(rpcSettings.rpcUrl, rpcSettings.chainId, rpcSettings.ticker, rpcSettings.nickname)
+    } else {
+      this.networkController.setRpcTarget(rpcTarget, chainId, ticker, nickname)
+      await this.preferencesController.addToFrequentRpcList(rpcTarget, chainId, ticker, nickname)
+    }
     return rpcTarget
   }
 
diff --git a/test/unit/app/controllers/preferences-controller-test.js b/test/unit/app/controllers/preferences-controller-test.js
index 67d1875c1..558597ae7 100644
--- a/test/unit/app/controllers/preferences-controller-test.js
+++ b/test/unit/app/controllers/preferences-controller-test.js
@@ -511,6 +511,18 @@ describe('preferences controller', function () {
     })
   })
 
+  describe('#updateRpc', function () {
+    it('should update the rpcDetails properly', () => {
+      preferencesController.store.updateState({frequentRpcListDetail: [{}, { rpcUrl: 'test' }, {}]})
+      preferencesController.updateRpc({ rpcUrl: 'test', chainId: 1 })
+      preferencesController.updateRpc({ rpcUrl: 'test/1', chainId: 1 })
+      preferencesController.updateRpc({ rpcUrl: 'test/2', chainId: 1 })
+      preferencesController.updateRpc({ rpcUrl: 'test/3', chainId: 1 })
+      const list = preferencesController.getFrequentRpcListDetail()
+      assert.deepEqual(list[1], { rpcUrl: 'test', chainId: 1 })
+    })
+  })
+
   describe('on updateFrequentRpcList', function () {
     it('should add custom RPC url to state', function () {
       preferencesController.addToFrequentRpcList('rpc_url', 1)
diff --git a/ui/app/actions.js b/ui/app/actions.js
index c2622d0a0..2b073e915 100644
--- a/ui/app/actions.js
+++ b/ui/app/actions.js
@@ -242,6 +242,7 @@ var actions = {
   removeSuggestedTokens,
   addKnownMethodData,
   UPDATE_TOKENS: 'UPDATE_TOKENS',
+  updateAndSetCustomRpc: updateAndSetCustomRpc,
   setRpcTarget: setRpcTarget,
   delRpcTarget: delRpcTarget,
   setProviderType: setProviderType,
@@ -1971,10 +1972,26 @@ function setPreviousProvider (type) {
   }
 }
 
-function setRpcTarget (newRpc, chainId, ticker = 'ETH', nickname = '') {
+function updateAndSetCustomRpc (newRpc, chainId, ticker = 'ETH', nickname) {
+  return (dispatch) => {
+    log.debug(`background.updateAndSetCustomRpc: ${newRpc} ${chainId} ${ticker} ${nickname}`)
+    background.updateAndSetCustomRpc(newRpc, chainId, ticker, nickname || newRpc, (err, result) => {
+      if (err) {
+        log.error(err)
+        return dispatch(actions.displayWarning('Had a problem changing networks!'))
+      }
+      dispatch({
+        type: actions.SET_RPC_TARGET,
+        value: newRpc,
+      })
+    })
+  }
+}
+
+function setRpcTarget (newRpc, chainId, ticker = 'ETH', nickname) {
   return (dispatch) => {
     log.debug(`background.setRpcTarget: ${newRpc} ${chainId} ${ticker} ${nickname}`)
-    background.setCustomRpc(newRpc, chainId, ticker, nickname, (err, result) => {
+    background.setCustomRpc(newRpc, chainId, ticker, nickname || newRpc, (err, result) => {
       if (err) {
         log.error(err)
         return dispatch(actions.displayWarning('Had a problem changing networks!'))
diff --git a/ui/app/components/dropdowns/network-dropdown.js b/ui/app/components/dropdowns/network-dropdown.js
index d4cc695a6..6e002219a 100644
--- a/ui/app/components/dropdowns/network-dropdown.js
+++ b/ui/app/components/dropdowns/network-dropdown.js
@@ -277,7 +277,6 @@ NetworkDropdown.prototype.getNetworkName = function () {
 NetworkDropdown.prototype.renderCommonRpc = function (rpcListDetail, provider) {
   const props = this.props
   const reversedRpcListDetail = rpcListDetail.slice().reverse()
-  const network = props.network
 
   return reversedRpcListDetail.map((entry) => {
     const rpc = entry.rpcUrl
@@ -288,7 +287,7 @@ NetworkDropdown.prototype.renderCommonRpc = function (rpcListDetail, provider) {
     if ((rpc === 'http://localhost:8545') || currentRpcTarget) {
       return null
     } else {
-      const chainId = entry.chainId || network
+      const chainId = entry.chainId
       return h(
         DropdownMenuItem,
         {
diff --git a/ui/app/components/pages/settings/settings-tab/settings-tab.component.js b/ui/app/components/pages/settings/settings-tab/settings-tab.component.js
index 16afa85cb..8cd596423 100644
--- a/ui/app/components/pages/settings/settings-tab/settings-tab.component.js
+++ b/ui/app/components/pages/settings/settings-tab/settings-tab.component.js
@@ -230,7 +230,6 @@ export default class SettingsTab extends PureComponent {
 
   validateRpc (newRpc, chainId, ticker = 'ETH', nickname) {
     const { setRpcTarget, displayWarning } = this.props
-
     if (validUrl.isWebUri(newRpc)) {
       if (!!chainId && Number.isNaN(parseInt(chainId))) {
         return displayWarning(`${this.context.t('invalidInput')} chainId`)
diff --git a/ui/app/components/pages/settings/settings-tab/settings-tab.container.js b/ui/app/components/pages/settings/settings-tab/settings-tab.container.js
index b6c33a5b2..d5fd8416a 100644
--- a/ui/app/components/pages/settings/settings-tab/settings-tab.container.js
+++ b/ui/app/components/pages/settings/settings-tab/settings-tab.container.js
@@ -4,7 +4,7 @@ import { connect } from 'react-redux'
 import { withRouter } from 'react-router-dom'
 import {
   setCurrentCurrency,
-  setRpcTarget,
+  updateAndSetCustomRpc,
   displayWarning,
   revealSeedConfirmation,
   setUseBlockie,
@@ -50,7 +50,7 @@ const mapStateToProps = state => {
 const mapDispatchToProps = dispatch => {
   return {
     setCurrentCurrency: currency => dispatch(setCurrentCurrency(currency)),
-    setRpcTarget: (newRpc, chainId, ticker, nickname) => dispatch(setRpcTarget(newRpc, chainId, ticker, nickname)),
+    setRpcTarget: (newRpc, chainId, ticker, nickname) => dispatch(updateAndSetCustomRpc(newRpc, chainId, ticker, nickname)),
     displayWarning: warning => dispatch(displayWarning(warning)),
     revealSeedConfirmation: () => dispatch(revealSeedConfirmation()),
     setUseBlockie: value => dispatch(setUseBlockie(value)),