fix: mm_pay_time_to_complete_s is logging incorrect (too low) values by jpuri · Pull Request #27862 · MetaMask/metamask-mobile

jpuri · 2026-03-24T12:50:08Z

Description

Fix time calculation for metrics mm_pay_time_to_complete_s. Replace the use of submittedTime in current transaction to latest submitted time of child transactions.

Changelog

CHANGELOG entry:

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/CONF-1086

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

NA

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Mobile 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.

Note

Medium Risk
Changes MetaMask Pay analytics attribution logic (parent/child relationships and completion time calculation), which can affect reporting correctness across multi-transaction flows. Risk is moderate due to reliance on requiredTransactionIds (removing batchId/nonce ordering) and updated tests covering new behavior.

Overview
Fixes mm_pay_time_to_complete_s to avoid under-reporting by computing completion time from the latest submittedTime among a parent MM Pay transaction’s requiredTransactionIds, instead of using the current transaction’s submittedTime.

Simplifies transaction linkage and step calculation by dropping batchId/nonce-based grouping and relying solely on requiredTransactionIds to find parents and order steps; updates unit tests accordingly (including asserting the metric is not emitted for finalized child transactions).

^{Written by Cursor Bugbot for commit 22bbd5c. This will update automatically on new commits. Configure here.}

github-actions · 2026-03-24T12:50:19Z

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.

codecov-commenter · 2026-03-24T13:16:20Z

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.51%. Comparing base (c56e9d0) to head (d5a335f).
⚠️ Report is 30 commits behind head on main.

Files with missing lines	Patch %	Lines
...tion-controller/metrics_properties/metamask-pay.ts	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #27862      +/-   ##
==========================================
+ Coverage   82.46%   82.51%   +0.04%     
==========================================
  Files        4801     4804       +3     
  Lines      123807   124003     +196     
  Branches    27602    27636      +34     
==========================================
+ Hits       102101   102322     +221     
+ Misses      14634    14604      -30     
- Partials     7072     7077       +5

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…mask-mobile into mm_pay_metrics_fix

matthewwalsh0 · 2026-03-25T13:30:15Z

+      addTimeToComplete(
+        properties,
+        eventType,
+        getLatestChildSubmittedTime(transactionMeta, allTransactions),


Could we call this inside addTimeToComplete to avoid unnecessary logic until finalized?

And just pass allTransactions to addTimeToComplete?

PR is updated

matthewwalsh0 · 2026-03-25T13:30:32Z

+    .filter(
+      (tx) =>
+        requiredTransactionIds?.includes(tx.id) ||
+        (batchId && tx.batchId === batchId && tx.id !== transactionMeta.id),


Batch ID should be irrelevant as we no longer use the bridge strategy.

PR is updated

matthewwalsh0 · 2026-03-25T13:30:52Z


    if (hasTransactionType(transactionMeta, PAY_TYPES) || properties.mm_pay) {
-      addTimeToComplete(properties, eventType, transactionMeta.submittedTime);
+      addTimeToComplete(


Minor, could we remove the duplication condition, maybe assign a variable? Or re-work the conditions?

PR is updated

matthewwalsh0

Just clarifications regarding filtering.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

github-actions · 2026-03-25T16:20:31Z

✅ E2E Fixture Validation — Schema is up to date
16 value mismatches detected (expected — fixture represents an existing user).
View details

…mask-mobile into mm_pay_metrics_fix

github-actions · 2026-03-26T09:46:49Z

🔍 Smart E2E Test Selection

Selected E2E tags: None (no tests recommended)
Selected Performance tags: None (no tests recommended)
Risk Level: low
AI Confidence: 92%

click to see 🤖 AI reasoning details

E2E Test Selection:
The two changed files are:

metamask-pay.ts - An analytics/metrics property builder for MetaMask Pay transactions (perps deposits, swaps). The changes refactor how parent-child transaction relationships are resolved: removing batchId-based lookup in favor of requiredTransactionIds-only lookup, and updating addTimeToComplete to use the latest child submittedTime via requiredTransactionIds. This is purely internal analytics logic with no effect on transaction execution, UI, or user-facing behavior.
metamask-pay.test.ts - Unit tests updated to match the new logic. No E2E test infrastructure changes.

The only consumer of metamask-pay.ts is metrics.ts (the transaction event handler), which fires analytics events. No UI components, navigation flows, confirmations, or wallet state are affected. The changes are well-covered by the updated unit tests. No E2E tags are warranted since no user-visible behavior changed.

Performance Test Selection:
The changes are limited to analytics metrics property builders for transaction events. There is no UI rendering, data loading, state management, or critical user flow impact. No performance tests are needed.

View GitHub Actions results

sonarqubecloud · 2026-03-26T10:11:18Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

fix: mm_pay_time_to_complete_s is logging incorrect (too low) values

d5a335f

metamaskbot added the team-confirmations Push issues to confirmations team label Mar 24, 2026

github-actions Bot added the size-S label Mar 24, 2026

jpuri added the no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed label Mar 24, 2026

jpuri marked this pull request as ready for review March 25, 2026 10:29

jpuri requested a review from a team as a code owner March 25, 2026 10:29

Merge branch 'main' into mm_pay_metrics_fix

1c4d1ec

github-actions Bot added the risk-low Low testing needed · Low bug introduction risk label Mar 25, 2026

update

7e5f2a6

cursor Bot reviewed Mar 25, 2026

View reviewed changes

Comment thread app/core/Engine/controllers/transaction-controller/metrics_properties/metamask-pay.ts

Merge branch 'mm_pay_metrics_fix' of https://github.com/MetaMask/meta…

b7841a4

…mask-mobile into mm_pay_metrics_fix

jpuri enabled auto-merge March 25, 2026 10:36

github-actions Bot added risk-low Low testing needed · Low bug introduction risk and removed risk-low Low testing needed · Low bug introduction risk labels Mar 25, 2026

matthewwalsh0 approved these changes Mar 25, 2026

View reviewed changes

jpuri added this pull request to the merge queue Mar 25, 2026

matthewwalsh0 requested changes Mar 25, 2026

View reviewed changes

matthewwalsh0 removed this pull request from the merge queue due to a manual request Mar 25, 2026

update

fe628fc

jpuri requested a review from matthewwalsh0 March 25, 2026 15:58

Merge branch 'main' into mm_pay_metrics_fix

b2a260c

github-actions Bot added size-M risk-low Low testing needed · Low bug introduction risk and removed size-S risk-low Low testing needed · Low bug introduction risk labels Mar 25, 2026

jpuri enabled auto-merge March 25, 2026 16:00

github-actions Bot added risk-low Low testing needed · Low bug introduction risk and removed risk-low Low testing needed · Low bug introduction risk labels Mar 25, 2026

cursor Bot reviewed Mar 25, 2026

View reviewed changes

Comment thread app/core/Engine/controllers/transaction-controller/metrics_properties/metamask-pay.test.ts

jpuri added 3 commits March 26, 2026 15:13

update

e053bdf

Merge branch 'mm_pay_metrics_fix' of https://github.com/MetaMask/meta…

5c36745

…mask-mobile into mm_pay_metrics_fix

Merge branch 'main' into mm_pay_metrics_fix

22bbd5c

github-actions Bot added risk-low Low testing needed · Low bug introduction risk and removed risk-low Low testing needed · Low bug introduction risk labels Mar 26, 2026

matthewwalsh0 approved these changes Mar 27, 2026

View reviewed changes

jpuri added this pull request to the merge queue Mar 27, 2026

Merged via the queue into main with commit faa146d Mar 27, 2026
66 checks passed

jpuri deleted the mm_pay_metrics_fix branch March 27, 2026 11:54

github-actions Bot locked and limited conversation to collaborators Mar 27, 2026

metamaskbot added the release-7.73.0 Issue or pull request that will be included in release 7.73.0 label Mar 27, 2026

Uh oh!

Conversation

jpuri commented Mar 24, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changelog

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions Bot commented Mar 24, 2026

Uh oh!

codecov-commenter commented Mar 24, 2026

Codecov Report

Uh oh!

Uh oh!

matthewwalsh0 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpuri Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpuri Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

jpuri Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

matthewwalsh0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Mar 25, 2026

Uh oh!

github-actions Bot commented Mar 26, 2026

🔍 Smart E2E Test Selection

Uh oh!

sonarqubecloud Bot commented Mar 26, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jpuri commented Mar 24, 2026 •

edited by cursor Bot

Loading