From cdfa2e66fd84792510b745a27c02310ecb155bbb Mon Sep 17 00:00:00 2001 From: Mark Stacey Date: Wed, 7 Oct 2020 19:29:38 -0230 Subject: [PATCH] Hide retry button for on-chain failures (#9506) On-chain failed transactions have a transaction status of `confirmed`, and should not be retried. Our retry function doesn't handle on-chain failures yet, so it would inevitably fail due to the transaction having the same nonce as a confirmed on-chain transaction. When determining whether to show the retry button in the UI, we had mistakenly been using a `status` variable that determined whether we should show "Failed" on that transaction in the activity log. That display status includes both network and on-chain failures, unlike the `txMeta.status` property. The `showRetry` logic has been updated to ensure it's only shown when `txMeta.status` is `failed`, meaning on-chain failures will no longer show the retry button. Additionally, the display-specific `status` variable has been renamed to `displayedStatusKey`, to indicate that it is a string that corresponds to a localized message, and that it's the status meant for display purposes. --- .../transaction-list-item.component.js | 12 ++++++------ .../tests/useTransactionDisplayData.test.js | 18 +++++++++--------- ui/app/hooks/useTransactionDisplayData.js | 6 +++--- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js index 0b64678ff..c501a2020 100644 --- a/ui/app/components/app/transaction-list-item/transaction-list-item.component.js +++ b/ui/app/components/app/transaction-list-item/transaction-list-item.component.js @@ -32,7 +32,7 @@ export default function TransactionListItem ({ transactionGroup, isEarliestNonce const { hasCancelled } = transactionGroup const [showDetails, setShowDetails] = useState(false) - const { initialTransaction: { id }, primaryTransaction: { err, submittedTime, gasPrice } } = transactionGroup + const { initialTransaction: { id }, primaryTransaction: { err, gasPrice, status, submittedTime } } = transactionGroup const [cancelEnabled, cancelTransaction] = useCancelTransaction(transactionGroup) const retryTransaction = useRetryTransaction(transactionGroup) const shouldShowSpeedUp = useShouldShowSpeedUp(transactionGroup, isEarliestNonce) @@ -46,7 +46,7 @@ export default function TransactionListItem ({ transactionGroup, isEarliestNonce primaryCurrency, recipientAddress, secondaryCurrency, - status, + displayedStatusKey, isPending, senderAddress, } = useTransactionDisplayData(transactionGroup) @@ -55,10 +55,10 @@ export default function TransactionListItem ({ transactionGroup, isEarliestNonce const isSignatureReq = category === TRANSACTION_CATEGORY_SIGNATURE_REQUEST const isApproval = category === TRANSACTION_CATEGORY_APPROVAL - const isUnapproved = status === UNAPPROVED_STATUS + const isUnapproved = displayedStatusKey === UNAPPROVED_STATUS const className = classnames('transaction-list-item', { - 'transaction-list-item--unconfirmed': isPending || [FAILED_STATUS, DROPPED_STATUS, REJECTED_STATUS].includes(status), + 'transaction-list-item--unconfirmed': isPending || [FAILED_STATUS, DROPPED_STATUS, REJECTED_STATUS].includes(displayedStatusKey), }) const toggleShowDetails = useCallback(() => { @@ -124,7 +124,7 @@ export default function TransactionListItem ({ transactionGroup, isEarliestNonce label={timeRemaining} /> )} - icon={} + icon={} subtitle={(

{subtitle} diff --git a/ui/app/hooks/tests/useTransactionDisplayData.test.js b/ui/app/hooks/tests/useTransactionDisplayData.test.js index 7808ac521..5291a3023 100644 --- a/ui/app/hooks/tests/useTransactionDisplayData.test.js +++ b/ui/app/hooks/tests/useTransactionDisplayData.test.js @@ -26,7 +26,7 @@ const expectedResults = [ recipientAddress: '0xffe5bc4e8f1f969934d773fa67da095d2e491a97', secondaryCurrency: '-1 ETH', isPending: false, - status: 'confirmed', + displayedStatusKey: 'confirmed', }, { title: 'Send ETH', @@ -39,7 +39,7 @@ const expectedResults = [ recipientAddress: '0x0ccc8aeeaf5ce790f3b448325981a143fdef8848', secondaryCurrency: '-2 ETH', isPending: false, - status: 'confirmed', + displayedStatusKey: 'confirmed', }, { title: 'Send ETH', @@ -52,7 +52,7 @@ const expectedResults = [ recipientAddress: '0xffe5bc4e8f1f969934d773fa67da095d2e491a97', secondaryCurrency: '-2 ETH', isPending: false, - status: 'confirmed', + displayedStatusKey: 'confirmed', }, { title: 'Receive', @@ -65,7 +65,7 @@ const expectedResults = [ recipientAddress: '0x9eca64466f257793eaa52fcfff5066894b76a149', secondaryCurrency: '18.75 ETH', isPending: false, - status: 'confirmed', + displayedStatusKey: 'confirmed', }, { title: 'Receive', @@ -78,7 +78,7 @@ const expectedResults = [ recipientAddress: '0x9eca64466f257793eaa52fcfff5066894b76a149', secondaryCurrency: '0 ETH', isPending: false, - status: 'confirmed', + displayedStatusKey: 'confirmed', }, { title: 'Receive', @@ -91,7 +91,7 @@ const expectedResults = [ recipientAddress: '0x9eca64466f257793eaa52fcfff5066894b76a149', secondaryCurrency: '1 ETH', isPending: false, - status: 'confirmed', + displayedStatusKey: 'confirmed', }, { title: 'Swap ETH to ABC', @@ -104,7 +104,7 @@ const expectedResults = [ recipientAddress: '0xabca64466f257793eaa52fcfff5066894b76a149', secondaryCurrency: undefined, isPending: false, - status: 'confirmed', + displayedStatusKey: 'confirmed', }, ] @@ -167,9 +167,9 @@ describe('useTransactionDisplayData', function () { const { result } = renderHookWithRouter(() => useTransactionDisplayData(transactionGroup), tokenAddress) assert.equal(result.current.secondaryCurrency, expected.secondaryCurrency) }) - it(`should return a status of ${expected.status}`, function () { + it(`should return a displayedStatusKey of ${expected.displayedStatusKey}`, function () { const { result } = renderHookWithRouter(() => useTransactionDisplayData(transactionGroup), tokenAddress) - assert.equal(result.current.status, expected.status) + assert.equal(result.current.displayedStatusKey, expected.displayedStatusKey) }) it(`should return a recipientAddress of ${expected.recipientAddress}`, function () { const { result } = renderHookWithRouter(() => useTransactionDisplayData(transactionGroup), tokenAddress) diff --git a/ui/app/hooks/useTransactionDisplayData.js b/ui/app/hooks/useTransactionDisplayData.js index d03283dfd..097db7119 100644 --- a/ui/app/hooks/useTransactionDisplayData.js +++ b/ui/app/hooks/useTransactionDisplayData.js @@ -72,8 +72,8 @@ export function useTransactionDisplayData (transactionGroup) { // for smart contract interactions, methodData can be used to derive the name of the action being taken const methodData = useSelector((state) => getKnownMethodData(state, initialTransaction?.txParams?.data)) || {} - const status = getStatusKey(primaryTransaction) - const isPending = status in PENDING_STATUS_HASH + const displayedStatusKey = getStatusKey(primaryTransaction) + const isPending = displayedStatusKey in PENDING_STATUS_HASH const primaryValue = primaryTransaction.txParams?.value let prefix = '-' @@ -210,7 +210,7 @@ export function useTransactionDisplayData (transactionGroup) { (isTokenCategory && !tokenFiatAmount) || (transactionCategory === SWAP && !swapTokenFiatAmount) ) ? undefined : secondaryCurrency, - status, + displayedStatusKey, isPending, } }