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: missing deadline in swaps stx status screen #25779

Merged
merged 11 commits into from
Aug 20, 2024
Merged

Conversation

infiniteflower
Copy link
Contributor

@infiniteflower infiniteflower commented Jul 11, 2024

Description

This PR fixes an issue where the STX status screen for a swap was showing a 0:00 for the timer.

Open in GitHub Codespaces

Related issues

Related to: #25063

Manual testing steps

  1. Make sure Smart Transactions is on (Settings > Advanced)
  2. Do a Swap
  3. Observe that timer is not 0:00 and is a reasonable number

Screenshots/Recordings

Before

Screen.Recording.2024-07-10.at.2.41.53.PM.mov

After

Screen.Recording.2024-07-11.at.1.28.53.PM.mov

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.

@infiniteflower infiniteflower requested a review from a team as a code owner July 11, 2024 17:31
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.

Copy link

sonarcloud bot commented Jul 15, 2024

@martahj martahj requested a review from a team as a code owner August 16, 2024 18:26
martahj
martahj previously approved these changes Aug 16, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [190ac46]
Page Load Metrics (74 ± 14 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint692101023215
domContentLoaded39185693014
load46185742814
domInteractive9139342713
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 62 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

Copy link

codecov bot commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 69.96%. Comparing base (fc3da74) to head (84ac8b4).
Report is 1 commits behind head on develop.

Files Patch % Lines
app/scripts/controllers/swaps/index.ts 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #25779      +/-   ##
===========================================
- Coverage    69.96%   69.96%   -0.00%     
===========================================
  Files         1405     1405              
  Lines        49000    49003       +3     
  Branches     13699    13701       +2     
===========================================
  Hits         34282    34282              
- Misses       14718    14721       +3     

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

Copy link
Contributor

@dan437 dan437 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding the fallback! One thing: this PR adds a fallback for the deadline, but it should use a value from backend in the first place. Can you check if the value from backend is being used?

@@ -247,6 +247,7 @@
"swapsQuoteRefreshTime": 60000,
"swapsQuotePrefetchingRefreshTime": 60000,
"swapsStxBatchStatusRefreshTime": 10000,
"swapsStxStatusDeadline": "number",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a "number" string and not some exact number like other fallback values?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The value wasn't being set up correctly in the test, but it is now 👍

digiwand
digiwand previously approved these changes Aug 19, 2024
@martahj martahj dismissed stale reviews from digiwand and themself via 596e722 August 19, 2024 16:56
@metamaskbot
Copy link
Collaborator

Builds ready [596e722]
Page Load Metrics (72 ± 6 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7013598178
domContentLoaded498566126
load499272126
domInteractive10412794
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 194 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [af3c138]
Page Load Metrics (78 ± 8 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint771581072110
domContentLoaded4211974199
load5111978178
domInteractive125827115
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 194 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 26 Bytes (0.00%)

@martahj
Copy link
Contributor

martahj commented Aug 19, 2024

Thanks for adding the fallback! One thing: this PR adds a fallback for the deadline, but it should use a value from backend in the first place. Can you check if the value from backend is being used?

@dan437 You were right - the fallback rather than the value from the PR was used. I pushed some updates and that is no longer the case.

Copy link

sonarcloud bot commented Aug 20, 2024

@metamaskbot
Copy link
Collaborator

Builds ready [84ac8b4]
Page Load Metrics (66 ± 7 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint5814692199
domContentLoaded3810060157
load4410866157
domInteractive9442184
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 194 Bytes (0.01%)
  • ui: 0 Bytes (0.00%)
  • common: 26 Bytes (0.00%)

@martahj martahj merged commit 9ee22f8 into develop Aug 20, 2024
78 checks passed
@martahj martahj deleted the fix/swaps-stx-timer branch August 20, 2024 13:46
@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-swaps
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants