feat: Add simulation metrics to "Transaction Submitted" and "Transact…

…ion Finalized" events (#28240) ## **Description** This PR adds simulation metrics to the `Transaction Submitted` and `Transaction Finalized` events. Please refer to **Solution B)** below for details. ---- The challenge here is that there are two separate transaction fragment buckets: `transaction-added-<id>` and `transaction-submitted-<id>`. `transaction-added-<id>` fragments are associated with the events: - Transaction Added - Transaction Approved - Transaction Rejected `transaction-submitted-<id>` fragments are associated with the events: - Transaction Submitted - Transaction Finalized Simulation metrics were added to "Transaction Approved" and "Transaction Rejected" events using updateEventFragment. When they were added here through the useTransactionEventFragment hook, the `transaction-submitted-<id>` fragment had not yet been created. The `transaction-submitted-<id>` fragment is created after a user clicks the Confirm button on a transaction which invokes metrics#handleTransactionSubmitted and calls createTransactionEventFragment To solve this, I proposed two solutions (TLDR; went with Solution B): #### Solution A) - Create a single transaction fragment ID to handle all related transaction events. Here, the fragment would specify the props: ``` initialEvent: "Transaction Added" successEvent:"Transaction Finalized" failureEvent: "Transaction Rejected" ``` - Add new method to MetaMetricsController to call trackEvent with specified eventName utilizing fragment props. This would support calling "Transaction Approved" and "Transaction Submitted" ad-hoc style. #### Solution B) - Allow a premature fragment to exist for `transaction-submitted-<id>` fragment. Prior to this PR fragments only exist once they are created. - From useTransactionEventFragment hook - Adds a premature `transaction-submitted-<id>` fragment - Calls "updateEventFragment(`transaction-submitted-${transactionId}`, params)" to store simulation metric props - When handleTransactionSubmitted event is invoked, creating the fragment now merges the premature fragment with the newly created fragment - When a premature fragment is found as an abandoned fragment, we delete it from the store #### Solution C) - store unlinked props in new store state: fragmentsUnlinkedProps on updateEventFragment - fragmentsUnlinkedProps can be applied to newly created fragments on createEventFragment - deleted abandoned fragmentsUnlinkedProps would need more investigation I went with solution B due to time constraints. ## **Related issues** Fixes: MetaMask/MetaMask-planning#3507 ## **Manual testing steps** 1. Turn on "Participate in MetaMetrics" setting 2. Confirm or Reject a Send transaction 3. Observe simulation props are added to `Transaction Submitted` and `Transaction Finalized` events ## **Screenshots/Recordings**  no UI/UX changes ### **Before**  ### **After**  ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
MetaMask · Nov 6, 2024 · 08ec2ab · 08ec2ab
1 parent 7798ef4
commit 08ec2ab
Show file tree

Hide file tree

