fix: PPOM - Metrics information from ppom is not logged

[segun]

Description

Whenever we trigger a transaction from a dapp, we don't see any information about Blockaid validation. This is because the blockaid banner now shows loading initially for some time before the actual alert is shown. We were sending metrics only on component load, but we should send metrics for blockaid alert

The solution used here is to delay metrics for blockaid until user clicks confirm or cancel. At that point we have a definitive blockaid status to send (if it's loading, we send loading, if it's loaded we send the normal metrics).

For the deleted test cases in MessageSign (because enzyme does not support useEffect). I had a call with @cryptotavares and we will convert the tests to RTL in another PR and add the required useEffect tests there. There's no need to delay this because of that.

Related issues

Fixes: #7795

Manual testing steps

1. Checkout this branch and run MetaMask Mobile
2. Enable blockaid in settings
3. Open testdapp and perform a PPOM transaction
4. Check the console and notice how security alert metrics is not sent
5. Now click/confirm cancel while the banner is still showing loading and check console to see how security alert metrics is sent with loading
6. Now perform another PPOM transaction, this time wait for the blockaid banner to fully load and then click confirm/cancel and see how the security alert response is sent in metrics.

Screenshots/Recordings

Before

metrics-blockaid-mobile.mp4

After

new_metrics.mov

Pre-merge author checklist

• I’ve followed MetaMask Coding Standards.
• I've clearly explained what problem this PR is solving and how it is solved.
• I've linked related issues
• I've included manual testing steps
• I've included screenshots/recordings if applicable
• 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.
• I’ve properly set the pull request status:
  • In case it's not yet "ready for review", I've set it to "draft".
  • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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]

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.

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9524cc40-8a6a-4f0e-8e8a-96acfa937761
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

[seaona]

In both cases we send metrics when the result type changes. It would initially be undefined (we don't send any metrics), then it goes to Loading (we send loading metrics) and then go to actual alert (we send metrics of the securityAlertReponse)

I think this might not be 100% what we expect here, as in the case the validation is complete, for one transaction we will have now x2 Confirm Started events instead of 1, so we will see suddenly more Transactions added to the wallet, which won't be the reality. We should either inform the data team about this change or look for an approach of only 1 transaction being counted (Confirm Started) as added to the wallet - no matter if there was change of validation state

Ie. Transactions with Blockaid will log more "Confirm Started" events than transactions without Blockaid.

I would like to clarify the behaviour. What do you think? cc @segun @cryptotavares @bschorchit

[cryptotavares]

In both cases we send metrics when the result type changes. It would initially be undefined (we don't send any metrics), then it goes to Loading (we send loading metrics) and then go to actual alert (we send metrics of the securityAlertReponse)

I think this might not be 100% what we expect here, as in the case the validation is complete, for one transaction we will have now x2 Confirm Started events instead of 1, so we will see suddenly more Transactions added to the wallet, which won't be the reality. We should either inform the data team about this change or look for an approach of only 1 transaction being counted (Confirm Started) as added to the wallet - no matter if there was change of validation state

Ie. Transactions with Blockaid will log more "Confirm Started" events than transactions without Blockaid.

I would like to clarify the behaviour. What do you think? cc @segun @cryptotavares @bschorchit

I agree with @seaona this does not seem the desired behaviour. Should we have a specific blockaid status event that we fire for a given transaction/signature whenever the blockaid status is updated? @bschorchit @jpuri

[cryptotavares]

Also besides all the above, it would be good to add unit tests, where we check that intended metrics event fired (once and only once) and that it contained blockaid expected data

[cryptotavares]

It is looking good! 🚀
Besides @jpuri comments, I would just add some unit tests to ensure that the AnalyticsV2.trackEvent is being called properly with the blockaid data

[codecov-commenter]

Codecov Report

Attention: 42 lines in your changes are missing coverage. Please review.

Comparison is base (89eb8cd) 36.59% compared to head (5e05b80) 37.09%.
Report is 6 commits behind head on main.

Files	Patch %	Lines
app/components/UI/MessageSign/MessageSign.tsx	57.69%	10 Missing and 1 partial ⚠️
...pp/components/UI/ApproveTransactionReview/index.js	0.00%	10 Missing ⚠️
app/components/Views/Approval/index.js	0.00%	10 Missing ⚠️
app/util/blockaid/index.ts	40.00%	2 Missing and 1 partial ⚠️
app/util/confirmation/signatureUtils.js	62.50%	3 Missing ⚠️
app/components/UI/TypedSign/index.js	66.66%	2 Missing ⚠️
app/components/UI/PersonalSign/PersonalSign.tsx	66.66%	0 Missing and 1 partial ⚠️
app/components/UI/SignatureRequest/index.js	0.00%	1 Missing ⚠️
app/components/UI/TransactionReview/index.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7822      +/-   ##
==========================================
+ Coverage   36.59%   37.09%   +0.49%     
==========================================
  Files        1120     1130      +10     
  Lines       29188    29118      -70     
  Branches     2717     2722       +5     
==========================================
+ Hits        10682    10801     +119     
+ Misses      17894    17694     -200     
- Partials      612      623      +11     

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

[segun]

I've added some comments to MessageSign tests for the reason why some tests are removed. This should apply to the TypedSign too.

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/92d2a9f4-943f-44f6-80d9-75dee64878f4
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

1 Bug
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

41.1% Coverage
0.0% Duplication