fix: transaction pay quote refresh (#7020)

## Explanation

Ensure `isLoading` is also cleared after refreshing quotes. 

## References

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs),
highlighting breaking changes as necessary
- [x] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes

Author: matthewwalsh0

packages/transaction-pay-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Prevent infinite loading after quotes are refreshed ([#7020](https://github.com/MetaMask/core/pull/7020))
+
 ## [2.0.1]
 
 ### Fixed

packages/transaction-pay-controller/src/TransactionPayController.test.ts

@@ -119,7 +119,7 @@ describe('TransactionPayController', () => {
       expect(
         controller.state.transactionData[TRANSACTION_ID_MOCK],
       ).toStrictEqual({
-        isLoading: true,
+        isLoading: false,
         sourceAmounts: [{ sourceAmountHuman: '1.23' }],
         tokens: [],
       });

packages/transaction-pay-controller/src/TransactionPayController.ts

@@ -113,7 +113,6 @@ export class TransactionPayController extends BaseController<
         updateSourceAmounts(transactionId, current as never, this.messenger);
 
         shouldUpdateQuotes = true;
-        current.isLoading = true;
       }
     });
 
@@ -123,13 +122,7 @@ export class TransactionPayController extends BaseController<
         transactionData: this.state.transactionData[transactionId],
         transactionId,
         updateTransactionData: this.#updateTransactionData.bind(this),
-      })
-        .finally(() => {
-          this.#updateTransactionData(transactionId, (data) => {
-            data.isLoading = false;
-          });
-        })
-        .catch(noop);
+      }).catch(noop);
     }
   }
 

packages/transaction-pay-controller/src/utils/quotes.test.ts

