Skip to content

RI-7218 Add telemetry collection for Vector Search page #4811

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8c75ca1
feat(ui): collect telemetry on vector search page when running a query
valkirilov Aug 6, 2025
201467b
feat(ui): collect telemetry on vector search page when copy a query
valkirilov Aug 6, 2025
f79fe5f
Revert "feat(ui): collect telemetry on vector search page when copy a…
valkirilov Aug 7, 2025
4181c8b
reafctor(ui): copy the QueryCardHeader component for the Vector Search
valkirilov Aug 7, 2025
ac73644
feat(ui): collect telemetry on vector search page when copy a query
valkirilov Aug 7, 2025
02fbb30
feat(ui): collect telemetry on vector search page when re-run a query
valkirilov Aug 7, 2025
0ce5ac1
feat(ui): collect telemetry on vector search page when toggle full-sc…
valkirilov Aug 7, 2025
5b6f82a
feat(ui): collect telemetry on vector search page when collapse/uncol…
valkirilov Aug 7, 2025
9fe6641
feat(ui): collect telemetry on vector search page when clearing a que…
valkirilov Aug 7, 2025
9efa866
feat(ui): collect telemetry on vector search page when clearing all q…
valkirilov Aug 7, 2025
45a30e8
feat(ui): collect telemetry on vector search page when clearing query
valkirilov Aug 7, 2025
910e4a7
reafactor(ui): try to use real interfaces for the command execution f…
valkirilov Aug 7, 2025
05ca0b2
reafactor(ui): export telemetry for "query re-run" into util
valkirilov Aug 8, 2025
4159613
reafactor(ui): export telemetry for "query run" into util
valkirilov Aug 8, 2025
02fde1c
reafactor(ui): export telemetry for "query clear-all" into util
valkirilov Aug 8, 2025
8671bc7
reafactor(ui): export telemetry for "query clear editor" into util
valkirilov Aug 8, 2025
d6587d2
refactor(ui): remove copy/paste QueryCardHeader component
valkirilov Aug 8, 2025
49f5bee
refactor(ui): introduce context to keep information for the view mode…
valkirilov Aug 8, 2025
09b8e86
refactor(ui): use the view mode from the context to emit copy query c…
valkirilov Aug 8, 2025
accd720
reafactor(ui): export telemetry for "toggle query full-screen" into util
valkirilov Aug 8, 2025
836641f
refactor(ui): use the view mode from the context to emit collapse/exa…
valkirilov Aug 8, 2025
456ffd5
refactor(ui): use the view mode from the context to emit delete query…
valkirilov Aug 8, 2025
7a7477a
test(ui): fix tests by adding the new context provider
valkirilov Aug 8, 2025
822f3b4
test(ui): fix tests by adding the new context provider
valkirilov Aug 8, 2025
4ab9820
test(ui): fix tests by adding the new context provider
valkirilov Aug 8, 2025
da82437
refactor(ui): simplify the context to not allow state change
valkirilov Aug 8, 2025
044430e
refactor(ui): simplify the context
valkirilov Aug 8, 2025
264e320
Merge remote-tracking branch 'origin/feature/RI-6855/vector-search' i…
valkirilov Aug 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React from 'react'
import { render, screen } from '@testing-library/react'

import {
useViewModeContext,
ViewMode,
ViewModeContextProvider,
} from './view-mode.context'

// Test component to consume the context
const TestComponent: React.FC = () => {
const { viewMode } = useViewModeContext()

return (
<div>
<p data-testid="view-mode">Current View Mode: {viewMode}</p>
</div>
)
}

describe('ViewModeContext', () => {
it('provides the default view mode', () => {
render(
<ViewModeContextProvider>
<TestComponent />
</ViewModeContextProvider>,
)

expect(screen.getByTestId('view-mode')).toHaveTextContent(
`Current View Mode: ${ViewMode.Workbench}`,
)
})

it('uses the initial view mode if provided', () => {
render(
<ViewModeContextProvider viewMode={ViewMode.VectorSearch}>
<TestComponent />
</ViewModeContextProvider>,
)

expect(screen.getByTestId('view-mode')).toHaveTextContent(
`Current View Mode: ${ViewMode.VectorSearch}`,
)
})
})
33 changes: 33 additions & 0 deletions redisinsight/ui/src/components/query/context/view-mode.context.tsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

