Skip to content

Commit

Permalink
Force background state update after removing an account (#7840)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gudahtt authored Jan 16, 2020
1 parent 8225bbe commit 20a7b8f
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 21 deletions.
23 changes: 15 additions & 8 deletions test/unit/ui/app/actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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)
}

})
Expand Down
32 changes: 19 additions & 13 deletions ui/app/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}

Expand Down

0 comments on commit 20a7b8f

Please sign in to comment.