Skip to content

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

Merged
hmalik88 merged 1 commit intodevelopfrom
hm/signature-insight-fix
Jul 31, 2024
Merged

fix: add extra condition to prevent erroneous calls to fetch insight#26248
hmalik88 merged 1 commit intodevelopfrom
hm/signature-insight-fix

Conversation

@hmalik88
Copy link
Copy Markdown
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.

@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.

@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-deprecated DEPRECATED: please use "team-core-platform" instead (or "team-new-networks" for Solana snap issues) labels Jul 31, 2024
@sonarqubecloud
Copy link
Copy Markdown

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

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
@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
Copy Markdown
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-deprecated DEPRECATED: please use "team-core-platform" instead (or "team-new-networks" for Solana snap issues)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants