-
Notifications
You must be signed in to change notification settings - Fork 55
[AXON-1545] refactor rovodev analytics #1320
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
base: main
Are you sure you want to change the base?
Conversation
4de19fe to
42aca1d
Compare
| @@ -0,0 +1,175 @@ | |||
| // All Rovo Dev analytics events and types | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file replaces rovodevAnalyticsTypes with a different approach to event definition, but same content otherwise
| expect(Container.analyticsClient.sendTrackEvent).toHaveBeenCalledWith(mockEvent); | ||
| }); | ||
|
|
||
| // Rovo Dev event tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to src/rovo-dev/rovoDevTelemetryProvider.test.ts without changes to the assertions
| // Rovo Dev events, previously from src/analytics.ts | ||
| // TODO: refactor these, and particularly the usage in RovoDevTelemetryProvider, in a follow-up | ||
|
|
||
| export function rovoDevEntitlementCheckEvent(isEntitled: boolean, type: string, source?: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleted in favour of using explicit types and a discriminated union in analyticsApi.sendTrackEvent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw we have same func in analytics.ts file
just curious are these two same event?
Co-authored-by: atlassian[bot] <40802693+atlassian[bot]@users.noreply.github.com>
81d44d5 to
6baf15f
Compare
What Is This Change?
This PR is a somewhat broad rework of how analytics are implemented, scoped to
rovodev-related stuff.The main reasoning behind this is to avoid dragging the legacy implementation of analytics from into the new
rovodev, whatever shape that takes :DTelemetry,Performance, andDwellevents are defined in the same place and all sent throughextensionApi.analyticsWhat's NOT in this PR?
performanceEventinsrc/analytics.tsstill references rovodev but is now unused, it'll be cleaned up separatelytrackEventfromsrc/analytics.tsto enrich the events with stuff like userId - that will need to be moved separatelyHow Has This Been Tested?
Somewhat extensively :D
Basic checks:
npm run lintnpm run test