@@ -133,7 +133,9 @@ describe('Quotes Utils', () => {
 
       const transactionDataMock = {};
 
-      updateTransactionDataMock.mock.calls[0][1](transactionDataMock);
+      updateTransactionDataMock.mock.calls.map((call) =>
+        call[1](transactionDataMock),
+      );
 
       expect(transactionDataMock).toMatchObject({
         quotes: [QUOTE_MOCK],
@@ -152,7 +154,9 @@ describe('Quotes Utils', () => {
         quotes: [QUOTE_MOCK],
       };
 
-      updateTransactionDataMock.mock.calls[0][1](transactionDataMock);
+      updateTransactionDataMock.mock.calls.map((call) =>
+        call[1](transactionDataMock),
+      );
 
       expect(transactionDataMock).toMatchObject({
         quotes: [],
@@ -174,7 +178,9 @@ describe('Quotes Utils', () => {
         quotes: [QUOTE_MOCK],
       };
 
-      updateTransactionDataMock.mock.calls[0][1](transactionDataMock);
+      updateTransactionDataMock.mock.calls.map((call) =>
+        call[1](transactionDataMock),
+      );
 
       expect(transactionDataMock).toMatchObject({
         quotes: [],
@@ -243,7 +249,9 @@ describe('Quotes Utils', () => {
 
       const transactionDataMock = {};
 
-      updateTransactionDataMock.mock.calls[0][1](transactionDataMock);
+      updateTransactionDataMock.mock.calls.map((call) =>
+        call[1](transactionDataMock),
+      );
 
       expect(transactionDataMock).toStrictEqual(
         expect.objectContaining({ totals: TOTALS_MOCK }),
@@ -297,11 +305,12 @@ describe('Quotes Utils', () => {
 
       await refreshQuotes(messenger, updateTransactionDataMock);
 
-      expect(updateTransactionDataMock).toHaveBeenCalledTimes(2);
+      expect(updateTransactionDataMock).toHaveBeenCalledTimes(3);
 
       const transactionDataMock = {};
-      updateTransactionDataMock.mock.calls[0][1](transactionDataMock);
-      updateTransactionDataMock.mock.calls[1][1](transactionDataMock);
+      updateTransactionDataMock.mock.calls.map((call) =>
+        call[1](transactionDataMock),
+      );
 
       expect(transactionDataMock).toMatchObject({
         quotes: [],
@@ -321,11 +330,12 @@ describe('Quotes Utils', () => {
 
       await refreshQuotes(messenger, updateTransactionDataMock);
 
-      expect(updateTransactionDataMock).toHaveBeenCalledTimes(2);
+      expect(updateTransactionDataMock).toHaveBeenCalledTimes(3);
 
       const transactionDataMock = {};
-      updateTransactionDataMock.mock.calls[0][1](transactionDataMock);
-      updateTransactionDataMock.mock.calls[1][1](transactionDataMock);
+      updateTransactionDataMock.mock.calls.map((call) =>
+        call[1](transactionDataMock),
+      );
 
       expect(transactionDataMock).toMatchObject({
         quotes: [],

packages/transaction-pay-controller/src/utils/quotes.ts

@@ -75,49 +75,59 @@ export async function updateQuotes(request: UpdateQuotesRequest) {
 
   let quotes: TransactionPayQuote<Json>[] | undefined = [];
 
-  const strategy = await getStrategy(messenger as never, transaction);
+  updateTransactionData(transactionId, (data) => {
+    data.isLoading = true;
+  });
 
   try {
-    quotes = requests?.length
-      ? ((await strategy.getQuotes({
-          messenger,
-          requests,
-          transaction,
-        })) as TransactionPayQuote<Json>[])
-      : [];
-  } catch (error) {
-    log('Error fetching quotes', { error, transactionId });
-  }
+    const strategy = await getStrategy(messenger as never, transaction);
+
+    try {
+      quotes = requests?.length
+        ? ((await strategy.getQuotes({
+            messenger,
+            requests,
+            transaction,
+          })) as TransactionPayQuote<Json>[])
+        : [];
+    } catch (error) {
+      log('Error fetching quotes', { error, transactionId });
+    }
 
-  log('Updated', { transactionId, quotes });
+    log('Updated', { transactionId, quotes });
 
-  const batchTransactions =
-    quotes?.length && strategy.getBatchTransactions
-      ? await strategy.getBatchTransactions({
-          messenger,
-          quotes,
-        })
-      : [];
+    const batchTransactions =
+      quotes?.length && strategy.getBatchTransactions
+        ? await strategy.getBatchTransactions({
+            messenger,
+            quotes,
+          })
+        : [];
 
-  log('Batch transactions', { transactionId, batchTransactions });
+    log('Batch transactions', { transactionId, batchTransactions });
 
-  const totals = calculateTotals(quotes as never, tokens, messenger);
+    const totals = calculateTotals(quotes as never, tokens, messenger);
 
-  log('Calculated totals', { transactionId, totals });
+    log('Calculated totals', { transactionId, totals });
 
-  syncTransaction({
-    batchTransactions,
-    messenger: messenger as never,
-    paymentToken,
-    totals,
-    transactionId,
-  });
+    syncTransaction({
+      batchTransactions,
+      messenger: messenger as never,
+      paymentToken,
+      totals,
+      transactionId,
+    });
 
-  updateTransactionData(transactionId, (data) => {
-    data.quotes = quotes as never;
-    data.quotesLastUpdated = Date.now();
-    data.totals = totals;
-  });
+    updateTransactionData(transactionId, (data) => {
+      data.quotes = quotes as never;
+      data.quotesLastUpdated = Date.now();
+      data.totals = totals;
+    });
+  } finally {
+    updateTransactionData(transactionId, (data) => {
+      data.isLoading = false;
+    });
+  }
 }
 
 /**
@@ -206,10 +216,6 @@ export async function refreshQuotes(
       refreshInterval,
     });
 
-    updateTransactionData(transactionId, (data) => {
-      data.isLoading = true;
-    });
-
     await updateQuotes({
       messenger,
       transactionData,