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 for visiting the self-paced PL marketing page #51951

Merged
merged 1 commit into from
May 18, 2023

Conversation

TurnerRiley
Copy link
Contributor

@TurnerRiley TurnerRiley commented May 17, 2023

Adds a new Amplitude event for visiting a marketing page (the self-paced PL marketing page in this case). This event was added following the same format as other marketing page visit events.

Amplitude logging event (locally)

SelfPacedAmplitude

Links

Jira ticket: here
Associated Amplitude event: here
Reference PR: here

Testing story

Localhost testing.

PR Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@TurnerRiley TurnerRiley requested a review from a team May 17, 2023 19:21
Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Unfortunately I don't think this is going to work very well, because this page is cached:

Screenshot 2023-05-18 at 12 30 19 PM

That means that many requests for this page are served by cloudfront, without hitting our servers.

I am also just realizing that we may have the same problem on other pegasus pages where we try to record amplitude events.

I could be missing something here, but I think we are going to have to have a conversation with the product team about what we need this metric for, since it is not clear to me that amplitude (as we are currently using it) is going to be effective.

@davidsbailey
Copy link
Member

davidsbailey commented May 18, 2023

TurnerRiley do you think you could gather a list of which amplitude metrics we're trying to capture upon viewing of a pegasus page, so we can try and figure out which metrics we may need to investigate? metrics which are captured in response to a user action on the page can be excluded since those should be working properly.

@davidsbailey
Copy link
Member

Apologies @TurnerRiley , per https://codedotorg.slack.com/archives/C0453TUMLE7/p1684443407266619?thread_ts=1684442534.547659&cid=C0453TUMLE7 I realized this should be working fine, because we do the logging from js, which will be executed regardless of whether the page is cached. this change LGTM!

@TurnerRiley TurnerRiley merged commit 0a7d8dd into staging May 18, 2023
2 checks passed
@TurnerRiley TurnerRiley deleted the add-self-paced-pl-page-visit-event branch May 18, 2023 21:17
pablo-code-org pushed a commit that referenced this pull request May 25, 2023
snickell pushed a commit that referenced this pull request Feb 3, 2024
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

2 participants