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

FFS-2492: store applicant id in session, use it when logging events as the dist… #460

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

millerti
Copy link
Contributor

@millerti millerti commented Feb 19, 2025

FFS-2492

Changes

Use applicant ID as distinct ID on Mixpanel

Testing Instructions

  1. Log a few events
  2. Log into Mixpanel and verify that the Distinct ID being used for your new events uses the Applicant ID

Acceptance testing

  • No acceptance testing needed
    • This change will not affect the user experience (bugfix, dependency updates, etc.)
  • Acceptance testing prior to merge
    • This change can be verified visually via screenshots attached below or by sending a link to a local development environment to the acceptance tester
    • Acceptance testing should be done by design for visual changes, product for behavior/logic changes, or both for changes that impact both.
  • Acceptance testing after merge
    • This change is hard to test locally, so we'll test it in the demo environment (deployed automatically after merge.)
    • Make sure to notify the team once this PR is merged so we don't inadvertently deploy the unaccepted change to production. (e.g. :alert: Deploy block! @ffs-eng I just merged PR [#123] and will be doing acceptance testing in demo - please don't deploy until I'm finished!)

@millerti millerti marked this pull request as ready for review February 19, 2025 20:50
@@ -90,6 +90,7 @@ def capture_page_view
event_logger.track("CbvPageView", request, {
cbv_flow_id: @cbv_flow.id,
invitation_id: @cbv_flow.cbv_flow_invitation_id,
cbv_applicant_id: @cbv_flow.cbv_applicant_id,
Copy link
Contributor

Choose a reason for hiding this comment

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

How will this work if the vast majority of the mixpanel events don't have the cbv_applicant_id field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it wouldn't. Maybe I'm misunderstanding how the applicant id works. I assumed that all people going through the cbv flow would have one, invited or not. But if that's not the case then maybe I should fall back to a distinct ID of "invite-#" when applicant ID is missing.

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've verified all the events have the applicant id

@millerti millerti requested a review from tdooner February 21, 2025 18:04
@allthesignals allthesignals changed the title store applicant id in session, use it when logging events as the dist… FFS-2492: store applicant id in session, use it when logging events as the dist… Feb 21, 2025
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.

2 participants