feat: Add `ui_customizations` metric for transactions #25736

pedronfigueiredo · 2024-07-10T11:02:53Z

Description

This PR also includes an e2e test, and migrates existing transaction redesign e2e tests to TypeScript.

Related issues

Fixes: #24486

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-07-10T11:03:09Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

OGPoyraz · 2024-07-10T12:31:51Z

test/e2e/tests/confirmations/transactions/contract-interaction-redesign.spec.ts

@@ -256,6 +311,7 @@ describe('Confirmation Redesign Contract Interaction Component', function () {
        },
      );
    });
+<<<<<<< Updated upstream:test/e2e/tests/confirmations/transactions/contract-interaction-redesign.spec.js


Guess this is forgotten.

OGPoyraz · 2024-07-10T12:32:28Z

test/e2e/tests/confirmations/transactions/contract-interaction-redesign.spec.ts

+        }: {
+          driver: Driver;
+          contractRegistry: GanacheContractAddressRegistry;
+        }) => {


Nit: we could've create a small interface for this repetitive argument type.

OGPoyraz · 2024-07-10T12:33:00Z

test/e2e/tests/confirmations/transactions/contract-interaction-redesign.spec.ts

@@ -402,3 +458,97 @@ async function assertAdvancedGasDetailsWithL2Breakdown(driver) {
  await driver.waitForSelector({ css: 'p', text: 'Speed' });
  await driver.waitForSelector({ css: 'p', text: 'Max fee' });
 }
+=======


Missed this one as well.

OGPoyraz · 2024-07-10T12:33:06Z

test/e2e/tests/confirmations/transactions/contract-interaction-redesign.spec.ts

+    });
+  });
+});
+>>>>>>> Stashed changes:test/e2e/tests/confirmations/transactions/contract-interaction-redesign.spec.ts


jpuri · 2024-07-10T16:02:51Z

app/scripts/lib/transaction/metrics.ts

+    uiCustomizations.push(
+      MetaMetricsEventUiCustomization.RedesignedConfirmation,
+    );
+  }


I think it should also include check to see if user has enabled re-designs

added in 50e5516

matthewwalsh0 · 2024-07-10T20:12:30Z

app/scripts/lib/transaction/metrics.ts

