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

fix: add extra condition to prevent erroneous calls to fetch insight #26248

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

hmalik88
Copy link
Contributor

@hmalik88 hmalik88 commented Jul 31, 2024

Description

Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions:

  1. What is the reason for the change? Sentry was showing undefined parameters for a signature insight call, it was found that this was reproducible when a signature request was rejected.
  2. What is the improvement/solution? Added an extra condition to prevent unnecessary calls to fetchInsight in the useSignatureInsights hook.

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.

Copy link
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.

@hmalik88 hmalik88 marked this pull request as ready for review July 31, 2024 13:28
@hmalik88 hmalik88 requested a review from a team as a code owner July 31, 2024 13:28
@metamaskbot metamaskbot added INVALID-PR-TEMPLATE PR's body doesn't match template team-snaps-platform Snaps Platform team labels Jul 31, 2024
Copy link

@hmalik88 hmalik88 removed the INVALID-PR-TEMPLATE PR's body doesn't match template label Jul 31, 2024
Copy link

codecov bot commented Jul 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.95%. Comparing base (44ecc0f) to head (05b94d2).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #26248      +/-   ##
===========================================
- Coverage    69.95%   69.95%   -0.00%     
===========================================
  Files         1411     1411              
  Lines        49963    49963              
  Branches     13800    13800              
===========================================
- Hits         34948    34947       -1     
- Misses       15015    15016       +1     

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

@hmalik88 hmalik88 merged commit ee8009f into develop Jul 31, 2024
86 of 87 checks passed
@hmalik88 hmalik88 deleted the hm/signature-insight-fix branch July 31, 2024 13:51
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2024
@metamaskbot metamaskbot added the release-12.4.0 Issue or pull request that will be included in release 12.4.0 label Jul 31, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [05b94d2]
Page Load Metrics (223 ± 239 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6059512412158
domContentLoaded8111262914
load371917223498239
domInteractive8111262914
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 25 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@gauthierpetetin gauthierpetetin added release-12.3.0 Issue or pull request that will be included in release 12.3.0 and removed release-12.4.0 Issue or pull request that will be included in release 12.4.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.3.0 Issue or pull request that will be included in release 12.3.0 team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants