fix: Correctly disable previous period query (#1528)

Closes HDX-3118

# Summary

This PR ensures that "previous period" queries are not run when `compareToPreviousPeriod` is undefined. Previously, these queries were running unnecessarily, increasing the burden on ClickHouse.
This commit is contained in:
Drew Davis 2025-12-29 10:51:36 -05:00 committed by GitHub
parent 5062d80daf
commit 141b4969b6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 133 additions and 1 deletions

View file

@ -0,0 +1,5 @@
---
"@hyperdx/app": patch
---
fix: Correctly disable previous period query

View file

@ -285,7 +285,7 @@ function DBTimeChartComponent({
useQueriedChartConfig(previousPeriodChartConfig, {
placeholderData: (prev: any) => prev,
queryKey: [queryKeyPrefix, previousPeriodChartConfig, 'chunked'],
enabled: enabled && config.compareToPreviousPeriod,
enabled: !!(enabled && config.compareToPreviousPeriod),
enableQueryChunking: true,
});

View file

@ -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);
});
});