@@ -978,6 +979,13 @@ async function buildEventFragmentProperties({
    uiCustomizations.push(MetaMetricsEventUiCustomization.GasEstimationFailed);
  }

+  if (
+    REDESIGN_TRANSACTION_TYPES.includes(transactionMeta.type as TransactionType)


Following the developer settings PR, this will be static and not based on the ENV, so do we also want to temporarily check the env / preference here? @pnarayanaswamy

Depends which PR goes in first.

added in 50e5516

matthewwalsh0 · 2024-07-10T20:14:05Z

test/e2e/tests/confirmations/transactions/contract-interaction-redesign.spec.ts

-            smartContract,
-          );
+        async ({ driver, contractRegistry }: TestSuiteArguments) => {
+          const contractAddress = await (


Minor, is this worth a small function to avoid the duplication?

Added in c326fdb8d3

matthewwalsh0 · 2024-07-10T20:14:50Z

test/e2e/tests/confirmations/transactions/metrics.spec.ts

+} = require('../../../helpers');
+const FixtureBuilder = require('../../../fixture-builder');
+
+describe('Metrics', function () {


Since integration tests are now merged, would this be a great use case for one, rather than adding more E2E tests for functionality that won't directly impact the user?

I agree this is a use case, but I would prefer to avoid deleting working e2e tests in this PR

sonarqubecloud · 2024-07-12T10:13:40Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-07-12T10:51:21Z

Builds ready [cd1c1d8]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (63 ± 7 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	64	128	96	18	9
		domContentLoaded	9	58	27	15	7
		load	39	90	63	15	7
		domInteractive	9	58	27	15	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 407 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

codecov · 2024-07-12T10:51:57Z

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 69.96%. Comparing base (5ee57a6) to head (cd1c1d8).
Report is 35 commits behind head on develop.

Files	Patch %	Lines
app/scripts/lib/transaction/metrics.ts	71.43%	2 Missing ⚠️
app/scripts/metamask-controller.js	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25736      +/-   ##
===========================================
+ Coverage    69.77%   69.96%   +0.18%     
===========================================
  Files         1376     1390      +14     
  Lines        48403    48904     +501     
  Branches     13348    13455     +107     
===========================================
+ Hits         33773    34212     +439     
- Misses       14630    14692      +62

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

all feedback has been addressed

metamaskbot · 2024-08-19T15:12:09Z

Missing release label release-12.2.0 on PR. Adding release label release-12.2.0 on PR and removing other release labels(release-12.3.0), as PR was added to branch 12.2.0 when release was cut.

pedronfigueiredo added confirmation-redesign confirmation-re-design team-confirmations Push issues to confirmations team labels Jul 10, 2024

pedronfigueiredo self-assigned this Jul 10, 2024

pedronfigueiredo requested a review from a team as a code owner July 10, 2024 11:02

pedronfigueiredo force-pushed the pnf/24486 branch from c933486 to 42b7b28 Compare July 10, 2024 11:09

OGPoyraz reviewed Jul 10, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/24486 branch from 099f087 to 0d8b061 Compare July 10, 2024 13:45

jpuri reviewed Jul 10, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/24486 branch from 0d8b061 to 49e33f9 Compare July 10, 2024 16:03

matthewwalsh0 previously requested changes Jul 10, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/24486 branch 2 times, most recently from 53533cf to 3aeb614 Compare July 11, 2024 13:23

jpuri previously approved these changes Jul 12, 2024

View reviewed changes

pedronfigueiredo dismissed jpuri’s stale review via 9d62aa9 July 12, 2024 08:56

pedronfigueiredo added 6 commits July 12, 2024 09:56

feat: Add ui_customizations metric for transactions

5886d94

wip

8ca5c90

wip

725da5c

wip

ba61b49

wip

460b39c

wip

e5bb603

pedronfigueiredo force-pushed the pnf/24486 branch from 9d62aa9 to e5bb603 Compare July 12, 2024 08:56

wip

cd1c1d8

jpuri approved these changes Jul 12, 2024

View reviewed changes

cryptotavares approved these changes Jul 12, 2024

View reviewed changes

pedronfigueiredo merged commit 81c85a6 into develop Jul 12, 2024
73 checks passed

pedronfigueiredo deleted the pnf/24486 branch July 12, 2024 11:51

github-actions bot locked and limited conversation to collaborators Jul 12, 2024

metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 12, 2024

metamaskbot added release-12.2.0 Issue or pull request that will be included in release 12.2.0 and removed release-12.3.0 Issue or pull request that will be included in release 12.3.0 labels Aug 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add `ui_customizations` metric for transactions #25736

feat: Add `ui_customizations` metric for transactions #25736

pedronfigueiredo commented Jul 10, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Jul 10, 2024

OGPoyraz Jul 10, 2024

OGPoyraz Jul 10, 2024

OGPoyraz Jul 10, 2024

OGPoyraz Jul 10, 2024

jpuri Jul 10, 2024

pedronfigueiredo Jul 11, 2024

matthewwalsh0 Jul 10, 2024

pedronfigueiredo Jul 11, 2024

matthewwalsh0 Jul 10, 2024

pedronfigueiredo Jul 11, 2024

matthewwalsh0 Jul 10, 2024

pedronfigueiredo Jul 11, 2024

sonarqubecloud bot commented Jul 12, 2024

metamaskbot commented Jul 12, 2024

codecov bot commented Jul 12, 2024

metamaskbot commented Aug 19, 2024

feat: Add ui_customizations metric for transactions #25736

feat: Add ui_customizations metric for transactions #25736

Conversation

pedronfigueiredo commented Jul 10, 2024 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jul 12, 2024

Quality Gate passed

metamaskbot commented Jul 12, 2024

codecov bot commented Jul 12, 2024

Codecov Report

metamaskbot commented Aug 19, 2024

feat: Add `ui_customizations` metric for transactions #25736

feat: Add `ui_customizations` metric for transactions #25736

pedronfigueiredo commented Jul 10, 2024 •

edited by github-actions bot

Loading