fix(money): show toast and keep keyboard open when amount update fails on Done

Author: matthewwalsh0

app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.test.tsx

@@ -1,5 +1,6 @@
 import React, { act } from 'react';
 import { merge, noop } from 'lodash';
+import { ToastContext } from '../../../../../../component-library/components/Toast';
 import renderWithProvider from '../../../../../../util/test/renderWithProvider';
 import {
   CustomAmountInfo,
@@ -177,32 +178,42 @@ jest.mock('../../../../../UI/Ramp/hooks/useRampsPaymentMethods', () => ({
 const TOKEN_ADDRESS_MOCK = '0x123' as Hex;
 const CHAIN_ID_MOCK = '0x1' as Hex;
 
+const mockShowToast = jest.fn();
+const mockToastRef = {
+  current: { showToast: mockShowToast, closeToast: jest.fn() },
+};
+
 function render(
   props: CustomAmountInfoProps & { transactionType?: TransactionType } = {},
 ) {
-  return renderWithProvider(<CustomAmountInfo {...props} />, {
-    state: merge(
-      {},
-      simpleSendTransactionControllerMock,
-      transactionApprovalControllerMock,
-      otherControllersMock,
-      {
-        engine: {
-          backgroundState: {
-            TransactionController: {
-              transactions: [
-                {
-                  type:
-                    props.transactionType ||
-                    TransactionType.contractInteraction,
-                },
-              ],
+  return renderWithProvider(
+    <ToastContext.Provider value={{ toastRef: mockToastRef } as never}>
+      <CustomAmountInfo {...props} />
+    </ToastContext.Provider>,
+    {
+      state: merge(
+        {},
+        simpleSendTransactionControllerMock,
+        transactionApprovalControllerMock,
+        otherControllersMock,
+        {
+          engine: {
+            backgroundState: {
+              TransactionController: {
+                transactions: [
+                  {
+                    type:
+                      props.transactionType ||
+                      TransactionType.contractInteraction,
+                  },
+                ],
+              },
             },
           },
         },
-      },
-    ),
-  });
+      ),
+    },
+  );
 }
 
 describe('CustomAmountInfo', () => {
@@ -261,6 +272,7 @@ describe('CustomAmountInfo', () => {
 
   beforeEach(() => {
     jest.resetAllMocks();
+    mockShowToast.mockReset();
 
     mockUseRampsUserRegion.mockReturnValue({
       userRegion: { regionCode: 'us-ca' },
@@ -577,7 +589,7 @@ describe('CustomAmountInfo', () => {
     expect(onConfirmMock).toHaveBeenCalledTimes(1);
   });
 
-  it('still runs UI cleanup and logs the error when updateTokenAmount rejects on Done', async () => {
+  it('shows toast, keeps keyboard open, and skips onAmountSubmit when updateTokenAmount rejects on Done', async () => {
     const error = new Error('update failed');
     const updateTokenAmountMock = jest.fn().mockRejectedValue(error);
     const mockOnAmountSubmit = jest.fn();
@@ -596,14 +608,24 @@ describe('CustomAmountInfo', () => {
       updateTokenAmount: updateTokenAmountMock,
     });
 
-    const { getByText } = render({ onAmountSubmit: mockOnAmountSubmit });
+    const { getByText, queryByText } = render({
+      onAmountSubmit: mockOnAmountSubmit,
+    });
 
     await act(async () => {
       fireEvent.press(getByText(strings('confirm.edit_amount_done')));
     });
 
     expect(updateTokenAmountMock).toHaveBeenCalledTimes(1);
-    expect(mockOnAmountSubmit).toHaveBeenCalledTimes(1);
+    // onAmountSubmit must NOT be called — keep keyboard visible for retry
+    expect(mockOnAmountSubmit).not.toHaveBeenCalled();
+    // Toast must be shown with the transaction-update title
+    expect(mockShowToast).toHaveBeenCalledTimes(1);
+    expect(mockShowToast).toHaveBeenCalledWith(
+      expect.objectContaining({ labelOptions: expect.any(Array) }),
+    );
+    // Keyboard stays open: Done button still present
+    expect(queryByText(strings('confirm.edit_amount_done'))).toBeOnTheScreen();
     expect(loggerErrorMock).toHaveBeenCalledWith(
       error,
       expect.stringContaining('Failed to apply custom amount on Done press'),
@@ -932,7 +954,7 @@ describe('CustomAmountInfo', () => {
       );
     });
 
-    it('does not fire RAMPS_ORDER_PROPOSED when applying the amount throws on Done', async () => {
+    it('does not fire RAMPS_ORDER_PROPOSED and shows toast when applying the amount throws on Done', async () => {
       setMoneyFlow();
       const error = new Error('update failed');
       const loggerErrorMock = jest.mocked(Logger.error);
@@ -949,8 +971,9 @@ describe('CustomAmountInfo', () => {
         fireEvent.press(getByText(strings('confirm.edit_amount_done')));
       });
 
-      // The Done handler's catch suppresses the commit but logs the error.
+      // The Done handler's catch shows a toast and returns early without committing.
       expect(emittedPayloadFor('RAMPS_ORDER_PROPOSED')).toBeUndefined();
+      expect(mockShowToast).toHaveBeenCalledTimes(1);
       expect(loggerErrorMock).toHaveBeenCalledWith(
         error,
         expect.stringContaining('Failed to apply custom amount on Done press'),

app/components/Views/confirmations/components/info/custom-amount-info/custom-amount-info.tsx

@@ -1,4 +1,11 @@
-import React, { ReactNode, memo, useCallback, useRef, useState } from 'react';
+import React, {
+  ReactNode,
+  memo,
+  useCallback,
+  useContext,
+  useRef,
+  useState,
+} from 'react';
 import { toCaipAssetType } from '@metamask/utils';
 import { TransactionType } from '@metamask/transaction-controller';
 import { PayTokenAmount, PayTokenAmountSkeleton } from '../../pay-token-amount';
@@ -65,6 +72,8 @@ import { useConfirmActions } from '../../../hooks/useConfirmActions';
 import EngineService from '../../../../../../core/EngineService';
 import Engine from '../../../../../../core/Engine';
 import Logger from '../../../../../../util/Logger';
+import { getTransactionUpdateErrorToastOptions } from '../../../../../../util/confirmation/transactions';
+import { ToastContext } from '../../../../../../component-library/components/Toast';
 import { ConfirmationFooterSelectorIDs } from '../../../ConfirmationView.testIds';
 import { useTransactionPayToken } from '../../../hooks/pay/useTransactionPayToken';
 import { useMoneyNoFeeTokens } from '../../../hooks/pay/useMoneyNoFeeTokens';
@@ -161,6 +170,8 @@ export const CustomAmountInfo: React.FC<CustomAmountInfoProps> = memo(
     const accountOverride = useTransactionAccountOverride();
     const isWithdraw = isTransactionPayWithdraw(transactionMeta);
 
+    const { toastRef } = useContext(ToastContext);
+
     const isResultReady = useIsResultReady({ isKeyboardVisible });
     const quotes = useTransactionPayQuotes();
     const isQuotesLoading = useIsTransactionPayLoading();
@@ -198,18 +209,24 @@ export const CustomAmountInfo: React.FC<CustomAmountInfoProps> = memo(
     const handleDone = useCallback(async () => {
       try {
         if (selectedFiatPaymentMethodId && transactionId) {
-          Engine.context.TransactionPayController.updateFiatPayment({
-            transactionId,
-            callback: (fp) => {
-              fp.amountFiat = amountFiat;
-            },
-          });
-
-          // Fiat deposits need nested calldata (approve + deposit) populated
-          // with approximate amounts now so the transaction is valid at submit
-          // time. Core will re-encode with exact amounts after settlement.
           if (isMoneyAccountDeposit) {
+            // Update calldata first; only commit the fiat payment amount after
+            // success so Pay config does not reflect an amount whose calldata
+            // update failed.
             await updateTokenAmount();
+            Engine.context.TransactionPayController.updateFiatPayment({
+              transactionId,
+              callback: (fp) => {
+                fp.amountFiat = amountFiat;
+              },
+            });
+          } else {
+            Engine.context.TransactionPayController.updateFiatPayment({
+              transactionId,
+              callback: (fp) => {
+                fp.amountFiat = amountFiat;
+              },
+            });
           }
         } else {
           await updateTokenAmount();
@@ -222,16 +239,21 @@ export const CustomAmountInfo: React.FC<CustomAmountInfoProps> = memo(
           error as Error,
           'Failed to apply custom amount on Done press',
         );
-      } finally {
-        EngineService.flushState();
-        setIsKeyboardVisible(false);
-        onAmountSubmit?.();
+        toastRef?.current?.showToast(
+          getTransactionUpdateErrorToastOptions(error),
+        );
+        // Keep keyboard visible so the user can retry; do not advance the flow.
+        return;
       }
+      EngineService.flushState();
+      setIsKeyboardVisible(false);
+      onAmountSubmit?.();
     }, [
       amountFiat,
       isMoneyAccountDeposit,
       onAmountSubmit,
       selectedFiatPaymentMethodId,
+      toastRef,
       trackAmountCommitted,
       transactionId,
       updateTokenAmount,

app/components/Views/confirmations/hooks/pay/useUpdateTransactionPayAmount.test.ts

@@ -179,7 +179,7 @@ describe('useUpdateTransactionPayAmount', () => {
     expect(updateAtomicBatchDataMock).not.toHaveBeenCalled();
   });
 
-  it('logs an error when updateAtomicBatchData rejects', async () => {
+  it('rejects and logs when updateAtomicBatchData rejects', async () => {
     const error = new Error('boom');
     updateAtomicBatchDataMock.mockRejectedValue(error);
     updateMoneyAccountDepositTokenAmountMock.mockResolvedValue([
@@ -188,27 +188,20 @@ describe('useUpdateTransactionPayAmount', () => {
 
     const { result } = runHook({ transactionMeta: moneyAccountDepositMeta });
 
-    result.current.updateTransactionPayAmount('1.23');
-
-    await flushPromises();
-
-    expect(loggerErrorMock).toHaveBeenCalledWith(
-      error,
-      expect.stringContaining(
-        'Failed to update transaction pay amount in nested transaction',
-      ),
-    );
+    await expect(
+      result.current.updateTransactionPayAmount('1.23'),
+    ).rejects.toThrow('boom');
   });
 
-  it('logs an error when updateMoneyAccountDepositTokenAmount rejects', async () => {
+  it('rejects and logs when updateMoneyAccountDepositTokenAmount rejects', async () => {
     const error = new Error('rpc failure');
     updateMoneyAccountDepositTokenAmountMock.mockRejectedValue(error);
 
     const { result } = runHook({ transactionMeta: moneyAccountDepositMeta });
 
-    result.current.updateTransactionPayAmount('1.23');
-
-    await flushPromises();
+    await expect(
+      result.current.updateTransactionPayAmount('1.23'),
+    ).rejects.toThrow('rpc failure');
 
     expect(updateAtomicBatchDataMock).not.toHaveBeenCalled();
     expect(loggerErrorMock).toHaveBeenCalledWith(
@@ -281,15 +274,15 @@ describe('useUpdateTransactionPayAmount', () => {
     );
   });
 
-  it('logs an error when updateMoneyAccountWithdrawTokenAmount rejects', async () => {
+  it('rejects and logs when updateMoneyAccountWithdrawTokenAmount rejects', async () => {
     const error = new Error('withdraw rpc failure');
     updateMoneyAccountWithdrawTokenAmountMock.mockRejectedValue(error);
 
     const { result } = runHook({ transactionMeta: moneyAccountWithdrawMeta });
 
-    result.current.updateTransactionPayAmount('4.56');
-
-    await flushPromises();
+    await expect(
+      result.current.updateTransactionPayAmount('4.56'),
+    ).rejects.toThrow('withdraw rpc failure');
 
     expect(updateAtomicBatchDataMock).not.toHaveBeenCalled();
     expect(loggerErrorMock).toHaveBeenCalledWith(
@@ -408,7 +401,7 @@ describe('useUpdateTransactionPayAmount', () => {
       expect(updateTransactionMock).not.toHaveBeenCalled();
     });
 
-    it('logs an error when updateTransaction throws', async () => {
+    it('rejects and logs when updateTransaction throws', async () => {
       const error = new Error('updateTransaction failed');
       updateTransactionMock.mockImplementation(() => {
         throw error;
@@ -418,9 +411,9 @@ describe('useUpdateTransactionPayAmount', () => {
         transactionMeta: moneyAccountDepositMetaWithRequiredAssets,
       });
 
-      result.current.updateTransactionPayAmount('1');
-
-      await flushPromises();
+      await expect(
+        result.current.updateTransactionPayAmount('1'),
+      ).rejects.toThrow('updateTransaction failed');
 
       expect(loggerErrorMock).toHaveBeenCalledWith(
         error,

app/components/Views/confirmations/hooks/pay/useUpdateTransactionPayAmount.ts

@@ -45,34 +45,27 @@ export function useUpdateTransactionPayAmount() {
         return;
       }
 
+      let updates: UpdateTransactionPayAmountCall[];
       try {
-        const updates = await updater(
+        updates = await updater(
           transactionMeta,
           amountHuman,
           recipientOverride,
         );
-
-        const results = await Promise.allSettled(
-          updates.map(({ nestedTransactionIndex, transactionData }) =>
-            updateAtomicBatchData({
-              transactionId: transactionMeta.id,
-              transactionIndex: nestedTransactionIndex,
-              transactionData,
-            }),
-          ),
-        );
-
-        for (const result of results) {
-          if (result.status === 'rejected') {
-            Logger.error(
-              result.reason as Error,
-              'Failed to update transaction pay amount in nested transaction',
-            );
-          }
-        }
       } catch (error) {
         Logger.error(error as Error, preparationErrorMessage);
+        throw error;
       }
+
+      await Promise.all(
+        updates.map(({ nestedTransactionIndex, transactionData }) =>
+          updateAtomicBatchData({
+            transactionId: transactionMeta.id,
+            transactionIndex: nestedTransactionIndex,
+            transactionData,
+          }),
+        ),
+      );
     },
     [transactionMeta],
   );
@@ -158,5 +151,6 @@ function syncMoneyAccountDepositRequiredAssets(
       error as Error,
       'Failed to sync Money Account deposit requiredAssets amount',
     );
+    throw error;
   }
 }