So basically, this is a context that shares single value across components, right?
Is there any reason not to just add this to Redux store?

Copy link
Collaborator

@KrumTy KrumTy Aug 8, 2025

Choose a reason for hiding this comment

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

it's used in different views now. It would not make sense to dispatch redux actions to update the store as you switch between Search and Workbench tabs

This context solution is the least path of resistance to reusing the QueryCard component. The other being prop drilling, which @valkirilov said was too hard (4-5 levels deep).

The pre-context solution was to copy the whole component and it's test and adjust 3-4 telemetry lines.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Its already merged, but I still fail to see how context "dispatch" makes more sense than redux dispatch.
If anything this adds more techincal overhead

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React, { createContext, ReactNode, useContext } from 'react'

export enum ViewMode {
Workbench = 'workbench',
VectorSearch = 'vector-search',
}

interface ViewModeContextType {
viewMode: ViewMode
}

const ViewModeContext = createContext<ViewModeContextType>({
viewMode: ViewMode.Workbench,
})

// Props for the provider
interface ViewModeContextProviderProps {
children: ReactNode
viewMode?: ViewMode
}

export const ViewModeContextProvider: React.FC<
ViewModeContextProviderProps
> = ({ children, viewMode = ViewMode.Workbench }) => {
return (
<ViewModeContext.Provider value={{ viewMode }}>
{children}
</ViewModeContext.Provider>
)
}

export const useViewModeContext = (): ViewModeContextType =>
useContext(ViewModeContext)
150 changes: 74 additions & 76 deletions redisinsight/ui/src/components/query/query-card/QueryCard.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from 'uiSrc/utils/test-utils'
import { CommandExecutionStatus } from 'uiSrc/slices/interfaces/cli'
import QueryCard, { Props, getSummaryText } from './QueryCard'
import { ViewMode, ViewModeContextProvider } from '../context/view-mode.context'

const mockedProps = mock<Props>()

Expand Down Expand Up @@ -44,15 +45,27 @@ jest.mock('uiSrc/slices/app/plugins', () => ({
}),
}))

const renderQueryCardComponent = (props: Partial<Props> = {}) => {
return render(
<ViewModeContextProvider viewMode={ViewMode.Workbench}>
<QueryCard {...instance(mockedProps)} {...props} />
</ViewModeContextProvider>,
{
store,
},
)
}

