From 22440f9eec3af60cb49332ee3d33c6d0eac222af Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 15 Jan 2020 19:30:51 -0400 Subject: [PATCH 1/2] Force background state update after removing an account The e2e tests were failing intermittently after removing an account because the account was shown as not deleted after the removal. I suspect this was because the account _had_ been removed, but that change to the background state hadn't yet propagated to the UI. The background state is now synced before the loading overlay for removing the account is removed, ensuring that the removed account cannot be seen in the UI after removal. --- test/unit/ui/app/actions.spec.js | 23 +++++++++++++++-------- ui/app/store/actions.js | 32 +++++++++++++++++++------------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/test/unit/ui/app/actions.spec.js b/test/unit/ui/app/actions.spec.js index f2e676ab5a4c..349368701ed7 100644 --- a/test/unit/ui/app/actions.spec.js +++ b/test/unit/ui/app/actions.spec.js @@ -239,25 +239,29 @@ describe('Actions', () => { const store = mockStore(devState) const expectedActions = [ - { type: 'SHOW_LOADING_INDICATION', value: undefined }, - { type: 'HIDE_LOADING_INDICATION' }, - { type: 'SHOW_ACCOUNTS_PAGE' }, + 'SHOW_LOADING_INDICATION', + 'UPDATE_METAMASK_STATE', + 'HIDE_LOADING_INDICATION', + 'SHOW_ACCOUNTS_PAGE', ] removeAccountSpy = sinon.spy(background, 'removeAccount') await store.dispatch(actions.removeAccount('0xe18035bf8712672935fdb4e5e431b1a0183d2dfc')) assert(removeAccountSpy.calledOnce) - assert.deepEqual(store.getActions(), expectedActions) + const actionTypes = store + .getActions() + .map(action => action.type) + assert.deepEqual(actionTypes, expectedActions) }) it('displays warning error message when removeAccount callback errors', async () => { const store = mockStore() const expectedActions = [ - { type: 'SHOW_LOADING_INDICATION', value: undefined }, - { type: 'HIDE_LOADING_INDICATION' }, - { type: 'DISPLAY_WARNING', value: 'error' }, + 'SHOW_LOADING_INDICATION', + 'DISPLAY_WARNING', + 'HIDE_LOADING_INDICATION', ] removeAccountSpy = sinon.stub(background, 'removeAccount') @@ -269,7 +273,10 @@ describe('Actions', () => { await store.dispatch(actions.removeAccount('0xe18035bf8712672935fdb4e5e431b1a0183d2dfc')) assert.fail('Should have thrown error') } catch (_) { - assert.deepEqual(store.getActions(), expectedActions) + const actionTypes = store + .getActions() + .map(action => action.type) + assert.deepEqual(actionTypes, expectedActions) } }) diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 6f1717afecad..16dbe87b8dcf 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -385,22 +385,28 @@ export function resetAccount () { } export function removeAccount (address) { - return dispatch => { + return async dispatch => { dispatch(showLoadingIndication()) - return new Promise((resolve, reject) => { - background.removeAccount(address, (err, account) => { - dispatch(hideLoadingIndication()) - if (err) { - dispatch(displayWarning(err.message)) - return reject(err) - } - - log.info('Account removed: ' + account) - dispatch(showAccountsPage()) - resolve() + try { + await new Promise((resolve, reject) => { + background.removeAccount(address, async (error, account) => { + if (error) { + return reject(error) + } + return resolve(account) + }) }) - }) + await forceUpdateMetamaskState(dispatch) + } catch (error) { + dispatch(displayWarning(error.message)) + throw error + } finally { + dispatch(hideLoadingIndication()) + } + + log.info('Account removed: ' + address) + dispatch(showAccountsPage()) } } From e33de326880b030fe4ea11abadcabfaf9972acc8 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 16 Jan 2020 13:02:06 -0400 Subject: [PATCH 2/2] Remove unnecessary `async` Co-Authored-By: Whymarrh Whitby --- ui/app/store/actions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/app/store/actions.js b/ui/app/store/actions.js index 16dbe87b8dcf..988639f3c5d8 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -390,7 +390,7 @@ export function removeAccount (address) { try { await new Promise((resolve, reject) => { - background.removeAccount(address, async (error, account) => { + background.removeAccount(address, (error, account) => { if (error) { return reject(error) }