Showing 5 changed files with 280 additions and 22 deletions.
diff --git a/app/scripts/controllers/metametrics.test.ts b/app/scripts/controllers/metametrics.test.ts
@@ -10,6 +10,7 @@ import {
 import { InternalAccount } from '@metamask/keyring-api';
 import { Browser } from 'webextension-polyfill';
 import { Hex } from '@metamask/utils';
+import { merge } from 'lodash';
 import { ENVIRONMENT_TYPE_BACKGROUND } from '../../../shared/constants/app';
 import { createSegmentMock } from '../lib/segment';
 import {
@@ -93,8 +94,19 @@ const DEFAULT_PAGE_PROPERTIES = {
   ...DEFAULT_SHARED_PROPERTIES,
 };
 
-const SAMPLE_PERSISTED_EVENT = {
-  id: 'testid',
+const SAMPLE_TX_SUBMITTED_PARTIAL_FRAGMENT = {
+  id: 'transaction-submitted-0000',
+  canDeleteIfAbandoned: true,
+  category: 'Unit Test',
+  successEvent: 'Transaction Finalized',
+  persist: true,
+  properties: {
+    simulation_response: 'no_balance_change',
+    test_stored_prop: 1,
+  },
+};
+
+const SAMPLE_PERSISTED_EVENT_NO_ID = {
   persist: true,
   category: 'Unit Test',
   successEvent: 'sample persisted event success',
@@ -104,6 +116,11 @@ const SAMPLE_PERSISTED_EVENT = {
   },
 };
 
+const SAMPLE_PERSISTED_EVENT = {
+  id: 'testid',
+  ...SAMPLE_PERSISTED_EVENT_NO_ID,
+};
+
 const SAMPLE_NON_PERSISTED_EVENT = {
   id: 'testid2',
   persist: false,
@@ -255,6 +272,185 @@ describe('MetaMetricsController', function () {
     });
   });
 
+  describe('createEventFragment', function () {
+    it('should throw an error if the param is missing successEvent or category', async function () {
+      const metaMetricsController = getMetaMetricsController();
+
+      await expect(() => {
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-expect-error because we are testing the error case
+        metaMetricsController.createEventFragment({ event: 'test' });
+      }).toThrow(/Must specify success event and category\./u);
+
+      await expect(() => {
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-expect-error because we are testing the error case
+        metaMetricsController.createEventFragment({ category: 'test' });
+      }).toThrow(/Must specify success event and category\./u);
+    });
+
+    it('should update fragments state with new fragment', function () {
+      jest.useFakeTimers().setSystemTime(1730798301422);
+
+      const metaMetricsController = getMetaMetricsController();
+      const mockNewId = 'testid3';
+
+      metaMetricsController.createEventFragment({
+        ...SAMPLE_PERSISTED_EVENT_NO_ID,
+        uniqueIdentifier: mockNewId,
+      });
+
+      const resultFragment = metaMetricsController.state.fragments[mockNewId];
+
+      expect(resultFragment).toStrictEqual({
+        ...SAMPLE_PERSISTED_EVENT_NO_ID,
+        id: mockNewId,
+        uniqueIdentifier: mockNewId,
+        lastUpdated: 1730798301422,
+      });
+
+      jest.useRealTimers();
+    });
+
+    it('should track the initial event if provided', function () {
+      const metaMetricsController = getMetaMetricsController({
+        participateInMetaMetrics: true,
+      });
+      const spy = jest.spyOn(segmentMock, 'track');
+      const mockInitialEventName = 'Test Initial Event';
+
+      metaMetricsController.createEventFragment({
+        ...SAMPLE_PERSISTED_EVENT_NO_ID,
+        initialEvent: mockInitialEventName,
+      });
+
+      expect(spy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not call track if no initialEvent was provided', function () {
+      const metaMetricsController = getMetaMetricsController({
+        participateInMetaMetrics: true,
+      });
+      const spy = jest.spyOn(segmentMock, 'track');
+
+      metaMetricsController.createEventFragment({
+        ...SAMPLE_PERSISTED_EVENT_NO_ID,
+      });
+
+      expect(spy).toHaveBeenCalledTimes(0);
+    });
+
+    describe('when intialEvent is "Transaction Submitted" and a fragment exists before createEventFragment is called', function () {
+      it('should update existing fragment state with new fragment props', function () {
+        jest.useFakeTimers().setSystemTime(1730798302222);
+
+        const metaMetricsController = getMetaMetricsController();
+        const { id } = SAMPLE_TX_SUBMITTED_PARTIAL_FRAGMENT;
+
+        metaMetricsController.updateEventFragment(
+          SAMPLE_TX_SUBMITTED_PARTIAL_FRAGMENT.id,
+          {
+            ...SAMPLE_TX_SUBMITTED_PARTIAL_FRAGMENT,
+          },
+        );
+        metaMetricsController.createEventFragment({
+          ...SAMPLE_PERSISTED_EVENT_NO_ID,
+          initialEvent: 'Transaction Submitted',
+          uniqueIdentifier: id,
+        });
+
+        const resultFragment = metaMetricsController.state.fragments[id];
+        const expectedFragment = merge(
+          SAMPLE_TX_SUBMITTED_PARTIAL_FRAGMENT,
+          SAMPLE_PERSISTED_EVENT_NO_ID,
+          {
+            canDeleteIfAbandoned: false,
+            id,
+            initialEvent: 'Transaction Submitted',
+            uniqueIdentifier: id,
+            lastUpdated: 1730798302222,
+          },
+        );
+
+        expect(resultFragment).toStrictEqual(expectedFragment);
+
+        jest.useRealTimers();
+      });
+    });
+  });
+
+  describe('updateEventFragment', function () {
+    beforeEach(function () {
+      jest.useFakeTimers().setSystemTime(1730798303333);
+    });
+    afterEach(function () {
+      jest.useRealTimers();
+    });
+
+    it('updates fragment with additional provided props', async function () {
+      const metaMetricsController = getMetaMetricsController();
+      const MOCK_PROPS_TO_UPDATE = {
+        properties: {
+          test: 1,
+        },
+      };
+
+      metaMetricsController.updateEventFragment(
+        SAMPLE_PERSISTED_EVENT.id,
+        MOCK_PROPS_TO_UPDATE,
+      );
+
+      const resultFragment =
+        metaMetricsController.state.fragments[SAMPLE_PERSISTED_EVENT.id];
+      const expectedPartialFragment = {
+        ...SAMPLE_PERSISTED_EVENT,
+        ...MOCK_PROPS_TO_UPDATE,
+        lastUpdated: 1730798303333,
+      };
+      expect(resultFragment).toStrictEqual(expectedPartialFragment);
+    });
+
+    it('throws error when no existing fragment exists', async function () {
+      const metaMetricsController = getMetaMetricsController();
+
+      const MOCK_NONEXISTING_ID = 'test-nonexistingid';
+
+      await expect(() => {
+        metaMetricsController.updateEventFragment(MOCK_NONEXISTING_ID, {
+          properties: { test: 1 },
+        });
+      }).toThrow(/Event fragment with id test-nonexistingid does not exist\./u);
+    });
+
+    describe('when id includes "transaction-submitted"', function () {
+      it('creates and stores new fragment props with canDeleteIfAbandoned set to true', function () {
+        const metaMetricsController = getMetaMetricsController();
+        const MOCK_ID = 'transaction-submitted-1111';
+        const MOCK_PROPS_TO_UPDATE = {
+          properties: {
+            test: 1,
+          },
+        };
+
+        metaMetricsController.updateEventFragment(
+          MOCK_ID,
+          MOCK_PROPS_TO_UPDATE,
+        );
+
+        const resultFragment = metaMetricsController.state.fragments[MOCK_ID];
+        const expectedPartialFragment = {
+          ...MOCK_PROPS_TO_UPDATE,
+          category: 'Transactions',
+          canDeleteIfAbandoned: true,
+          id: MOCK_ID,
+          lastUpdated: 1730798303333,
+          successEvent: 'Transaction Finalized',
+        };
+        expect(resultFragment).toStrictEqual(expectedPartialFragment);
+      });
+    });
+  });
+
   describe('generateMetaMetricsId', function () {
     it('should generate an 0x prefixed hex string', function () {
       const metaMetricsController = getMetaMetricsController();

diff --git a/app/scripts/controllers/metametrics.ts b/app/scripts/controllers/metametrics.ts
@@ -32,6 +32,7 @@ import { ENVIRONMENT_TYPE_BACKGROUND } from '../../../shared/constants/app';
 import {
   METAMETRICS_ANONYMOUS_ID,
   METAMETRICS_BACKGROUND_PAGE_OBJECT,
+  MetaMetricsEventCategory,
   MetaMetricsEventName,
   MetaMetricsEventFragment,
   MetaMetricsUserTrait,
@@ -312,7 +313,7 @@ export default class MetaMetricsController {
     // fragments that are not marked as persistent will be purged and the
     // failure event will be emitted.
     Object.values(abandonedFragments).forEach((fragment) => {
-      this.finalizeEventFragment(fragment.id, { abandoned: true });
+      this.processAbandonedFragment(fragment);
     });
 
     // Code below submits any pending segmentApiCalls to Segment if/when the controller is re-instantiated
@@ -368,7 +369,7 @@ export default class MetaMetricsController {
         fragment.lastUpdated &&
         Date.now() - fragment.lastUpdated / 1000 > fragment.timeout
       ) {
-        this.finalizeEventFragment(fragment.id, { abandoned: true });
+        this.processAbandonedFragment(fragment);
       }
     });
   }
@@ -414,10 +415,30 @@ export default class MetaMetricsController {
       ...options,
       lastUpdated: Date.now(),
     };
+
+    /**
+     * HACK: "transaction-submitted-<id>" fragment hack
+     * A "transaction-submitted-<id>" fragment may exist following the "Transaction Added"
+     * event to persist accumulated event fragment props to the "Transaction Submitted" event
+     * which fires after a user confirms a transaction. Rejecting a confirmation does not fire the
+     * "Transaction Submitted" event. In this case, these abandoned fragments will be deleted
+     * instead of finalized with canDeleteIfAbandoned set to true.
+     */
+    const hasExistingSubmittedFragment =
+      options.initialEvent === TransactionMetaMetricsEvent.submitted &&
+      fragments[id];
+
+    const additionalFragmentProps = hasExistingSubmittedFragment
+      ? {
+          ...fragments[id],
+          canDeleteIfAbandoned: false,
+        }
+      : {};
+
     this.store.updateState({
       fragments: {
         ...fragments,
-        [id]: fragment,
+        [id]: merge(additionalFragmentProps, fragment),
       },
     });
 
@@ -455,6 +476,19 @@ export default class MetaMetricsController {
     return fragment;
   }
 
+  /**
+   * Deletes to finalizes event fragment based on the canDeleteIfAbandoned property.
+   *
+   * @param fragment
+   */
+  processAbandonedFragment(fragment: MetaMetricsEventFragment): void {
+    if (fragment.canDeleteIfAbandoned) {
+      this.deleteEventFragment(fragment.id);
+    } else {
+      this.finalizeEventFragment(fragment.id, { abandoned: true });
+    }
+  }
+
   /**
    * Updates an event fragment in state
    *
@@ -469,7 +503,22 @@ export default class MetaMetricsController {
 
     const fragment = fragments[id];
 
-    if (!fragment) {
+    /**
+     * HACK: "transaction-submitted-<id>" fragment hack
+     * Creates a "transaction-submitted-<id>" fragment if it does not exist to persist
+     * accumulated event metrics. In the case it is unused, the abandoned fragment will
+     * eventually be deleted with canDeleteIfAbandoned set to true.
+     */
+    const createIfNotFound = !fragment && id.includes('transaction-submitted-');
+
+    if (createIfNotFound) {
+      fragments[id] = {
+        canDeleteIfAbandoned: true,
+        category: MetaMetricsEventCategory.Transactions,
+        successEvent: TransactionMetaMetricsEvent.finalized,
+        id,
+      };
+    } else if (!fragment) {
       throw new Error(`Event fragment with id ${id} does not exist.`);
     }
 
@@ -484,6 +533,19 @@ export default class MetaMetricsController {
     });
   }
 
+  /**
+   * Deletes an event fragment from state
+   *
+   * @param id - The fragment id to delete
+   */
+  deleteEventFragment(id: string): void {
+    const { fragments } = this.store.getState();
+
+    if (fragments[id]) {
+      delete fragments[id];
+    }
+  }
+
   /**
    * Finalizes a fragment, tracking either a success event or failure Event
    * and then removes the fragment from state.