From 20a7b8fa36a68a96f8fd9a800059abc7ce6d9204 Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Thu, 16 Jan 2020 13:43:40 -0400 Subject: [PATCH] Force background state update after removing an account (#7840) 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 f2e676ab5..349368701 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 4ae342a95..7b1f78b29 100644 --- a/ui/app/store/actions.js +++ b/ui/app/store/actions.js @@ -380,22 +380,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, (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()) } }