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.

Sign up for GitHub

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

Jump to bottom

feat: add missing TWAP events and some fortune cookies #2777

Merged
merged 14 commits into from
Jul 6, 2023
Merged

feat: add missing TWAP events and some fortune cookies #2777

merged 14 commits into from
Jul 6, 2023

Conversation

nenadV91
Copy link
Contributor

@nenadV91 nenadV91 commented Jul 3, 2023 •
edited
Loading

Summary

TWAP analytic events from this document

Events

  • When you are on advanced-orders widget and you connect a wallet (compatible, non-compatible, safe-that-could-be-converted)
  • When user checks "Modify Safes handler" checkbox
  • Click on read more (These components are still not implemented)
  • Open on advanced orders tab

These events will happen only when the user still hasn't set fallback handler and now is trying to

  • TWAP with conversion initiated (When users clicks on "Review TWAP order" button)
  • TWAP with conversion posted (When he confirms in the modal)
  • TWAP with conversion signed (When he signs in safe)
  • TWAP with conversion rejected (When he rejects signing)

Some COW fortune events

  • Share COW Fortune on Twitter
  • Open COW Fortune

Deprecated

  • Click on “Change connected wallet” button Deprecated
  • Click on “Setup supported Safe” button Deprecated

To test

  1. turn on GA debugger
  2. test different events

@vercel
Copy link

vercel bot commented Jul 3, 2023 •
edited by github-actions bot
Loading

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

Name Status Preview Comments Updated (UTC)
swap-dev 🔄 Building (Inspect) Visit Preview

🌃 Cosmos ↗︎

@nenadV91 nenadV91 added the TWAP label Jul 3, 2023
@nenadV91 nenadV91 marked this pull request as ready for review July 5, 2023 10:46
@nenadV91 nenadV91 requested review from a team July 5, 2023 10:46
nenadV91
nenadV91 commented Jul 5, 2023
View reviewed changes
src/legacy/components/analytics/index.ts
@@ -41,6 +41,7 @@ if (typeof GOOGLE_ANALYTICS_ID === 'string') {
storage: 'none',
storeGac: false,
clientId: storedClientId ?? undefined,
cookieFlags: 'SameSite=None; Secure',
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 was fix for Safe App not sending GA events

@nenadV91 nenadV91 changed the title feat: add wallet compatibility events feat: add missing TWAP events and some fortune cookies Jul 5, 2023
shoom3301
shoom3301 approved these changes Jul 6, 2023
View reviewed changes
Copy link
Collaborator

@shoom3301 shoom3301 left a comment

Choose a reason for hiding this comment

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

Good! Thank you!
I've added a small fix to prevent falsy triggers of safe-that-could-be-converted

alfetopito
alfetopito approved these changes Jul 6, 2023
View reviewed changes
Copy link
Collaborator

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Looks good, just a minor comment

src/legacy/components/analytics/events/twapEvents.ts Outdated Show resolved Hide resolved
elena-zh
elena-zh approved these changes Jul 6, 2023
View reviewed changes
Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

All events (besides the one that is not yet implemented) LGTM

@nenadV91 nenadV91 merged commit 8c2d3b3 into develop Jul 6, 2023
5 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 6, 2023
@alfetopito alfetopito deleted the twap/analytics branch July 6, 2023 14:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alfetopito alfetopito alfetopito approved these changes

@shoom3301 shoom3301 shoom3301 approved these changes

@elena-zh elena-zh elena-zh approved these changes

Assignees
No one assigned
Labels
TWAP
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nenadV91 @alfetopito @shoom3301 @elena-zh