Skip to content

fix: correct duplicate notifications event tracking in global menu#26525

Merged
matteoscurati merged 1 commit intodevelopfrom
fix/notifications-fix-menu-event-tracking
Aug 20, 2024
Merged

fix: correct duplicate notifications event tracking in global menu#26525
matteoscurati merged 1 commit intodevelopfrom
fix/notifications-fix-menu-event-tracking

Conversation

@matteoscurati
Copy link
Copy Markdown
Contributor

@matteoscurati matteoscurati commented Aug 19, 2024

Description

This PR addresses an issue where duplicate event tracking calls were made in the handleNotificationsClick function within the GlobalMenu component. This redundancy could lead to inaccurate event logging and unnecessary data collection. Also, the PR removes an unnecessary property.

Open in GitHub Codespaces

Related issues

Fixes: N/A

Manual testing steps

  1. Open the global menu
  2. Click on the notifications menu item
  3. Verify that the event is tracked only once

Screenshots/Recordings

Before

N/A

After

N/A

Pre-merge author checklist

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.

@matteoscurati matteoscurati added the team-notifications-deprecated DEPRECATED: please use "team-assets" instead label Aug 19, 2024
@github-actions
Copy link
Copy Markdown
Contributor

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.

@sonarqubecloud
Copy link
Copy Markdown

@matteoscurati matteoscurati marked this pull request as ready for review August 19, 2024 21:23
@matteoscurati matteoscurati requested a review from a team as a code owner August 19, 2024 21:23
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [5c04d02]
Page Load Metrics (73 ± 16 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint65246993818
domContentLoaded41200693416
load47200733316
domInteractive10161293115
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -22 Bytes (-0.00%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.96%. Comparing base (81af2e8) to head (5c04d02).

Files Patch % Lines
...i/components/multichain/global-menu/global-menu.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #26525   +/-   ##
========================================
  Coverage    69.96%   69.96%           
========================================
  Files         1405     1405           
  Lines        48996    48996           
  Branches     13697    13697           
========================================
  Hits         34280    34280           
  Misses       14716    14716           

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

@matteoscurati matteoscurati merged commit 084768a into develop Aug 20, 2024
@matteoscurati matteoscurati deleted the fix/notifications-fix-menu-event-tracking branch August 20, 2024 15:05
@github-actions github-actions bot locked and limited conversation to collaborators Aug 20, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 20, 2024
@gauthierpetetin gauthierpetetin added release-12.4.0 Issue or pull request that will be included in release 12.4.0 and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Sep 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.4.0 Issue or pull request that will be included in release 12.4.0 team-notifications-deprecated DEPRECATED: please use "team-assets" instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants