Skip to content

chore: only fire RatesReceived once#9298

Merged
0xApotheosis merged 2 commits intodevelopfrom
do-not-RatesReceived-on-refresh
Apr 10, 2025
Merged

chore: only fire RatesReceived once#9298
0xApotheosis merged 2 commits intodevelopfrom
do-not-RatesReceived-on-refresh

Conversation

@0xApotheosis
Copy link
Copy Markdown
Member

Description

Only fire the RatesReceived MixPanel event once for a certain trade input.
Reporting-wise we don't want to clog the data with refreshes.

Issue (if applicable)

Closes #9297

Risk

Low

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

None

Testing

Engineering

Stick a debugger on mixpanel.track(MixPanelEvent.RatesReceived, quoteData) and confirm it fires only once for a rate input.

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Nothing required.

Screenshots (if applicable)

N/A

@0xApotheosis 0xApotheosis requested a review from a team as a code owner April 9, 2025 02:24
Copy link
Copy Markdown
Collaborator

@NeOMakinG NeOMakinG left a comment

Choose a reason for hiding this comment

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

I'm wondering if we also want to avoid that QuotesReceived spamming, it's sending multiple times at initial load:

image

Then 3 times after confirm (might due to some strict mode but 3 times seems more than the usual double rendering:
image

Rate event seems sane now, received one log at first input filling time
Then changing the input sent another event after fetching rates

image

One weird thing is that if you go back to the rate screen, it sends the event 2 times, obviously because of the strict mode but we are showing the exact same rate as before, so we will send an event for rates we already fetched in the past

Unrelated to this PR but, do you feel like we should try to avoid sending this event when going back to the rate view?

This one looks good to go!

useEffect(() => {
if (isAnyTradeQuoteLoading) return
if (mixpanel && sortedTradeQuotes.length) {
if (!mixpanel || !sortedTradeQuotes.length) return
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

❤️

@0xApotheosis
Copy link
Copy Markdown
Member Author

0xApotheosis commented Apr 10, 2025

Unrelated to this PR but, do you feel like we should try to avoid sending this event when going back to the rate view?

@NeOMakinG good find, we definitely should! I'll make a note for a follow-up.

Edit, follow-up: #9311

@0xApotheosis 0xApotheosis linked an issue Apr 10, 2025 that may be closed by this pull request
@0xApotheosis 0xApotheosis removed a link to an issue Apr 10, 2025
@0xApotheosis 0xApotheosis merged commit f454afa into develop Apr 10, 2025
3 checks passed
@0xApotheosis 0xApotheosis deleted the do-not-RatesReceived-on-refresh branch April 10, 2025 03:52
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.

Log RatesReceived only once

2 participants