Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signature Redesign - add any required metrics to re-designed signature request pages #23555

Closed
jpuri opened this issue Mar 18, 2024 · 8 comments · Fixed by #24095 or #24182
Closed

Signature Redesign - add any required metrics to re-designed signature request pages #23555

jpuri opened this issue Mar 18, 2024 · 8 comments · Fixed by #24095 or #24182
Assignees
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. release-11.16.0 Issue or pull request that will be included in release 11.16.0 release-12.0.0 Issue or pull request that will be included in release 12.0.0 team-confirmations Push issues to confirmations team team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead

Comments

@jpuri
Copy link
Contributor

jpuri commented Mar 18, 2024

Signature rest pages did not had any metrics events to be ported to new designed pages. (Typed sign V3, V4 pages has a legacy events that was re-redundant).

We need to check with @bschorchit and any new metrics events required for redesigned pages.

@jpuri jpuri added the team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead label Mar 18, 2024
@metamaskbot metamaskbot added the INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. label Mar 18, 2024
@bschorchit
Copy link

bschorchit commented Mar 18, 2024

New metrics:

  • tracking that it's the redesigned UI being used (while we have both new and old co-existing)
    For this we can add the value redesigned_confirmation to the ui_customizations property of all signature events when the redesigned UI is being displayed

  • tracking opening of account info modal
    new property within Signatures events:
    account_modal_opened: true if user opened the account modal during the request

@blackdevelopa
Copy link
Contributor

Hey team! Please add your planning poker estimate with Zenhub @digiwand @jpuri @pedronfigueiredo @segun

@jpuri jpuri self-assigned this Mar 22, 2024
@jpuri
Copy link
Contributor Author

jpuri commented Mar 25, 2024

Hey @bschorchit : can you plz spec

tracking opening of account info modal

@bschorchit
Copy link

done @jpuri !

@jpuri jpuri removed their assignment Mar 28, 2024
@digiwand digiwand self-assigned this Apr 9, 2024
@digiwand
Copy link
Contributor

digiwand commented Apr 11, 2024

I began working on this ticket. I discovered there was a flaw in an existing metric I attempted to fix 2 months ago.
PR: #22631
Issue: https://github.com/MetaMask/MetaMask-planning/issues/1756
Example metric

The transaction event with key value 'external_link_clicked': 'security_alert_support_link', is only applied to transactions that are not signatures. We wanted to apply this to both transactions and signatures.

This is relevant because I attempted to use the same method, useTransactionEventFragment, and then realized this bug.

ticket created to handle issue where external_link_clicked is not being called for signatures
#23995

@digiwand
Copy link
Contributor

digiwand commented Apr 15, 2024

re: redesigned_confirmation to the ui_customizations

  • The related PR updates the "Cancel" and "Confirm" signature events
  • The related PR does not update the "Signature <Approved,Failed,Rejected,Requested>" events

^ nevermind. As discussed in earlier's standup, "Cancel" and "Confirm" events will be deprecated. Instead, following the preference setting that will be added to enable the redesign confirmations, we will

  • pass preferencesController to createRPCMethodTrackingMiddleware
  • add the ui_customization property if the preference is enabled

@digiwand
Copy link
Contributor

digiwand commented Apr 15, 2024

re:

tracking opening of account info modal
new property within Signatures events:
account_modal_opened: true if user opened the account modal during the request

as discussed in earlier's standup, we'll not append the property to existing signature events. Instead, we will create a new event to be fired when the account modal is opened


we will use
https://www.notion.so/Account-Details-Opened-3deec398d4784bcea00b012cdf1cde03?pvs=4

  • pass the signature_confirmation value to the location property
  • pass signature_type

https://consensys.slack.com/archives/C04C0F6SSJH/p1713276122434229?thread_ts=1713275327.646559&cid=C04C0F6SSJH

digiwand added a commit that referenced this issue Apr 19, 2024
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

Adds "Account Details Opened" event when Account Info Icon in
Confirmation Redesign is clicked

Internal Thread
https://consensys.slack.com/archives/C03ETQA9EPK/p1710437328642929
Event Description
https://www.notion.so/Account-Details-Opened-3deec398d4784bcea00b012cdf1cde03?pvs=4

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24095?quickstart=1)

## **Related issues**

Fixes: #23555

## **Manual testing steps**



1. set `
ENABLE_CONFIRMATION_REDESIGN=true` in .metamaskrc
2. Turn on MetaMetrics in settings
3. Use test-dapp to test feature in a signature confirmation

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask 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.
@digiwand digiwand reopened this Apr 19, 2024
@metamaskbot metamaskbot added the release-11.16.0 Issue or pull request that will be included in release 11.16.0 label Apr 19, 2024
@cryptotavares cryptotavares added the team-confirmations Push issues to confirmations team label Apr 24, 2024
@digiwand digiwand changed the title Confirmation redesign - add any required metrics to re-designed signature request pages Signature Redesign - add any required metrics to re-designed signature request pages Apr 30, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-11.17.0 on issue. Adding release label release-11.17.0 on issue, as issue is linked to PR #24182 which has this release label.

@gauthierpetetin gauthierpetetin added the release-12.0.0 Issue or pull request that will be included in release 12.0.0 label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INVALID-ISSUE-TEMPLATE Issue's body doesn't match any issue template. release-11.16.0 Issue or pull request that will be included in release 11.16.0 release-12.0.0 Issue or pull request that will be included in release 12.0.0 team-confirmations Push issues to confirmations team team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead
Projects
None yet
7 participants