describe('QueryCard', () => {
it('should render', () => {
expect(render(<QueryCard {...instance(mockedProps)} />)).toBeTruthy()
const { container } = renderQueryCardComponent()
expect(container).toBeTruthy()
})

it('Cli result should not in the document before Expand', () => {
const cliResultTestId = 'query-cli-result'

const { queryByTestId } = render(<QueryCard {...instance(mockedProps)} />)
const { queryByTestId } = renderQueryCardComponent()

const cliResultEl = queryByTestId(cliResultTestId)
expect(cliResultEl).not.toBeInTheDocument()
Expand All @@ -61,9 +74,10 @@ describe('QueryCard', () => {
it('Cli result should in the document when "isOpen = true"', () => {
const cliResultTestId = 'query-cli-result'

const { queryByTestId } = render(
<QueryCard {...instance(mockedProps)} isOpen result={mockResult} />,
)
const { queryByTestId } = renderQueryCardComponent({
isOpen: true,
result: mockResult,
})

const cliResultEl = queryByTestId(cliResultTestId)

Expand All @@ -73,13 +87,10 @@ describe('QueryCard', () => {
it('Cli result should not in the document when "isOpen = false"', () => {
const cliResultTestId = 'query-cli-result'

const { queryByTestId } = render(
<QueryCard
{...instance(mockedProps)}
isOpen={false}
result={mockResult}
/>,
)
const { queryByTestId } = renderQueryCardComponent({
isOpen: false,
result: mockResult,
})

const cliResultEl = queryByTestId(cliResultTestId)

Expand All @@ -89,14 +100,11 @@ describe('QueryCard', () => {
it('Should be in the document when resultsMode === ResultsMode.GroupMode', () => {
const cliResultTestId = 'query-cli-result'

const { queryByTestId } = render(
<QueryCard
{...instance(mockedProps)}
isOpen={false}
result={mockResult}
resultsMode={ResultsMode.GroupMode}
/>,
)
const { queryByTestId } = renderQueryCardComponent({
isOpen: false,
result: mockResult,
resultsMode: ResultsMode.GroupMode,
})

const cliResultEl = queryByTestId(cliResultTestId)

Expand All @@ -107,9 +115,10 @@ describe('QueryCard', () => {
const cardHeaderTestId = 'query-card-open'
const mockId = '123'

const { queryByTestId } = render(
<QueryCard {...instance(mockedProps)} id={mockId} result={mockResult} />,
)
const { queryByTestId } = renderQueryCardComponent({
id: mockId,
result: mockResult,
})

const cardHeaderTestEl = queryByTestId(cardHeaderTestId)

Expand All @@ -131,15 +140,13 @@ describe('QueryCard', () => {
})

it('should render QueryCardCliResultWrapper when command is null', () => {
const { queryByTestId } = render(
<QueryCard
{...instance(mockedProps)}
resultsMode={ResultsMode.GroupMode}
result={null}
isOpen
command={null}
/>,
)
const { queryByTestId } = renderQueryCardComponent({
resultsMode: ResultsMode.GroupMode,
result: null,
isOpen: true,
command: null,
})

const queryCommonResultEl = queryByTestId('query-common-result-wrapper')
const queryCliResultEl = queryByTestId('query-cli-result-wrapper')

Expand All @@ -148,62 +155,53 @@ describe('QueryCard', () => {
})

it('should render QueryCardCliResult when result reached response size threshold', () => {
const { queryByTestId } = render(
<QueryCard
{...instance(mockedProps)}
resultsMode={ResultsMode.GroupMode}
result={[
{
status: CommandExecutionStatus.Success,
response: 'Any message about size limit threshold exceeded',
sizeLimitExceeded: true,
},
]}
isOpen
command={null}
/>,
)
const { queryByTestId } = renderQueryCardComponent({
resultsMode: ResultsMode.GroupMode,
result: [
{
status: CommandExecutionStatus.Success,
response: 'Any message about size limit threshold exceeded',
sizeLimitExceeded: true,
},
],
isOpen: true,
command: null,
})
const queryCliResultEl = queryByTestId('query-cli-result')

expect(queryCliResultEl).toBeInTheDocument()
})

it('should render properly result when it has pure number', () => {
const { getByTestId } = render(
<QueryCard
{...instance(mockedProps)}
resultsMode={ResultsMode.GroupMode}
result={[
{
status: CommandExecutionStatus.Success,
response: 1,
},
]}
isOpen
command="del key"
/>,
)
const { getByTestId } = renderQueryCardComponent({
resultsMode: ResultsMode.GroupMode,
result: [
{
status: CommandExecutionStatus.Success,
response: 1,
},
],
isOpen: true,
command: 'del key',
})
const queryCliResultEl = getByTestId('query-cli-result')

expect(queryCliResultEl.textContent).toBe('(integer) 1')
})

it('should render QueryCardCliResult when result reached response size threshold even w/o flag', () => {
const { queryByTestId } = render(
<QueryCard
{...instance(mockedProps)}
resultsMode={ResultsMode.GroupMode}
result={[
{
status: CommandExecutionStatus.Success,
response:
'Results have been deleted since they exceed 1 MB. Re-run the command to see new results.',
},
]}
isOpen
command={null}
/>,
)
const { queryByTestId } = renderQueryCardComponent({
resultsMode: ResultsMode.GroupMode,
result: [
{
status: CommandExecutionStatus.Success,
response:
'Results have been deleted since they exceed 1 MB. Re-run the command to see new results.',
},
],
isOpen: true,
command: null,
})
const queryCliResultEl = queryByTestId('query-cli-result')

expect(queryCliResultEl).toBeInTheDocument()
Expand Down
Loading
Loading