Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix batch transaction UX #7473

Merged
merged 4 commits into from
Jan 10, 2020
Merged

Fix batch transaction UX #7473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16a085f
fix batch transaction ui; update json-rpc-engine
rekmarks Nov 13, 2019
a47b384
fix tx request numbering
rekmarks Jan 8, 2020
5d120b3
fix UI console error
rekmarks Jan 8, 2020
5ed8494
update e2e and integration tests
rekmarks Jan 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
"eth-json-rpc-errors": "^2.0.0",
"eth-json-rpc-filters": "^4.1.1",
"eth-json-rpc-infura": "^4.0.1",
"eth-json-rpc-middleware": "^4.2.0",
"eth-json-rpc-middleware": "^4.4.0",
"eth-keyring-controller": "^5.3.0",
"eth-ledger-bridge-keyring": "^0.2.0",
"eth-method-registry": "^1.2.0",
Expand All @@ -122,7 +122,7 @@
"gaba": "^1.9.0",
"human-standard-token-abi": "^2.0.0",
"jazzicon": "^1.2.0",
"json-rpc-engine": "^5.1.5",
"json-rpc-engine": "^5.1.6",
"json-rpc-middleware-stream": "^2.1.1",
"jsonschema": "^1.2.4",
"lodash.debounce": "^4.0.8",
Expand Down
10 changes: 5 additions & 5 deletions test/e2e/metamask-ui.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,11 @@ describe('MetaMask', function () {
await delay(regularDelayMs)

let transactions = await findElements(driver, By.css('.transaction-list-item'))
await transactions[3].click()
await transactions[0].click()
await delay(regularDelayMs)
try {
transactions = await findElements(driver, By.css('.transaction-list-item'), 1000)
await transactions[3].click()
await transactions[0].click()
} catch (e) {
console.log(e)
}
Expand Down Expand Up @@ -646,7 +646,7 @@ describe('MetaMask', function () {

navigationElement = await findElement(driver, By.css('.confirm-page-container-navigation'))
navigationText = await navigationElement.getText()
assert.equal(navigationText.includes('3'), true, 'correct transaction in focus')
assert.equal(navigationText.includes('2'), true, 'correct (same) transaction in focus')
})

it('confirms a transaction', async () => {
Expand Down Expand Up @@ -853,9 +853,9 @@ describe('MetaMask', function () {
it('renders the correct ETH balance', async () => {
const balance = await findElement(driver, By.css('.transaction-view-balance__primary-balance'))
await delay(regularDelayMs)
await driver.wait(until.elementTextMatches(balance, /^87.*\s*ETH.*$/), 10000)
await driver.wait(until.elementTextMatches(balance, /^90.*\s*ETH.*$/), 10000)
const tokenAmount = await balance.getText()
assert.ok(/^87.*\s*ETH.*$/.test(tokenAmount))
assert.ok(/^90.*\s*ETH.*$/.test(tokenAmount))
Comment on lines -856 to +858
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are due to a send of 3 ETH now being rejected instead of approved due to the updated UI transaction order. An equivalent solution would be to ensure that transaction is approved, this was just easier.

await delay(regularDelayMs)
})
})
Expand Down
16 changes: 8 additions & 8 deletions test/integration/lib/confirm-sig-requests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,18 @@ async function runConfirmSigRequestsTest (assert) {

const pendingRequestItem = $.find('.transaction-list-item .transaction-list-item__grid')

if (pendingRequestItem[2]) {
pendingRequestItem[2].click()
if (pendingRequestItem[0]) {
pendingRequestItem[0].click()
}

await timeout(1000)

let confirmSigHeadline = await queryAsync($, '.request-signature__headline')
assert.equal(confirmSigHeadline[0].textContent, 'Your signature is being requested')

const confirmSigMessage = await queryAsync($, '.request-signature__notice')
assert.ok(confirmSigMessage[0].textContent.match(/^Signing\sthis\smessage/))

let confirmSigRowValue = await queryAsync($, '.request-signature__row-value')
assert.equal(confirmSigRowValue[0].textContent, '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0')
assert.equal(confirmSigRowValue[0].textContent, 'Hi, Alice!')
assert.equal(confirmSigRowValue[1].textContent, '1337')

let confirmSigSignButton = await queryAsync($, 'button.btn-secondary.btn--large')
confirmSigSignButton[0].click()
Expand All @@ -66,9 +64,11 @@ async function runConfirmSigRequestsTest (assert) {
confirmSigHeadline = await queryAsync($, '.request-signature__headline')
assert.equal(confirmSigHeadline[0].textContent, 'Your signature is being requested')

const confirmSigMessage = await queryAsync($, '.request-signature__notice')
assert.ok(confirmSigMessage[0].textContent.match(/^Signing\sthis\smessage/))

confirmSigRowValue = await queryAsync($, '.request-signature__row-value')
assert.equal(confirmSigRowValue[0].textContent, 'Hi, Alice!')
assert.equal(confirmSigRowValue[1].textContent, '1337')
assert.equal(confirmSigRowValue[0].textContent, '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0')

confirmSigSignButton = await queryAsync($, 'button.btn-secondary.btn--large')
confirmSigSignButton[0].click()
Expand Down
2 changes: 1 addition & 1 deletion ui/app/components/ui/button/button.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const typeHash = {

const Button = ({ type, submit, large, children, className, ...buttonProps }) => (
<button
type={submit && 'submit'}
type={submit ? 'submit' : undefined}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

className={classnames(
'button',
typeHash[type] || CLASSNAME_DEFAULT,
Expand Down
2 changes: 1 addition & 1 deletion ui/app/pages/confirm-transaction-base/confirm-transaction-base.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ export default class ConfirmTransactionBase extends Component {

getNavigateTxData () {
const { currentNetworkUnapprovedTxs, txData: { id } = {} } = this.props
const enumUnapprovedTxs = Object.keys(currentNetworkUnapprovedTxs).reverse()
const enumUnapprovedTxs = Object.keys(currentNetworkUnapprovedTxs)
const currentPosition = enumUnapprovedTxs.indexOf(id ? id.toString() : '')

return {
Expand Down
12 changes: 6 additions & 6 deletions ui/app/pages/confirm-transaction-base/confirm-transaction-base.container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { connect } from 'react-redux'
import { compose } from 'recompose'
import { withRouter } from 'react-router-dom'
import R from 'ramda'
import contractMap from 'eth-contract-metadata'
import ConfirmTransactionBase from './confirm-transaction-base.component'
import {
Expand Down Expand Up @@ -68,7 +67,9 @@ const mapStateToProps = (state, ownProps) => {
nonce,
} = confirmTransaction
const { txParams = {}, lastGasPrice, id: transactionId, transactionCategory } = txData
const transaction = Object.values(unapprovedTxs).find(({ id }) => id === (transactionId || Number(paramsTransactionId))) || {}
const transaction = Object.values(unapprovedTxs).find(
({ id }) => id === (transactionId || Number(paramsTransactionId))
) || {}
const {
from: fromAddress,
to: txParamsToAddress,
Expand Down Expand Up @@ -108,10 +109,9 @@ const mapStateToProps = (state, ownProps) => {
txData.simulationFails = transaction.simulationFails
}

const currentNetworkUnapprovedTxs = R.filter(
({ metamaskNetworkId }) => metamaskNetworkId === network,
unapprovedTxs,
)
const currentNetworkUnapprovedTxs = Object.keys(unapprovedTxs)
.filter(key => unapprovedTxs[key].metamaskNetworkId === network)
.reduce((acc, key) => ({ ...acc, [key]: unapprovedTxs[key] }), {})
const unapprovedTxCount = valuesFor(currentNetworkUnapprovedTxs).length

const insufficientBalance = !isBalanceSufficient({
Expand Down
2 changes: 1 addition & 1 deletion ui/app/pages/confirm-transaction-switch/confirm-transaction-switch.container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const mapStateToProps = (state, ownProps) => {
const unconfirmedTransactions = unconfirmedTransactionsListSelector(state)
const totalUnconfirmed = unconfirmedTransactions.length
const transaction = totalUnconfirmed
? unapprovedTxs[transactionId] || unconfirmedTransactions[totalUnconfirmed - 1]
? unapprovedTxs[transactionId] || unconfirmedTransactions[0]
: {}

return {
Expand Down
2 changes: 1 addition & 1 deletion ui/app/pages/confirm-transaction/confirm-transaction.container.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const mapStateToProps = (state, ownProps) => {
const unconfirmedTransactions = unconfirmedTransactionsListSelector(state)
const totalUnconfirmed = unconfirmedTransactions.length
const transaction = totalUnconfirmed
? unapprovedTxs[id] || unconfirmedTransactions[totalUnconfirmed - 1]
? unapprovedTxs[id] || unconfirmedTransactions[0]
: {}
const { id: transactionId, transactionCategory } = transaction

Expand Down
2 changes: 1 addition & 1 deletion ui/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ async function startApp (metamaskState, backgroundConnection, opts) {
const numberOfUnapprivedTx = unapprovedTxsAll.length
if (numberOfUnapprivedTx > 0) {
store.dispatch(actions.showConfTxPage({
id: unapprovedTxsAll[numberOfUnapprivedTx - 1].id,
id: unapprovedTxsAll[0].id,
}))
}

Expand Down
30 changes: 25 additions & 5 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10423,7 +10423,7 @@ eth-json-rpc-middleware@^1.5.0:
promise-to-callback "^1.0.0"
tape "^4.6.3"

eth-json-rpc-middleware@^4.1.4, eth-json-rpc-middleware@^4.1.5, eth-json-rpc-middleware@^4.2.0:
eth-json-rpc-middleware@^4.1.4, eth-json-rpc-middleware@^4.1.5:
version "4.2.0"
resolved "https://registry.yarnpkg.com/eth-json-rpc-middleware/-/eth-json-rpc-middleware-4.2.0.tgz#cfb77c5056cb8001548c6c7d54f4af5fce04d489"
integrity sha512-90LljqRyJhkg7fOwKunh1lu1Mr5bspXMBDitaTGyGPPNiFTbMrhtfbf9fteYlXRFCbq+aIFWwl/X+P7nkrdkLg==
Expand All @@ -10443,6 +10443,26 @@ eth-json-rpc-middleware@^4.1.4, eth-json-rpc-middleware@^4.1.5, eth-json-rpc-mid
pify "^3.0.0"
safe-event-emitter "^1.0.1"

eth-json-rpc-middleware@^4.4.0:
version "4.4.0"
resolved "https://registry.yarnpkg.com/eth-json-rpc-middleware/-/eth-json-rpc-middleware-4.4.0.tgz#ef63b783b48dcbea9c1fe25c79e6ea01510e5877"
integrity sha512-IeOsil/XiHsybJO9nFf86+1+YIqGQWPPfiTEp3WLkpLZhJm97kw6tFM7GttIZXIcwtaO3zEXgY6PWAH1jkB3ag==
dependencies:
btoa "^1.2.1"
clone "^2.1.1"
eth-json-rpc-errors "^1.0.1"
eth-query "^2.1.2"
eth-sig-util "^1.4.2"
ethereumjs-block "^1.6.0"
ethereumjs-tx "^1.3.7"
ethereumjs-util "^5.1.2"
ethereumjs-vm "^2.6.0"
fetch-ponyfill "^4.0.0"
json-rpc-engine "^5.1.3"
json-stable-stringify "^1.0.1"
pify "^3.0.0"
safe-event-emitter "^1.0.1"

eth-keyring-controller@^5.3.0:
version "5.3.0"
resolved "https://registry.yarnpkg.com/eth-keyring-controller/-/eth-keyring-controller-5.3.0.tgz#8d85a67b894360ab7d601222ca71df8ed5f456c6"
Expand Down Expand Up @@ -16189,10 +16209,10 @@ json-rpc-engine@^3.4.0, json-rpc-engine@^3.6.0:
promise-to-callback "^1.0.0"
safe-event-emitter "^1.0.1"

json-rpc-engine@^5.1.3, json-rpc-engine@^5.1.5:
version "5.1.5"
resolved "https://registry.yarnpkg.com/json-rpc-engine/-/json-rpc-engine-5.1.5.tgz#a5f9915356ea916d5305716354080723c63dede7"
integrity sha512-HTT9HixG4j8vHYrmJIckgbISW9Q8tCkySv7x7Q8zjMpcw10wSe/dZSQ0w08VkDm3y195K4074UlvD3hxaznvlw==
json-rpc-engine@^5.1.3, json-rpc-engine@^5.1.5, json-rpc-engine@^5.1.6:
version "5.1.6"
resolved "https://registry.yarnpkg.com/json-rpc-engine/-/json-rpc-engine-5.1.6.tgz#3823c1e227657ac5f22a36351db5bb76fa70cf38"
integrity sha512-9nDeIIu6o7cvzWRrHNuNi+TiGe+YWOp3ZQkHtpPnQzXuX8Y5ZU2Oot3FDI+DaQyXIqQ6SjtM6rixDOJTjjA8NA==
dependencies:
async "^2.0.1"
eth-json-rpc-errors "^2.0.0"
Expand Down