Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/olive-suns-repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@hyperdx/app": patch
---

fix: Correctly disable previous period query
2 changes: 1 addition & 1 deletion packages/app/src/components/DBTimeChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ function DBTimeChartComponent({
useQueriedChartConfig(previousPeriodChartConfig, {
placeholderData: (prev: any) => prev,
queryKey: [queryKeyPrefix, previousPeriodChartConfig, 'chunked'],
enabled: enabled && config.compareToPreviousPeriod,
enabled: !!(enabled && config.compareToPreviousPeriod),
Copy link
Contributor

Choose a reason for hiding this comment

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

would love a quick test added if possible. Okay to punt it if it takes longer than 5m

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, added!

enableQueryChunking: true,
});

Expand Down
127 changes: 127 additions & 0 deletions packages/app/src/components/__tests__/DBTimeChart.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import React from 'react';

import api from '@/api';
import { useQueriedChartConfig } from '@/hooks/useChartConfig';
import { useSource } from '@/source';

import { DBTimeChart } from '../DBTimeChart';

// Mock dependencies
jest.mock('@/hooks/useChartConfig', () => ({
useQueriedChartConfig: jest.fn(),
}));

jest.mock('@/api', () => ({
__esModule: true,
default: {
useMe: jest.fn(),
},
}));

jest.mock('@/source', () => ({
useSource: jest.fn(),
}));

describe('DBTimeChart', () => {
const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock;
const mockUseMe = api.useMe as jest.Mock;
const mockUseSource = useSource as jest.Mock;

const baseTestConfig = {
dateRange: [new Date('2024-01-01'), new Date('2024-01-02')] as [Date, Date],
from: { databaseName: 'test', tableName: 'test' },
timestampValueExpression: 'timestamp',
connection: 'test-connection',
select: 'value',
where: '',
};

beforeEach(() => {
jest.clearAllMocks();

mockUseMe.mockReturnValue({
data: { team: { parallelizeWhenPossible: false } },
isLoading: false,
});

mockUseSource.mockReturnValue({
data: undefined,
isLoading: false,
});

mockUseQueriedChartConfig.mockReturnValue({
data: {
data: [{ timestamp: 1704067200, value: 100 }],
meta: [],
rows: 1,
isComplete: true,
},
isLoading: false,
isError: false,
isSuccess: true,
isPlaceholderData: false,
});
});

it('passes enabled: false to useQueriedChartConfig for previous period when compareToPreviousPeriod is undefined', () => {
const config = {
...baseTestConfig,
compareToPreviousPeriod: undefined,
};

renderWithMantine(<DBTimeChart config={config} />);

// Get the second call (previous period query)
const [, secondCallOptions] = mockUseQueriedChartConfig.mock.calls[1];

// Verify that enabled is false for the previous period query
expect(secondCallOptions.enabled).toBe(false);
});

it('passes enabled: true to useQueriedChartConfig for previous period when compareToPreviousPeriod is true', () => {
const config = {
...baseTestConfig,
compareToPreviousPeriod: true,
};

renderWithMantine(<DBTimeChart config={config} />);

// Get the second call (previous period query)
const [, secondCallOptions] = mockUseQueriedChartConfig.mock.calls[1];

// Verify that enabled is true for the previous period query
expect(secondCallOptions.enabled).toBe(true);
});

it('passes enabled: false to useQueriedChartConfig for previous period when compareToPreviousPeriod is false', () => {
const config = {
...baseTestConfig,
compareToPreviousPeriod: false,
};

renderWithMantine(<DBTimeChart config={config} />);

// Get the second call (previous period query)
const [, secondCallOptions] = mockUseQueriedChartConfig.mock.calls[1];

// Verify that enabled is false for the previous period query
expect(secondCallOptions.enabled).toBe(false);
});

it('respects the enabled prop when determining if previous period query should run', () => {
const config = {
...baseTestConfig,
compareToPreviousPeriod: true,
};

// Render with enabled=false
renderWithMantine(<DBTimeChart config={config} enabled={false} />);

// Get the second call (previous period query)
const [, secondCallOptions] = mockUseQueriedChartConfig.mock.calls[1];

// Verify that enabled is false even when compareToPreviousPeriod is true
// because the enabled prop is false
expect(secondCallOptions.enabled).toBe(false);
});
});