-
Notifications
You must be signed in to change notification settings - Fork 405
RI-6953: Send telemetry events for ReJSON used editor #4537
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,130 @@ | ||
import thunk from 'redux-thunk' | ||
import configureStore from 'redux-mock-store' | ||
import { EditorType } from 'uiSrc/slices/interfaces' | ||
|
||
const mockStore = configureStore([thunk]) | ||
|
||
const originalConsoleError = console.error | ||
|
||
// Suppress Redux warnings about missing reducers | ||
beforeAll(() => { | ||
console.error = (...args: any[]) => { | ||
const message = args[0] | ||
if ( | ||
typeof message === 'string' && | ||
message.includes('No reducer provided for key') | ||
) { | ||
return | ||
} | ||
|
||
originalConsoleError(...args) | ||
} | ||
}) | ||
|
||
afterAll(() => { | ||
console.error = originalConsoleError | ||
}) | ||
|
||
describe('setReJSONDataAction', () => { | ||
let store: any | ||
let sendEventTelemetryMock: jest.Mock | ||
let setReJSONDataAction: any | ||
let apiService: any | ||
|
||
beforeEach(async () => { | ||
jest.resetModules() | ||
|
||
sendEventTelemetryMock = jest.fn() | ||
|
||
jest.doMock('uiSrc/telemetry', () => { | ||
const actual = jest.requireActual('uiSrc/telemetry') | ||
return { | ||
...actual, | ||
sendEventTelemetry: sendEventTelemetryMock, | ||
getBasedOnViewTypeEvent: jest.fn(() => 'mocked_event'), | ||
} | ||
}) | ||
|
||
jest.doMock('uiSrc/slices/browser/keys', () => { | ||
const actual = jest.requireActual('uiSrc/slices/browser/keys') | ||
return { | ||
...actual, | ||
refreshKeyInfoAction: () => ({ type: 'DUMMY_REFRESH' }), | ||
} | ||
}) | ||
|
||
const rejson = await import('uiSrc/slices/browser/rejson') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why the dynamic imports? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mocking process for this file is a bit tricky. It seems something gets imported before Jest has a chance to apply the usual |
||
setReJSONDataAction = rejson.setReJSONDataAction | ||
apiService = (await import('uiSrc/services')).apiService | ||
|
||
store = mockStore({ | ||
browser: { | ||
rejson: { editorType: 'Default' }, | ||
keys: { viewType: 'Browser' }, | ||
}, | ||
app: { | ||
info: { encoding: 'utf8' }, | ||
}, | ||
connections: { | ||
instances: { | ||
connectedInstance: { | ||
id: 'instance-id', | ||
}, | ||
}, | ||
}, | ||
}) | ||
|
||
apiService.patch = jest.fn().mockResolvedValue({ status: 200 }) | ||
apiService.post = jest.fn().mockResolvedValue({ status: 200, data: {} }) | ||
|
||
jest.clearAllMocks() | ||
}) | ||
|
||
it('should call sendEventTelemetry with correct args', async () => { | ||
await store.dispatch(setReJSONDataAction('key', '$', '{}', true, 100)) | ||
|
||
expect(sendEventTelemetryMock).toHaveBeenCalledWith({ | ||
event: 'mocked_event', | ||
eventData: { | ||
databaseId: 'instance-id', | ||
keyLevel: 0, | ||
}, | ||
}) | ||
}) | ||
|
||
it('should set entireKey: true when editor is Text', async () => { | ||
store = mockStore({ | ||
...store.getState(), | ||
browser: { | ||
...store.getState().browser, | ||
rejson: { editorType: EditorType.Text }, | ||
}, | ||
}) | ||
|
||
await store.dispatch(setReJSONDataAction('key', '$', '{}', true, 100)) | ||
|
||
expect(sendEventTelemetryMock).toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
eventData: expect.objectContaining({ | ||
keyLevel: 'entireKey', | ||
}), | ||
}), | ||
) | ||
}) | ||
|
||
it('should compute keyLevel from nested path', async () => { | ||
const nestedPath = '$.foo.bar[1].nested.key' // 5 levels of nesting | ||
|
||
await store.dispatch( | ||
setReJSONDataAction('key', nestedPath, '{}', true, 100), | ||
) | ||
|
||
expect(sendEventTelemetryMock).toHaveBeenCalledWith( | ||
expect.objectContaining({ | ||
eventData: expect.objectContaining({ | ||
keyLevel: 5, | ||
}), | ||
}), | ||
) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,6 @@ | |
*/ | ||
import isGlob from 'is-glob' | ||
import { cloneDeep, get } from 'lodash' | ||
import jsonpath from 'jsonpath' | ||
import { Maybe, isRedisearchAvailable } from 'uiSrc/utils' | ||
import { ApiEndpoints, KeyTypes } from 'uiSrc/constants' | ||
import { KeyViewType } from 'uiSrc/slices/interfaces/keys' | ||
|
@@ -135,18 +134,17 @@ const getBasedOnViewTypeEvent = ( | |
} | ||
} | ||
|
||
const getJsonPathLevel = (path: string): string => { | ||
const getJsonPathLevel = (path: string): number => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think some tests verifying this funciton works as expected won't hurt |
||
try { | ||
if (path === '$') { | ||
return 'root' | ||
} | ||
const levelsLength = jsonpath.parse( | ||
`$${path.startsWith('$') ? '.' : '..'}${path}`, | ||
).length | ||
if (!path || path === '$') return 0 | ||
|
||
const stripped = path.startsWith('$.') ? path.slice(2) : path.slice(1) | ||
|
||
const parts = stripped.split(/[.[\]]/).filter(Boolean) | ||
|
||
return levelsLength === 2 ? 'root' : `${levelsLength - 2}` | ||
return parts.length | ||
} catch (e) { | ||
return 'root' | ||
return 0 | ||
} | ||
} | ||
|
||
|
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.
Why for this test specifically?
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 is a brand new test suite, and I'm adding it here. I'm suppressing the expected Redux warnings to keep the logs clean and free of noise.