Default account selection update (#4609)

* Default accounts setting - account provider

* RPC support for default accounts

* Updating JS code

* Rename whitelist to addresses

* Set the defaults using default, allowing for null/full lists

* Update failing tests (after merge)

* Fix merge with wrong rpc call names

* One account needs to be selected

js/src/modals/DappPermissions/store.js

@@ -20,6 +20,7 @@ export default class Store {
   @observable accounts = [];
   @observable modalOpen = false;
   @observable whitelist = [];
+  @observable whitelistDefault = null;
 
   constructor (api) {
     this._api = api;
@@ -29,17 +30,14 @@ export default class Store {
 
   @action closeModal = () => {
     transaction(() => {
-      let addresses = null;
       const checkedAccounts = this.accounts.filter((account) => account.checked);
-
-      if (checkedAccounts.length) {
-        addresses = checkedAccounts.filter((account) => account.default)
-          .concat(checkedAccounts.filter((account) => !account.default))
-          .map((account) => account.address);
-      }
+      const defaultAddress = (this.accounts.find((account) => account.default) || {}).address;
+      const addresses = checkedAccounts.length === this.accounts.length
+        ? null
+        : checkedAccounts.map((account) => account.address);
 
       this.modalOpen = false;
-      this.updateWhitelist(addresses);
+      this.updateWhitelist(addresses, defaultAddress);
     });
   }
 
@@ -53,8 +51,8 @@ export default class Store {
             checked: this.whitelist
               ? this.whitelist.includes(account.address)
               : true,
-            default: this.whitelist
-              ? this.whitelist[0] === account.address
+            default: this.whitelistDefault
+              ? this.whitelistDefault === account.address
               : index === 0,
             description: account.meta.description,
             name: account.name
@@ -66,8 +64,10 @@ export default class Store {
 
   @action selectAccount = (address) => {
     transaction(() => {
+      const isSingleAccount = this.accounts.filter((account) => account.checked).length === 1;
+
       this.accounts = this.accounts.map((account) => {
-        if (account.address === address) {
+        if (account.address === address && (!isSingleAccount || !account.checked)) {
           account.checked = !account.checked;
           account.default = false;
         }
@@ -96,26 +96,35 @@ export default class Store {
     });
   }
 
-  @action setWhitelist = (whitelist) => {
-    this.whitelist = whitelist;
+  @action setWhitelist = (whitelist, whitelistDefault) => {
+    transaction(() => {
+      this.whitelist = whitelist;
+      this.whitelistDefault = whitelistDefault;
+    });
   }
 
   loadWhitelist () {
-    return this._api.parity
-      .getNewDappsAddresses()
-      .then((whitelist) => {
-        this.setWhitelist(whitelist);
+    return Promise
+      .all([
+        this._api.parity.getNewDappsAddresses(),
+        this._api.parity.getNewDappsDefaultAddress()
+      ])
+      .then(([whitelist, whitelistDefault]) => {
+        this.setWhitelist(whitelist, whitelistDefault);
       })
       .catch((error) => {
         console.warn('loadWhitelist', error);
       });
   }
 
-  updateWhitelist (whitelist) {
-    return this._api.parity
-      .setNewDappsAddresses(whitelist)
+  updateWhitelist (whitelist, whitelistDefault = null) {
+    return Promise
+      .all([
+        this._api.parity.setNewDappsAddresses(whitelist),
+        this._api.parity.setNewDappsDefaultAddress(whitelistDefault)
+      ])
       .then(() => {
-        this.setWhitelist(whitelist);
+        this.setWhitelist(whitelist, whitelistDefault);
       })
       .catch((error) => {
         console.warn('updateWhitelist', error);

js/src/modals/DappPermissions/store.spec.js

@@ -32,14 +32,16 @@ function create () {
   api = {
     parity: {
       getNewDappsAddresses: sinon.stub().resolves(WHITELIST),
-      setNewDappsAddresses: sinon.stub().resolves(true)
+      getNewDappsDefaultAddress: sinon.stub().resolves(WHITELIST[0]),
+      setNewDappsAddresses: sinon.stub().resolves(true),
+      setNewDappsDefaultAddress: sinon.stub().resolves(true)
     }
   };
 
   store = new Store(api);
 }
 
-describe('modals/DappPermissions/store', () => {
+describe.only('modals/DappPermissions/store', () => {
   beforeEach(() => {
     create();
   });
@@ -80,11 +82,11 @@ describe('modals/DappPermissions/store', () => {
       });
 
       it('calls setNewDappsAddresses', () => {
-        expect(api.parity.setNewDappsAddresses).to.have.been.calledOnce;
+        expect(api.parity.setNewDappsAddresses).to.have.been.calledWith(['456', '789']);
       });
 
-      it('has the default account in first position', () => {
-        expect(api.parity.setNewDappsAddresses).to.have.been.calledWith(['789', '456']);
+      it('calls into setNewDappsDefaultAddress', () => {
+        expect(api.parity.setNewDappsDefaultAddress).to.have.been.calledWith('789');
       });
     });
 
@@ -107,6 +109,14 @@ describe('modals/DappPermissions/store', () => {
         expect(store.accounts.find((account) => account.address === '456').default).to.be.false;
         expect(store.accounts.find((account) => account.address === '123').default).to.be.true;
       });
+
+      it('does not deselect the last account', () => {
+        store.selectAccount('123');
+        store.selectAccount('456');
+        console.log(store.accounts.map((account) => ({ address: account.address, checked: account.checked })));
+        expect(store.accounts.find((account) => account.address === '456').default).to.be.true;
+        expect(store.accounts.find((account) => account.address === '456').checked).to.be.true;
+      });
     });
 
     describe('setDefaultAccount', () => {

js/src/views/ParityBar/accountStore.js

@@ -50,18 +50,11 @@ export default class AccountStore {
     this.isLoading = isLoading;
   }
 
-  makeDefaultAccount = (address) => {
-    const accounts = [address].concat(
-      this.accounts
-        .filter((account) => account.address !== address)
-        .map((account) => account.address)
-    );
-
-    // Have optimistic UI: https://www.smashingmagazine.com/2016/11/true-lies-of-optimistic-user-interfaces/?utm_source=codropscollective
-    this.setDefaultAccount(address);
+  makeDefaultAccount = (defaultAddress) => {
+    this.setDefaultAccount(defaultAddress);
 
     return this._api.parity
-      .setNewDappsAddresses(accounts)
+      .setNewDappsDefaultAddress(defaultAddress)
       .catch((error) => {
         console.warn('makeDefaultAccount', error);
       });

js/src/views/ParityBar/accountStore.spec.js

@@ -18,7 +18,7 @@ import sinon from 'sinon';
 
 import AccountStore from './accountStore';
 
-import { ACCOUNT_DEFAULT, ACCOUNT_FIRST, ACCOUNT_NEW, createApi } from './parityBar.test.js';
+import { ACCOUNT_DEFAULT, ACCOUNT_NEW, createApi } from './parityBar.test.js';
 
 let api;
 let store;
@@ -104,10 +104,8 @@ describe('views/ParityBar/AccountStore', () => {
         return store.makeDefaultAccount(ACCOUNT_NEW);
       });
 
-      it('calls into parity_setNewDappsAddresses (with ordering)', () => {
-        expect(api.parity.setNewDappsAddresses).to.have.been.calledWith([
-          ACCOUNT_NEW, ACCOUNT_FIRST, ACCOUNT_DEFAULT
-        ]);
+      it('calls into parity_setNewDappsDefaultAddress', () => {
+        expect(api.parity.setNewDappsDefaultAddress).to.have.been.calledWith(ACCOUNT_NEW);
       });
     });
   });

js/src/views/ParityBar/parityBar.test.js

@@ -37,7 +37,8 @@ function createApi () {
       defaultAccount: sinon.stub().resolves(ACCOUNT_DEFAULT),
       allAccountsInfo: sinon.stub().resolves(ACCOUNTS),
       getNewDappsAddresses: sinon.stub().resolves(null),
-      setNewDappsAddresses: sinon.stub().resolves(true)
+      setNewDappsAddresses: sinon.stub().resolves(true),
+      setNewDappsDefaultAddress: sinon.stub().resolves(true)
     }
   };