Attach hardware wallets already in addressbook (#4912)

* Attach hardware wallets already in addressbook

* Only set values changed

js/src/mobx/hardwareStore.js

@@ -120,20 +120,22 @@ export default class HardwareStore {
       });
   }
 
-  createAccountInfo (entry) {
+  createAccountInfo (entry, original = {}) {
     const { address, manufacturer, name } = entry;
 
     return Promise
       .all([
-        this._api.parity.setAccountName(address, name),
-        this._api.parity.setAccountMeta(address, {
+        original.name
+          ? Promise.resolve(true)
+          : this._api.parity.setAccountName(address, name),
+        this._api.parity.setAccountMeta(address, Object.assign({
           description: `${manufacturer} ${name}`,
           hardware: {
             manufacturer
           },
           tags: ['hardware'],
           timestamp: Date.now()
-        })
+        }, original.meta || {}))
       ])
       .catch((error) => {
         console.warn('HardwareStore::createEntry', error);

js/src/mobx/hardwareStore.spec.js

@@ -130,6 +130,7 @@ describe('mobx/HardwareStore', () => {
 
   describe('operations', () => {
     describe('createAccountInfo', () => {
+      describe('when not existing', () => {
         beforeEach(() => {
           return store.createAccountInfo({
             address: 'testAddr',
@@ -147,11 +148,43 @@ describe('mobx/HardwareStore', () => {
             description: 'testMfg testName',
             hardware: {
               manufacturer: 'testMfg'
-          }
+            },
+            tags: ['hardware']
           }));
         });
       });
 
+      describe('when already exists', () => {
+        beforeEach(() => {
+          return store.createAccountInfo({
+            address: 'testAddr',
+            manufacturer: 'testMfg',
+            name: 'testName'
+          }, {
+            name: 'originalName',
+            meta: {
+              description: 'originalDescription',
+              tags: ['tagA', 'tagB']
+            }
+          });
+        });
+
+        it('does not call into parity_setAccountName', () => {
+          expect(api.parity.setAccountName).not.to.have.been.called;
+        });
+
+        it('calls into parity_setAccountMeta', () => {
+          expect(api.parity.setAccountMeta).to.have.been.calledWith('testAddr', sinon.match({
+            description: 'originalDescription',
+            hardware: {
+              manufacturer: 'testMfg'
+            },
+            tags: ['tagA', 'tagB']
+          }));
+        });
+      });
+    });
+
     describe('scanLedger', () => {
       beforeEach(() => {
         return store.scanLedger();

js/src/views/Accounts/accounts.js

@@ -394,8 +394,12 @@ class Accounts extends Component {
 
     Object
       .keys(wallets)
-      .filter((address) => !accountsInfo[address])
-      .forEach((address) => this.hwstore.createAccountInfo(wallets[address]));
+      .filter((address) => {
+        const account = accountsInfo[address];
+
+        return !account || !account.meta || !account.meta.hardware;
+      })
+      .forEach((address) => this.hwstore.createAccountInfo(wallets[address], accountsInfo[address]));
 
     this.setVisibleAccounts();
   }

js/src/views/Accounts/accounts.spec.js

@@ -0,0 +1,122 @@
+// Copyright 2015-2017 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+import { shallow } from 'enzyme';
+import React from 'react';
+import sinon from 'sinon';
+
+import Accounts from './';
+
+let api;
+let component;
+let hwstore;
+let instance;
+let redux;
+
+function createApi () {
+  api = {};
+
+  return api;
+}
+
+function createHwStore (walletAddress = '0x456') {
+  hwstore = {
+    wallets: {
+      [walletAddress]: {
+        address: walletAddress
+      }
+    },
+    createAccountInfo: sinon.stub()
+  };
+
+  return hwstore;
+}
+
+function createRedux () {
+  redux = {
+    dispatch: sinon.stub(),
+    subscribe: sinon.stub(),
+    getState: () => {
+      return {
+        personal: {
+          accounts: {},
+          accountsInfo: {
+            '0x123': { meta: '1' },
+            '0x999': { meta: { hardware: {} } }
+          }
+        },
+        balances: {
+          balances: {}
+        }
+      };
+    }
+  };
+
+  return redux;
+}
+
+function render (props = {}) {
+  component = shallow(
+    <Accounts { ...props } />,
+    {
+      context: {
+        store: createRedux()
+      }
+    }
+  ).find('Accounts').shallow({
+    context: {
+      api: createApi()
+    }
+  });
+  instance = component.instance();
+
+  return component;
+}
+
+describe('views/Accounts', () => {
+  beforeEach(() => {
+    render();
+  });
+
+  it('renders defaults', () => {
+    expect(component).to.be.ok;
+  });
+
+  describe('instance event methods', () => {
+    describe('onHardwareChange', () => {
+      it('detects completely new entries', () => {
+        instance.hwstore = createHwStore();
+        instance.onHardwareChange();
+
+        expect(hwstore.createAccountInfo).to.have.been.calledWith({ address: '0x456' });
+      });
+
+      it('detects addressbook entries', () => {
+        instance.hwstore = createHwStore('0x123');
+        instance.onHardwareChange();
+
+        expect(hwstore.createAccountInfo).to.have.been.calledWith({ address: '0x123' }, { meta: '1' });
+      });
+
+      it('ignores existing hardware entries', () => {
+        instance.hwstore = createHwStore('0x999');
+        instance.onHardwareChange();
+
+        expect(hwstore.createAccountInfo).not.to.have.been.called;
+      });
+    });
+  });
+});