Transaction updates methods return whole tx meta (#14147)

* Transaction updates methods return whole tx meta

* Fix unit test

Co-authored-by: Dan Miller <danjm@pop-os.localdomain>

app/scripts/controllers/transactions/index.js

@@ -373,6 +373,7 @@ export default class TransactionController extends EventEmitter {
   }
 
   /**
+   * updates the params that are editible in the send edit flow
    *
    * @param {string} txId - transaction id
    * @param {object} editableParams - holds the editable parameters
@@ -380,10 +381,13 @@ export default class TransactionController extends EventEmitter {
    * @param {string} editableParams.from
    * @param {string} editableParams.to
    * @param {string} editableParams.value
+   * @returns {TransactionMeta} the txMeta of the updated transaction
    */
   updateEditableParams(txId, { data, from, to, value }) {
     if (!this._checkIfTxStatusIsUnapproved(txId)) {
-      return;
+      throw new Error(
+        'Cannot call updateEditableParams on a transaction that is not in an unapproved state',
+      );
     }
 
     const editableParams = {
@@ -399,6 +403,7 @@ export default class TransactionController extends EventEmitter {
     editableParams.txParams = pickBy(editableParams.txParams);
     const note = `Update Editable Params for ${txId}`;
     this._updateTransaction(txId, editableParams, note);
+    return this._getTransaction(txId);
   }
 
   /**
@@ -415,6 +420,7 @@ export default class TransactionController extends EventEmitter {
    * @param {string} txGasFees.defaultGasEstimates
    * @param {string} txGasFees.gas
    * @param {string} txGasFees.originalGasEstimate
+   * @returns {TransactionMeta} the txMeta of the updated transaction
    */
   updateTransactionGasFees(
     txId,
@@ -431,7 +437,9 @@ export default class TransactionController extends EventEmitter {
     },
   ) {
     if (!this._checkIfTxStatusIsUnapproved(txId)) {
-      return;
+      throw new Error(
+        'Cannot call updateTransactionGasFees on a transaction that is not in an unapproved state',
+      );
     }
 
     let txGasFees = {
@@ -453,6 +461,7 @@ export default class TransactionController extends EventEmitter {
     txGasFees = pickBy(txGasFees);
     const note = `Update Transaction Gas Fees for ${txId}`;
     this._updateTransaction(txId, txGasFees, note);
+    return this._getTransaction(txId);
   }
 
   /**
@@ -462,13 +471,16 @@ export default class TransactionController extends EventEmitter {
    * @param {object} txEstimateBaseFees - holds the estimate base fees parameters
    * @param {string} txEstimateBaseFees.estimatedBaseFee
    * @param {string} txEstimateBaseFees.decEstimatedBaseFee
+   * @returns {TransactionMeta} the txMeta of the updated transaction
    */
   updateTransactionEstimatedBaseFee(
     txId,
     { estimatedBaseFee, decEstimatedBaseFee },
   ) {
     if (!this._checkIfTxStatusIsUnapproved(txId)) {
-      return;
+      throw new Error(
+        'Cannot call updateTransactionEstimatedBaseFee on a transaction that is not in an unapproved state',
+      );
     }
 
     let txEstimateBaseFees = { estimatedBaseFee, decEstimatedBaseFee };
@@ -477,6 +489,7 @@ export default class TransactionController extends EventEmitter {
 
     const note = `Update Transaction Estimated Base Fees for ${txId}`;
     this._updateTransaction(txId, txEstimateBaseFees, note);
+    return this._getTransaction(txId);
   }
 
   /**
@@ -487,10 +500,13 @@ export default class TransactionController extends EventEmitter {
    * @param {object} swapApprovalTransaction - holds the metadata and token symbol
    * @param {string} swapApprovalTransaction.type
    * @param {string} swapApprovalTransaction.sourceTokenSymbol
+   * @returns {TransactionMeta} the txMeta of the updated transaction
    */
   updateSwapApprovalTransaction(txId, { type, sourceTokenSymbol }) {
     if (!this._checkIfTxStatusIsUnapproved(txId)) {
-      return;
+      throw new Error(
+        'Cannot call updateSwapApprovalTransaction on a transaction that is not in an unapproved state',
+      );
     }
 
     let swapApprovalTransaction = { type, sourceTokenSymbol };
@@ -499,6 +515,7 @@ export default class TransactionController extends EventEmitter {
 
     const note = `Update Swap Approval Transaction for ${txId}`;
     this._updateTransaction(txId, swapApprovalTransaction, note);
+    return this._getTransaction(txId);
   }
 
   /**
@@ -516,6 +533,7 @@ export default class TransactionController extends EventEmitter {
    * @param {string} swapTransaction.swapTokenValue
    * @param {string} swapTransaction.estimatedBaseFee
    * @param {string} swapTransaction.approvalTxId
+   * @returns {TransactionMeta} the txMeta of the updated transaction
    */
   updateSwapTransaction(
     txId,
@@ -532,7 +550,9 @@ export default class TransactionController extends EventEmitter {
     },
   ) {
     if (!this._checkIfTxStatusIsUnapproved(txId)) {
-      return;
+      throw new Error(
+        'Cannot call updateSwapTransaction on a transaction that is not in an unapproved state',
+      );
     }
     let swapTransaction = {
       sourceTokenSymbol,
@@ -551,6 +571,7 @@ export default class TransactionController extends EventEmitter {
 
     const note = `Update Swap Transaction for ${txId}`;
     this._updateTransaction(txId, swapTransaction, note);
+    return this._getTransaction(txId);
   }
 
   /**
@@ -560,10 +581,13 @@ export default class TransactionController extends EventEmitter {
    * @param {object} userSettings - holds the metadata
    * @param {string} userSettings.userEditedGasLimit
    * @param {string} userSettings.userFeeLevel
+   * @returns {TransactionMeta} the txMeta of the updated transaction
    */
   updateTransactionUserSettings(txId, { userEditedGasLimit, userFeeLevel }) {
     if (!this._checkIfTxStatusIsUnapproved(txId)) {
-      return;
+      throw new Error(
+        'Cannot call updateTransactionUserSettings on a transaction that is not in an unapproved state',
+      );
     }
 
     let userSettings = { userEditedGasLimit, userFeeLevel };
@@ -572,6 +596,7 @@ export default class TransactionController extends EventEmitter {
 
     const note = `Update User Settings for ${txId}`;
     this._updateTransaction(txId, userSettings, note);
+    return this._getTransaction(txId);
   }
 
   // ====================================================================================================================================================

app/scripts/controllers/transactions/index.test.js

@@ -2170,7 +2170,7 @@ describe('Transaction Controller', function () {
       assert.equal(result.userFeeLevel, 'high');
     });
 
-    it('does not update if status is not unapproved', function () {
+    it('throws error if status is not unapproved', function () {
       txStateManager.addTransaction({
         id: '4',
         status: TRANSACTION_STATUSES.APPROVED,
@@ -2184,14 +2184,14 @@ describe('Transaction Controller', function () {
         estimateUsed: '0x009',
       });
 
-      txController.updateTransactionGasFees('4', { maxFeePerGas: '0x0088' });
+      try {
-      let result = txStateManager.getTransaction('4');
+        txController.updateTransactionGasFees('4', { maxFeePerGas: '0x0088' });
-      assert.equal(result.txParams.maxFeePerGas, '0x008');
+      } catch (e) {
-
+        assert.equal(
-      // test update estimate used
+          e.message,
-      txController.updateTransactionGasFees('4', { estimateUsed: '0x0099' });
+          'Cannot call updateTransactionGasFees on a transaction that is not in an unapproved state',
-      result = txStateManager.getTransaction('4');
+        );
-      assert.equal(result.estimateUsed, '0x009');
+      }
     });
 
     it('does not update unknown parameters in update method', function () {

ui/store/actions.js

@@ -673,8 +673,9 @@ const updateMetamaskStateFromBackground = () => {
 
 export function updateSwapApprovalTransaction(txId, txSwapApproval) {
   return async (dispatch) => {
+    let updatedTransaction;
     try {
-      await promisifiedBackground.updateSwapApprovalTransaction(
+      updatedTransaction = await promisifiedBackground.updateSwapApprovalTransaction(
         txId,
         txSwapApproval,
       );
@@ -685,14 +686,18 @@ export function updateSwapApprovalTransaction(txId, txSwapApproval) {
       throw error;
     }
 
-    return txSwapApproval;
+    return updatedTransaction;
   };
 }
 
 export function updateEditableParams(txId, editableParams) {
   return async (dispatch) => {
+    let updatedTransaction;
     try {
-      await promisifiedBackground.updateEditableParams(txId, editableParams);
+      updatedTransaction = await promisifiedBackground.updateEditableParams(
+        txId,
+        editableParams,
+      );
     } catch (error) {
       dispatch(txError(error));
       dispatch(goHome());
@@ -700,14 +705,18 @@ export function updateEditableParams(txId, editableParams) {
       throw error;
     }
 
-    return editableParams;
+    return updatedTransaction;
   };
 }
 
 export function updateTransactionGasFees(txId, txGasFees) {
   return async (dispatch) => {
+    let updatedTransaction;
     try {
-      await promisifiedBackground.updateTransactionGasFees(txId, txGasFees);
+      updatedTransaction = await promisifiedBackground.updateTransactionGasFees(
+        txId,
+        txGasFees,
+      );
     } catch (error) {
       dispatch(txError(error));
       dispatch(goHome());
@@ -715,14 +724,18 @@ export function updateTransactionGasFees(txId, txGasFees) {
       throw error;
     }
 
-    return txGasFees;
+    return updatedTransaction;
   };
 }
 
 export function updateSwapTransaction(txId, txSwap) {
   return async (dispatch) => {
+    let updatedTransaction;
     try {
-      await promisifiedBackground.updateSwapTransaction(txId, txSwap);
+      updatedTransaction = await promisifiedBackground.updateSwapTransaction(
+        txId,
+        txSwap,
+      );
     } catch (error) {
       dispatch(txError(error));
       dispatch(goHome());
@@ -730,7 +743,7 @@ export function updateSwapTransaction(txId, txSwap) {
       throw error;
     }
 
-    return txSwap;
+    return updatedTransaction;
   };
 }