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

Add amplitude event to code.org/music and update activity links #58450

Merged
merged 2 commits into from
May 7, 2024

Conversation

kelbyhawn
Copy link
Contributor

@kelbyhawn kelbyhawn commented May 7, 2024

Adds Amplitude (and Statsig) events to https://code.org/music, and updates the existing urls to the activity to include a /reset at the end in a few places.

Links

Jira ticket: ACQ-1869

Testing story

Local testing


Screenshot 2024-05-07 at 2 19 38 PM

@kelbyhawn kelbyhawn requested a review from a team May 7, 2024 18:03
@@ -86,3 +86,5 @@ theme: responsive_full_width
=hoc_s(:dance_afe_disclaimer)

= view :swiper_page_music_lab

= view :analytics_event_log_helper, event_name: AnalyticsConstants::MUSIC_PAGE_VISITED_EVENT
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the analytics event log helper automatically sending all events to both?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just double checked- that .js file would require a refactor to send Statsig events by default

Copy link
Contributor

Choose a reason for hiding this comment

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

We can pair on it this PM if you want! (Also... if Statsig isn't a requirement here then please ignore me)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how that Statsig console message originally showed up, and I can't get it to show up again, weird! 🤔

@dmcavoy do you want Statsig set up for Pegasus pages too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not right now. Lets keep this just amplitude

Copy link
Contributor

@hannahbergam hannahbergam left a comment

Choose a reason for hiding this comment

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

The screenshot is of two user properties events, not 'page visited' events. You should be able to see the event show up in the console under the two you showed (which happen on nearly every page load). Can you update the screenshot with the specific events?

Copy link
Contributor

@hannahbergam hannahbergam left a comment

Choose a reason for hiding this comment

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

I see the new screenshot yay!

@kelbyhawn kelbyhawn merged commit 806ab49 into staging May 7, 2024
2 checks passed
@kelbyhawn kelbyhawn deleted the add-amplitude-event-to-music-page branch May 7, 2024 21:20
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