From 8d79e285604407ad008901290e9e7fffa2b6343f Mon Sep 17 00:00:00 2001
From: Ariella Vu <20778143+digiwand@users.noreply.github.com>
Date: Thu, 1 Jun 2023 16:10:26 +0200
Subject: [PATCH] Clean ConfirmTransaction page related code (#19208)

* fix typo: uncofirmedTransactions -> unconfirmedTransactions

* txHelper: reorganize lines and improve test

* ConfirmTx: rn unconfirmedTxs->unconfirmedTxsSorted

* ConfirmTx: rn unconfirmedMessages-> unconfirmedTxs
---
 ...irm-page-container-navigation.component.js |  6 ++--
 ui/helpers/utils/tx-helper.test.js            | 16 ++++++---
 ui/helpers/utils/tx-helper.ts                 | 34 +++++++++----------
 .../confirm-transaction.component.js          | 14 ++++----
 4 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/ui/components/app/confirm-page-container/confirm-page-container-navigation/confirm-page-container-navigation.component.js b/ui/components/app/confirm-page-container/confirm-page-container-navigation/confirm-page-container-navigation.component.js
index f60c14318..d5a29aec4 100755
--- a/ui/components/app/confirm-page-container/confirm-page-container-navigation/confirm-page-container-navigation.component.js
+++ b/ui/components/app/confirm-page-container/confirm-page-container-navigation/confirm-page-container-navigation.component.js
@@ -23,7 +23,7 @@ const ConfirmPageContainerNavigation = () => {
   const unapprovedEncryptionPublicKeyMsgs = useSelector(
     unapprovedEncryptionPublicKeyMsgsSelector,
   );
-  const uncofirmedTransactions = useSelector(
+  const unconfirmedTransactions = useSelector(
     unconfirmedTransactionsHashSelector,
   );
 
@@ -36,7 +36,7 @@ const ConfirmPageContainerNavigation = () => {
     ...enumUnapprovedEncryptMsgsKey,
   ];
 
-  const enumUnapprovedTxs = Object.keys(uncofirmedTransactions).filter(
+  const enumUnapprovedTxs = Object.keys(unconfirmedTransactions).filter(
     (key) => enumDecryptAndEncryptMsgs.includes(key) === false,
   );
 
@@ -54,7 +54,7 @@ const ConfirmPageContainerNavigation = () => {
     if (txId) {
       dispatch(clearConfirmTransaction());
       history.push(
-        uncofirmedTransactions[txId]?.msgParams
+        unconfirmedTransactions[txId]?.msgParams
           ? `${CONFIRM_TRANSACTION_ROUTE}/${txId}${SIGNATURE_REQUEST_PATH}`
           : `${CONFIRM_TRANSACTION_ROUTE}/${txId}`,
       );
diff --git a/ui/helpers/utils/tx-helper.test.js b/ui/helpers/utils/tx-helper.test.js
index c48a67d03..579ffbf0d 100644
--- a/ui/helpers/utils/tx-helper.test.js
+++ b/ui/helpers/utils/tx-helper.test.js
@@ -5,15 +5,21 @@ describe('txHelper', () => {
   it('always shows the oldest tx first', () => {
     const metamaskNetworkId = NETWORK_IDS.MAINNET;
     const chainId = CHAIN_IDS.MAINNET;
-    const txs = {
+    const mockUnapprovedTxs = {
       a: { metamaskNetworkId, time: 3 },
-      b: { metamaskNetworkId, time: 1 },
+      b: { metamaskNetworkId, time: 6 },
       c: { metamaskNetworkId, time: 2 },
     };
 
+    const mockUnapprovedMsgs = {
+      d: { metamaskNetworkId, time: 4 },
+      e: { metamaskNetworkId, time: 1 },
+      f: { metamaskNetworkId, time: 5 },
+    };
+
     const sorted = txHelper(
-      txs,
-      null,
+      mockUnapprovedTxs,
+      mockUnapprovedMsgs,
       null,
       null,
       null,
@@ -21,7 +27,9 @@ describe('txHelper', () => {
       metamaskNetworkId,
       chainId,
     );
+
     expect(sorted[0].time).toStrictEqual(1);
     expect(sorted[2].time).toStrictEqual(3);
+    expect(sorted[5].time).toStrictEqual(6);
   });
 });
diff --git a/ui/helpers/utils/tx-helper.ts b/ui/helpers/utils/tx-helper.ts
index e29732d24..1ce48a75e 100644
--- a/ui/helpers/utils/tx-helper.ts
+++ b/ui/helpers/utils/tx-helper.ts
@@ -29,35 +29,33 @@ export default function txHelper(
         transactionMatchesNetwork(txMeta, chainId, networkId),
       )
     : valuesFor(unapprovedTxs);
-  log.debug(`tx helper found ${txValues.length} unapproved txs`);
 
   const msgValues = valuesFor(unapprovedMsgs);
-  log.debug(`tx helper found ${msgValues.length} unsigned messages`);
-  let allValues = txValues.concat(msgValues);
-
   const personalValues = valuesFor(personalMsgs);
+  const decryptValues = valuesFor(decryptMsgs);
+  const encryptionPublicKeyValues = valuesFor(encryptionPublicKeyMsgs);
+  const typedValues = valuesFor(typedMessages);
+
+  const allValues = txValues
+    .concat(msgValues)
+    .concat(personalValues)
+    .concat(decryptValues)
+    .concat(encryptionPublicKeyValues)
+    .concat(typedValues)
+    .sort((a, b) => {
+      return a.time - b.time;
+    });
+
+  log.debug(`tx helper found ${txValues.length} unapproved txs`);
+  log.debug(`tx helper found ${msgValues.length} unsigned messages`);
   log.debug(
     `tx helper found ${personalValues.length} unsigned personal messages`,
   );
-  allValues = allValues.concat(personalValues);
-
-  const decryptValues = valuesFor(decryptMsgs);
   log.debug(`tx helper found ${decryptValues.length} decrypt requests`);
-  allValues = allValues.concat(decryptValues);
-
-  const encryptionPublicKeyValues = valuesFor(encryptionPublicKeyMsgs);
   log.debug(
     `tx helper found ${encryptionPublicKeyValues.length} encryptionPublicKey requests`,
   );
-  allValues = allValues.concat(encryptionPublicKeyValues);
-
-  const typedValues = valuesFor(typedMessages);
   log.debug(`tx helper found ${typedValues.length} unsigned typed messages`);
-  allValues = allValues.concat(typedValues);
-
-  allValues = allValues.sort((a, b) => {
-    return a.time - b.time;
-  });
 
   return allValues;
 }
diff --git a/ui/pages/confirm-transaction/confirm-transaction.component.js b/ui/pages/confirm-transaction/confirm-transaction.component.js
index 698a729fe..16b8d502b 100644
--- a/ui/pages/confirm-transaction/confirm-transaction.component.js
+++ b/ui/pages/confirm-transaction/confirm-transaction.component.js
@@ -57,22 +57,22 @@ const ConfirmTransaction = () => {
   const mostRecentOverviewPage = useSelector(getMostRecentOverviewPage);
   const sendTo = useSelector(getSendTo);
   const unapprovedTxs = useSelector(getUnapprovedTransactions);
-  const unconfirmedTxs = useSelector(unconfirmedTransactionsListSelector);
-  const unconfirmedMessages = useSelector(unconfirmedTransactionsHashSelector);
+  const unconfirmedTxsSorted = useSelector(unconfirmedTransactionsListSelector);
+  const unconfirmedTxs = useSelector(unconfirmedTransactionsHashSelector);
 
-  const totalUnapproved = unconfirmedTxs.length || 0;
+  const totalUnapproved = unconfirmedTxsSorted.length || 0;
   const getTransaction = useCallback(() => {
     return totalUnapproved
       ? unapprovedTxs[paramsTransactionId] ||
-          unconfirmedMessages[paramsTransactionId] ||
-          unconfirmedTxs[0]
+          unconfirmedTxs[paramsTransactionId] ||
+          unconfirmedTxsSorted[0]
       : {};
   }, [
     paramsTransactionId,
     totalUnapproved,
     unapprovedTxs,
-    unconfirmedMessages,
     unconfirmedTxs,
+    unconfirmedTxsSorted,
   ]);
   const [transaction, setTransaction] = useState(getTransaction);
 
@@ -88,8 +88,8 @@ const ConfirmTransaction = () => {
     paramsTransactionId,
     totalUnapproved,
     unapprovedTxs,
-    unconfirmedMessages,
     unconfirmedTxs,
+    unconfirmedTxsSorted,
   ]);
 
   const { id, type } = transaction;