feat: track Stay Updated button click with PostHog cta_clicked event#3225
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
There was a problem hiding this comment.
Pull request overview
Adds missing analytics for the “Stay Updated” CTA on MITx Online course/program product pages by emitting a PostHog cta_clicked event with resource context.
Changes:
- Add a
resourceprop toProductPageTemplateand capturePostHogEvents.CallToActionClickedwhen “Stay Updated” is clicked. - Pass
resourcemetadata (id/readable_id/type) from Course and Program product pages into the template.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx | Adds PostHog capture on “Stay Updated” click and introduces optional resource prop to provide event context. |
| frontends/main/src/app-pages/ProductPages/CoursePage.tsx | Passes course resource metadata into ProductPageTemplate for CTA tracking. |
| frontends/main/src/app-pages/ProductPages/ProgramPage.tsx | Passes program resource metadata into ProductPageTemplate for CTA tracking. |
| frontends/main/src/app-pages/ProductPages/ProgramAsCoursePage.tsx | Passes program resource metadata into ProductPageTemplate for CTA tracking on the course-style program page. |
… in ProductPageTemplate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Also fix pre-existing broken 'as jest.MockedFunction' cast by using jest.mocked(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| }) | ||
|
|
||
| const handleStayUpdatedClick = () => { | ||
| if (process.env.NEXT_PUBLIC_POSTHOG_API_KEY && resource) { |
There was a problem hiding this comment.
Problem based on my assumption: posthog.capture() is called before NiceModal.show(). If posthog.capture throws (e.g., PostHog client not ready, misconfigured, or runtime error), NiceModal.show never executes — the "Stay Updated" button becomes silently non-functional to the user.
Should Either wrap with try/finally or open the modal first or it would no create issue because posthog will handle the errors internally?
| expect(mockedNiceModalShow).toHaveBeenCalled() | ||
| }) | ||
|
|
||
| describe("PostHog tracking", () => { |
There was a problem hiding this comment.
Missing regression test for the failure path above.
Current PostHog tests in ProductPageTemplate.test.tsx:142 verify event firing and non-firing, but there is no test where capture throws while asserting the modal still opens.
Suggested test: mock capture to throw and assert NiceModal.show is still called once.
There was a problem hiding this comment.
this test would be misleading for two reasons:
PostHog's SDK never throws from capture. The test would encode a premise that is false for this library — it would look like it's protecting against a real risk when it isn't.
It tests implementation at the wrong layer. If PostHog's internal behavior ever changed, it would be their SDK's regression to catch, not ours.
There was a problem hiding this comment.
Are you saying your original comment was out of place? Let's mark this as resolved if so.
There was a problem hiding this comment.
PR looks good. Left few comments with one question
1 . The resource prop is optional (resource?), meaning PostHog tracking silently skips when it's not provided. This is intentional and correct per the guard clause — but it means that if future callers forget to pass resource, there will be no analytics and no warning. Consider adding a console.warn in dev mode if shouldShowStayUpdatedButton is true but resource is absent?
| <div>Page content</div> | ||
| </ProductPageTemplate>, | ||
| ) | ||
| if (showStayUpdated) { |
There was a problem hiding this comment.
Request: Could you simplify this to just pass showStayUpdated={showStayUpdated}?
| expect(mockedNiceModalShow).toHaveBeenCalled() | ||
| }) | ||
|
|
||
| describe("PostHog tracking", () => { |
There was a problem hiding this comment.
Are you saying your original comment was out of place? Let's mark this as resolved if so.
| posthog.capture(PostHogEvents.CallToActionClicked, { | ||
| label: "Stay Updated", | ||
| resourceId: resource!.id, | ||
| readableId: resource!.readable_id, | ||
| resourceType: resource!.resource_type, | ||
| }) |
There was a problem hiding this comment.
Technical request: Let's remove the non-null assertions (!.readable_id). These are dangerous and have caused errors in the past. Ideally we should have a linting or TS rule against them.
If you add if (!showStayUpdated || !resource) return earlier in the callback, typescript will narrow the type and you can avoid the non-null asseritons.
There was a problem hiding this comment.
Feature Issue: I think we have a issue/problem—both here and on the enrollment button—with resourceId. This is a numeric primary key id. the issue we have is where it's coming from... In some places (resource drawer, cards, search page) it's the Learn PK, and in some places (enrollment button, stay updated) it's the MITxOnline pk.
Request: Could you address this issue, both here and in CourseEnrollmentButton.tsx, ProgramEnrollmentButton.tsx?
My suggestion for addressing would be to just remove resourceId from the enrollment button and "stay updated" clicks, though attaching it as mitxonlineId would be OK, too.
In particular, here I'd send:
{
label: "Stay Updated",
readableId: resource.readable_id,
resourceType: resource.resource_type,
platform: PlatformEnum.Mitxonline,
}| } | ||
|
|
||
| export type ResourceInfo = { | ||
| id: number |
There was a problem hiding this comment.
I think you could remove this id param now, right?
d2cba12 to
ba31b4d
Compare
What are the relevant tickets?
https://github.com/mitodl/hq/issues/10916
Description (What does it do?)
Adds PostHog cta_clicked event tracking when a user clicks the "Stay Updated" button on Course, Program product pages
How can this be tested?
same test as #3218
To test the "Stay Updated" button on your local:
verified(no audit mode)env/frontend.local.envThen visit the corresponding program page on http://open.odl.local:8062 and the "Stay Updated" button should appear alongside the enrollment button.
Click on "Stay Updated" and verify
cta_clickedis captured in posthog with data attributeAdditional Context