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

Search bar only analytics fix #1555

Merged
merged 1 commit into from
Sep 24, 2021

Conversation

cea2aj
Copy link
Member

@cea2aj cea2aj commented Sep 24, 2021

Fix the analytics for the search bar only assets

When analyticsEventsEnabled was added to the SDK, the parameter order of the AnalyticsReporter constructor changed and it broke analytics for the search bar only assets. This caused analytics to only be sent if analyticsOptions was present in the config. The data sent was also messed up.

Fix the parameter order so that it is the same as the regular SDK

J=none
TEST=manual

Build the search bar only assets and test analytics with voice search and the clear button. See that analytics are properly sent even without analyticsOptions existing, and see that the data is now correct. Test setAnalyticsOptIn and confirm that it can disable analytics

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 59.304% when pulling f0692e7 on dev/searchbar-analytics-fix into 7f600ea on hotfix/search-bar/v1.1.1.

@cea2aj cea2aj merged commit 702e751 into hotfix/search-bar/v1.1.1 Sep 24, 2021
@cea2aj cea2aj mentioned this pull request Sep 24, 2021
yen-tt pushed a commit that referenced this pull request Sep 24, 2021
### Bugfixes
- Fix the analytics for the search bar only assets (#1555)
tmeyer2115 pushed a commit that referenced this pull request Oct 12, 2021
### Bugfixes
- Fix the analytics for the search bar only assets (#1555)
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