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

Make Idv::AnalyticsEventsEnhancer opt-out #10263

Merged
merged 3 commits into from
Mar 20, 2024

Conversation

matthinz
Copy link
Member

🎫 Ticket

Link to the relevant ticket:
LG-12657

🛠 Summary of changes

(This is a redo of #10230 as that had gotten pretty messy.)

In working on #10216, I realized that the list of events that Idv::AnalyticsEventsEnhancer enhances has gotten a little stale. Specifically, we haven't been adding new events to it.

This PR converts Idv::AnalyticsEventsEnhancer from opt in to opt out. All idv_ methods will be "enhanced" UNLESS they are included in the IGNORED_METHODS list.

Initially my plan was to increase the scope of of events that are enhanced. However, this resulted in a lot of required spec updates. To keep the scope of this PR a little more manageable, it sets IGNORED_METHODS such that there is no actual change in behavior–it is the inverse of the old DECORATED_METHODS var.

Don't include nil values here
Update AnalyticsEventsEnhancer so that it augments any event where the _method name_ starts with `^idv_` UNLESS it has been told not to.

changelog: Internal, Analytics, Add additional data to IdV analytics events by default.
@matthinz matthinz requested a review from a team March 18, 2024 21:39
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@matthinz matthinz merged commit 0fc7795 into main Mar 20, 2024
2 checks passed
@matthinz matthinz deleted the matthinz/12657-update-analytics-events-enchanter branch March 20, 2024 17:00
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