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: Simplify event logging for SWAP_QUOTE_RECEIVED #6628

Merged
merged 2 commits into from
May 23, 2023

Conversation

tinaszheng
Copy link
Contributor

@tinaszheng tinaszheng commented May 23, 2023

Description

The previous version of the hook was infinite looping when trade is always defined, so simplify logic to just keep track of trade changes. Also removes the time-based logging since we have stats on the backend now

Screen capture

No UI changes

Test plan

Reproducing the error

Throw in a console.log right before sendAnalyticsEvent and see it firing like 1 million times lol

@vercel
Copy link

vercel bot commented May 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
interface ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2023 2:53pm
interface-node18 ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2023 2:53pm

@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:43 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:43 — with GitHub Actions Inactive
@tinaszheng tinaszheng requested a review from cmcewen May 23, 2023 14:43
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:44 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:44 — with GitHub Actions Inactive
])
if (!trade || prevTrade === trade) return // no new swap quote to log

setSwapQuoteReceivedDate(new Date())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this property is still used downstream in child components, so not getting rid of it for now

@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:45 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:45 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:45 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:45 — with GitHub Actions Inactive
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.32 🎉

Comparison is base (95814e3) 58.76% compared to head (ce61d12) 60.09%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6628      +/-   ##
==========================================
+ Coverage   58.76%   60.09%   +1.32%     
==========================================
  Files         720      721       +1     
  Lines       21172    21166       -6     
  Branches     6981     6976       -5     
==========================================
+ Hits        12442    12719     +277     
+ Misses       8657     8370     -287     
- Partials       73       77       +4     
Flag Coverage Δ
e2e-tests 60.48% <100.00%> (-1.20%) ⬇️
unit-tests 21.19% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/lib/utils/analytics.ts 71.42% <ø> (-2.77%) ⬇️
src/pages/Swap/index.tsx 89.35% <100.00%> (-0.28%) ⬇️

... and 30 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:51 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:51 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:51 — with GitHub Actions Inactive
@tinaszheng tinaszheng temporarily deployed to false May 23, 2023 14:51 — with GitHub Actions Inactive
@cypress
Copy link

cypress bot commented May 23, 2023

1 flaky tests on run #11235 ↗︎

0 69 8 0 Flakiness 1

Details:

fix: Simplify event logging for SWAP_QUOTE_RECEIVED
Project: Uniswap Interface Commit: 11fb474025
Status: Passed Duration: 05:34 💡
Started: May 23, 2023 2:54 PM Ended: May 23, 2023 2:59 PM
Flakiness  cypress/e2e/swap/swap.test.ts • 1 flaky test • e2e

View Output Video

Test Artifacts
Swap > Swap on main page > swaps ETH for USDC Output Screenshots Video

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@tinaszheng tinaszheng merged commit 6424fdf into main May 23, 2023
23 of 24 checks passed
@tinaszheng tinaszheng deleted the tina/swap-quote-event branch May 23, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants