mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
Fix bug where loading saved search from another page (#1563)
Fixes a bug where loading a saved search page from another non-search page may result in the default select/order by being used instead of the saved search values. Attempts to make these types of errors happen less by creating a new `defaultSearchConfig` object which represents the default values to fallback on (currently it was mixing concerns in a few places) also adds tests for the scenario Fixes HDX-3149
This commit is contained in:
Brandon Pereira
GitHub
parent
af3bbcf1a0
commit
1a9362e736
6 changed files with 345 additions and 37 deletions
5
.changeset/eighty-walls-wait.md
Normal file
5
.changeset/eighty-walls-wait.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@hyperdx/app": patch
|
||||
---
|
||||
|
||||
Fix bug where loading saved search from another page might use default values instead
|
||||
|
|
@ -622,14 +622,14 @@ function useLiveUpdate({
|
|||
]);
|
||||
}
|
||||
|
||||
function useSearchedConfigToChartConfig({
|
||||
select,
|
||||
source,
|
||||
whereLanguage,
|
||||
where,
|
||||
filters,
|
||||
orderBy,
|
||||
}: SearchConfig) {
|
||||
/**
|
||||
* Takes in a input search config (user edited search config) and a default search config (saved search or source default config)
|
||||
* and returns a chart config.
|
||||
*/
|
||||
function useSearchedConfigToChartConfig(
|
||||
{ select, source, whereLanguage, where, filters, orderBy }: SearchConfig,
|
||||
defaultSearchConfig?: Partial<SearchConfig>,
|
||||
) {
|
||||
const { data: sourceObj, isLoading } = useSource({
|
||||
id: source,
|
||||
});
|
||||
|
|
@ -639,7 +639,11 @@ function useSearchedConfigToChartConfig({
|
|||
if (sourceObj != null) {
|
||||
return {
|
||||
data: {
|
||||
select: select || (sourceObj.defaultTableSelectExpression ?? ''),
|
||||
select:
|
||||
select ||
|
||||
defaultSearchConfig?.select ||
|
||||
sourceObj.defaultTableSelectExpression ||
|
||||
'',
|
||||
from: sourceObj.from,
|
||||
source: sourceObj.id,
|
||||
...(sourceObj.tableFilterExpression != null
|
||||
|
|
@ -660,7 +664,7 @@ function useSearchedConfigToChartConfig({
|
|||
implicitColumnExpression: sourceObj.implicitColumnExpression,
|
||||
connection: sourceObj.connection,
|
||||
displayType: DisplayType.Search,
|
||||
orderBy: orderBy || defaultOrderBy,
|
||||
orderBy: orderBy || defaultSearchConfig?.orderBy || defaultOrderBy,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
@ -671,6 +675,7 @@ function useSearchedConfigToChartConfig({
|
|||
isLoading,
|
||||
select,
|
||||
filters,
|
||||
defaultSearchConfig,
|
||||
where,
|
||||
whereLanguage,
|
||||
defaultOrderBy,
|
||||
|
|
@ -752,6 +757,9 @@ export function useDefaultOrderBy(sourceID: string | undefined | null) {
|
|||
const { data: source } = useSource({ id: sourceID });
|
||||
const { data: tableMetadata } = useTableMetadata(tcFromSource(source));
|
||||
|
||||
// If no source, return undefined so that the orderBy is not set incorrectly
|
||||
if (!source) return undefined;
|
||||
|
||||
// When source changes, make sure select and orderby fields are set to default
|
||||
return useMemo(
|
||||
() =>
|
||||
|
|
@ -807,11 +815,6 @@ function DBSearchPage() {
|
|||
]).withDefault('results'),
|
||||
);
|
||||
|
||||
const [outlierSqlCondition, setOutlierSqlCondition] = useQueryState(
|
||||
'outlierSqlCondition',
|
||||
parseAsString,
|
||||
);
|
||||
|
||||
const [isLive, setIsLive] = useQueryState(
|
||||
'isLive',
|
||||
parseAsBoolean.withDefault(true),
|
||||
|
|
@ -867,12 +870,34 @@ function DBSearchPage() {
|
|||
});
|
||||
|
||||
const inputSource = useWatch({ name: 'source', control });
|
||||
|
||||
const defaultOrderBy = useDefaultOrderBy(inputSource);
|
||||
|
||||
// The default search config to use when the user hasn't changed the search config
|
||||
const defaultSearchConfig = useMemo(() => {
|
||||
let _savedSearch = savedSearch;
|
||||
// Ensure to not use the saved search if the saved search id is not the same as the current saved search id
|
||||
if (!savedSearchId || savedSearch?.id !== savedSearchId) {
|
||||
_savedSearch = undefined;
|
||||
}
|
||||
// Ensure to not use the saved search if the input source is not the same as the saved search source
|
||||
if (inputSource !== savedSearch?.source) {
|
||||
_savedSearch = undefined;
|
||||
}
|
||||
return {
|
||||
select:
|
||||
_savedSearch?.select ?? searchedSource?.defaultTableSelectExpression,
|
||||
where: _savedSearch?.where ?? '',
|
||||
whereLanguage: _savedSearch?.whereLanguage ?? 'lucene',
|
||||
source: _savedSearch?.source,
|
||||
orderBy: _savedSearch?.orderBy || defaultOrderBy,
|
||||
};
|
||||
}, [searchedSource, inputSource, savedSearch, defaultOrderBy, savedSearchId]);
|
||||
|
||||
// const { data: inputSourceObj } = useSource({ id: inputSource });
|
||||
const { data: inputSourceObjs } = useSources();
|
||||
const inputSourceObj = inputSourceObjs?.find(s => s.id === inputSource);
|
||||
|
||||
const defaultOrderBy = useDefaultOrderBy(inputSource);
|
||||
|
||||
const [displayedTimeInputValue, setDisplayedTimeInputValue] =
|
||||
useState('Live Tail');
|
||||
|
||||
|
|
@ -1023,14 +1048,14 @@ function DBSearchPage() {
|
|||
// Save the selected source ID to localStorage
|
||||
setLastSelectedSourceId(newInputSourceObj.id);
|
||||
|
||||
// If the user is in a saved search, prefer the saved search's select if available
|
||||
// If the user isn't in a saved search (or the source is different from the saved search source), reset fields
|
||||
if (savedSearchId == null || savedSearch?.source !== watchedSource) {
|
||||
setValue(
|
||||
'select',
|
||||
newInputSourceObj?.defaultTableSelectExpression ?? '',
|
||||
);
|
||||
setValue('select', '');
|
||||
setValue('orderBy', '');
|
||||
// If the user is in a saved search, prefer the saved search's select/orderBy if available
|
||||
} else {
|
||||
setValue('select', savedSearch?.select ?? '');
|
||||
setValue('orderBy', savedSearch?.orderBy ?? '');
|
||||
}
|
||||
// Clear all search filters
|
||||
searchFilters.clearAllFilters();
|
||||
|
|
@ -1066,7 +1091,7 @@ function DBSearchPage() {
|
|||
>(undefined);
|
||||
|
||||
const { data: chartConfig, isLoading: isChartConfigLoading } =
|
||||
useSearchedConfigToChartConfig(searchedConfig);
|
||||
useSearchedConfigToChartConfig(searchedConfig, defaultSearchConfig);
|
||||
|
||||
// query error handling
|
||||
const { hasQueryError, queryError } = useMemo(() => {
|
||||
|
|
@ -1239,11 +1264,9 @@ function DBSearchPage() {
|
|||
const displayedColumns = useMemo(
|
||||
() =>
|
||||
splitAndTrimWithBracket(
|
||||
dbSqlRowTableConfig?.select ??
|
||||
searchedSource?.defaultTableSelectExpression ??
|
||||
'',
|
||||
dbSqlRowTableConfig?.select ?? defaultSearchConfig.select ?? '',
|
||||
),
|
||||
[dbSqlRowTableConfig?.select, searchedSource?.defaultTableSelectExpression],
|
||||
[dbSqlRowTableConfig?.select, defaultSearchConfig.select],
|
||||
);
|
||||
|
||||
const toggleColumn = useCallback(
|
||||
|
|
@ -1397,10 +1420,10 @@ function DBSearchPage() {
|
|||
setSearchedConfig({
|
||||
orderBy: sort
|
||||
? `${sort.id} ${sort.desc ? 'DESC' : 'ASC'}`
|
||||
: defaultOrderBy,
|
||||
: defaultSearchConfig.orderBy,
|
||||
});
|
||||
},
|
||||
[setIsLive, defaultOrderBy, setSearchedConfig],
|
||||
[setIsLive, defaultSearchConfig.orderBy, setSearchedConfig],
|
||||
);
|
||||
// Parse the orderBy string into a SortingState. We need the string
|
||||
// version in other places so we keep this parser separate.
|
||||
|
|
@ -1578,10 +1601,8 @@ function DBSearchPage() {
|
|||
tableConnection={inputSourceTableConnection}
|
||||
control={control}
|
||||
name="select"
|
||||
defaultValue={inputSourceObj?.defaultTableSelectExpression}
|
||||
placeholder={
|
||||
inputSourceObj?.defaultTableSelectExpression || 'SELECT Columns'
|
||||
}
|
||||
defaultValue={defaultSearchConfig.select}
|
||||
placeholder={defaultSearchConfig.select || 'SELECT Columns'}
|
||||
onSubmit={onSubmit}
|
||||
label="SELECT"
|
||||
size="xs"
|
||||
|
|
@ -1592,7 +1613,7 @@ function DBSearchPage() {
|
|||
tableConnection={inputSourceTableConnection}
|
||||
control={control}
|
||||
name="orderBy"
|
||||
defaultValue={defaultOrderBy}
|
||||
defaultValue={defaultSearchConfig.orderBy}
|
||||
onSubmit={onSubmit}
|
||||
label="ORDER BY"
|
||||
size="xs"
|
||||
|
|
@ -1758,7 +1779,6 @@ function DBSearchPage() {
|
|||
<SaveSearchModal
|
||||
opened={saveSearchModalState != null}
|
||||
onClose={clearSaveSearchModalState}
|
||||
// @ts-ignore FIXME: Do some sort of validation?
|
||||
searchedConfig={searchedConfig}
|
||||
isUpdate={saveSearchModalState === 'update'}
|
||||
savedSearchId={savedSearchId}
|
||||
|
|
|
|||
|
|
@ -203,7 +203,7 @@ describe('useDefaultOrderBy', () => {
|
|||
|
||||
const { result } = renderHook(() => useDefaultOrderBy(null));
|
||||
|
||||
expect(result.current).toBe(' DESC');
|
||||
expect(result.current).toBe(undefined);
|
||||
});
|
||||
|
||||
it('should handle undefined sourceID ungracefully', () => {
|
||||
|
|
@ -221,7 +221,7 @@ describe('useDefaultOrderBy', () => {
|
|||
|
||||
const { result } = renderHook(() => useDefaultOrderBy(undefined));
|
||||
|
||||
expect(result.current).toBe(' DESC');
|
||||
expect(result.current).toBe(undefined);
|
||||
});
|
||||
|
||||
it('should handle complex Timestamp expressions', () => {
|
||||
|
|
|
|||
|
|
@ -1229,6 +1229,20 @@ function DBSqlRowTableComponent({
|
|||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [sourceId]);
|
||||
|
||||
// Sync local orderBy state with initialSortBy when it changes
|
||||
// (e.g., when loading a saved search)
|
||||
const prevInitialSortBy = usePrevious(initialSortBy);
|
||||
useEffect(() => {
|
||||
const currentSort = initialSortBy?.[0] ?? null;
|
||||
const prevSort = prevInitialSortBy?.[0] ?? null;
|
||||
|
||||
// Only sync if initialSortBy actually changed (not orderBy)
|
||||
// We don't include orderBy in deps to avoid infinite loop
|
||||
if (JSON.stringify(currentSort) !== JSON.stringify(prevSort)) {
|
||||
setOrderBy(currentSort);
|
||||
}
|
||||
}, [initialSortBy, prevInitialSortBy]);
|
||||
|
||||
const mergedConfigObj = useMemo(() => {
|
||||
const base = {
|
||||
...searchChartConfigDefaults(me?.team),
|
||||
|
|
|
|||
|
|
@ -185,4 +185,255 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
|
|||
});
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'should load saved search when navigating from another page',
|
||||
{ tag: '@full-stack' },
|
||||
async ({ page }) => {
|
||||
/**
|
||||
* This test verifies the fix for the issue where saved searches would not
|
||||
* load properly when users navigate to them from another page (e.g., service map).
|
||||
*
|
||||
* Test flow:
|
||||
* 1. Create a saved search with custom configuration (WHERE and ORDER BY)
|
||||
* 2. Navigate to a different page (service map)
|
||||
* 3. Navigate to the saved search URL
|
||||
* 4. Verify saved search loaded correctly with all configuration restored
|
||||
*/
|
||||
|
||||
let savedSearchUrl: string;
|
||||
const customOrderBy = 'ServiceName ASC';
|
||||
|
||||
await test.step('Create a saved search with custom WHERE and ORDER BY', async () => {
|
||||
// Set up a custom search with WHERE clause
|
||||
// Use SeverityText which is a valid column in the demo data
|
||||
await searchPage.performSearch('SeverityText:info');
|
||||
|
||||
// Set custom ORDER BY
|
||||
await searchPage.setCustomOrderBy(customOrderBy);
|
||||
|
||||
// Submit the search to ensure configuration is applied
|
||||
await searchPage.submitButton.click();
|
||||
await searchPage.table.waitForRowsToPopulate();
|
||||
|
||||
// Save the search
|
||||
await searchPage.openSaveSearchModal();
|
||||
await searchPage.savedSearchModal.saveSearch(
|
||||
'Info Logs Navigation Test',
|
||||
);
|
||||
|
||||
// Wait for save to complete and URL to change
|
||||
await expect(searchPage.savedSearchModal.container).toBeHidden();
|
||||
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
|
||||
|
||||
// Capture the saved search URL (without query params)
|
||||
savedSearchUrl = page.url().split('?')[0];
|
||||
});
|
||||
|
||||
await test.step('Navigate to a different page (service map)', async () => {
|
||||
// Navigate to service map page
|
||||
await page.goto('/service-map');
|
||||
|
||||
// Wait for service map page to load
|
||||
await expect(page.getByTestId('service-map-page')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Navigate to saved search from service map', async () => {
|
||||
// Navigate directly to the saved search URL (simulating clicking a link)
|
||||
await page.goto(savedSearchUrl);
|
||||
|
||||
// Wait for the search page to load
|
||||
await expect(page.getByTestId('search-page')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Verify saved search loaded and executed automatically', async () => {
|
||||
// Verify the WHERE clause is populated
|
||||
const whereInput = searchPage.input;
|
||||
await expect(whereInput).toHaveValue('SeverityText:info');
|
||||
|
||||
// Verify ORDER BY is restored
|
||||
const orderByEditor = searchPage.getOrderByEditor();
|
||||
const orderByContent = await orderByEditor.textContent();
|
||||
expect(orderByContent).toContain('ServiceName ASC');
|
||||
|
||||
// Verify search results are visible (search executed automatically)
|
||||
await searchPage.table.waitForRowsToPopulate();
|
||||
const rowCount = await searchPage.table.getRows().count();
|
||||
expect(rowCount).toBeGreaterThan(0);
|
||||
|
||||
// Verify the search actually ran (not just showing cached results)
|
||||
const resultsTable = searchPage.getSearchResultsTable();
|
||||
await expect(resultsTable).toBeVisible();
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'should preserve custom SELECT when loading saved search from another page',
|
||||
{ tag: '@full-stack' },
|
||||
async ({ page }) => {
|
||||
/**
|
||||
* This test specifically verifies that custom SELECT statements are preserved
|
||||
* when navigating to a saved search from another page.
|
||||
*/
|
||||
|
||||
let savedSearchUrl: string;
|
||||
const customSelect =
|
||||
'Timestamp, Body, upper(ServiceName) as service_name';
|
||||
|
||||
await test.step('Create saved search with custom SELECT', async () => {
|
||||
await searchPage.setCustomSELECT(customSelect);
|
||||
await searchPage.performSearch('ServiceName:frontend');
|
||||
await searchPage.openSaveSearchModal();
|
||||
await searchPage.savedSearchModal.saveSearch(
|
||||
'Custom Select Navigation Test',
|
||||
);
|
||||
|
||||
await expect(searchPage.savedSearchModal.container).toBeHidden();
|
||||
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
|
||||
|
||||
savedSearchUrl = page.url().split('?')[0];
|
||||
});
|
||||
|
||||
await test.step('Navigate to dashboards page', async () => {
|
||||
await page.goto('/dashboards');
|
||||
await expect(page.getByTestId('dashboard-page')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Navigate back to saved search', async () => {
|
||||
await page.goto(savedSearchUrl);
|
||||
await expect(page.getByTestId('search-page')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Verify custom SELECT is preserved', async () => {
|
||||
// Wait for results to load
|
||||
await searchPage.table.waitForRowsToPopulate();
|
||||
|
||||
// Verify SELECT content
|
||||
const selectEditor = searchPage.getSELECTEditor();
|
||||
const selectContent = await selectEditor.textContent();
|
||||
|
||||
expect(selectContent).toContain('upper(ServiceName) as service_name');
|
||||
expect(selectContent).toContain('Timestamp, Body');
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'should handle navigation via browser back button',
|
||||
{ tag: '@full-stack' },
|
||||
async ({ page }) => {
|
||||
/**
|
||||
* This test verifies that using browser back/forward navigation
|
||||
* properly loads saved searches.
|
||||
*/
|
||||
|
||||
await test.step('Create and save a search', async () => {
|
||||
await searchPage.performSearch('SeverityText:info');
|
||||
await searchPage.openSaveSearchModal();
|
||||
await searchPage.savedSearchModal.saveSearch('Browser Navigation Test');
|
||||
|
||||
await expect(searchPage.savedSearchModal.container).toBeHidden();
|
||||
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
|
||||
});
|
||||
|
||||
await test.step('Navigate to sessions page', async () => {
|
||||
await page.goto('/sessions');
|
||||
await expect(page.getByTestId('sessions-page')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Use browser back button', async () => {
|
||||
await page.goBack();
|
||||
await expect(page.getByTestId('search-page')).toBeVisible();
|
||||
});
|
||||
|
||||
await test.step('Verify saved search loads correctly after back navigation', async () => {
|
||||
// Verify WHERE clause
|
||||
const whereInput = searchPage.input;
|
||||
await expect(whereInput).toHaveValue('SeverityText:info');
|
||||
|
||||
// Verify results load
|
||||
await searchPage.table.waitForRowsToPopulate();
|
||||
const rowCount = await searchPage.table.getRows().count();
|
||||
expect(rowCount).toBeGreaterThan(0);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'should update ORDER BY when switching sources multiple times',
|
||||
{ tag: '@full-stack' },
|
||||
async ({ page }) => {
|
||||
/**
|
||||
* This test verifies the fix for the issue where ORDER BY does not update
|
||||
* correctly after the first source change on saved search pages.
|
||||
*
|
||||
* Reproduction steps:
|
||||
* 1. Create saved search with custom ORDER BY on Source A
|
||||
* 2. Switch to Source B (ORDER BY should change to Source B's default)
|
||||
* 3. Switch back to Source A (ORDER BY should restore to saved search's custom value)
|
||||
*/
|
||||
|
||||
let originalSourceName: string | null = null;
|
||||
let secondSourceName: string | null = null;
|
||||
const thirdSourceName: string | null = null;
|
||||
const customOrderBy = 'Body DESC';
|
||||
|
||||
await test.step('Create saved search with custom ORDER BY', async () => {
|
||||
originalSourceName = await searchPage.currentSource.inputValue();
|
||||
|
||||
// Set custom ORDER BY
|
||||
await searchPage.setCustomOrderBy(customOrderBy);
|
||||
|
||||
// Submit and save the search
|
||||
await searchPage.submitEmptySearch();
|
||||
await searchPage.openSaveSearchModal();
|
||||
await searchPage.savedSearchModal.saveSearch(
|
||||
'ORDER BY Multiple Source Switch Test',
|
||||
);
|
||||
|
||||
await expect(searchPage.savedSearchModal.container).toBeHidden();
|
||||
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
|
||||
});
|
||||
|
||||
await test.step('Switch to second source', async () => {
|
||||
await searchPage.sourceDropdown.click();
|
||||
secondSourceName = await searchPage.otherSources.first().textContent();
|
||||
await searchPage.otherSources.first().click();
|
||||
await page.waitForLoadState('networkidle');
|
||||
await searchPage.table.waitForRowsToPopulate();
|
||||
});
|
||||
|
||||
await test.step('Verify ORDER BY changed to second source default', async () => {
|
||||
const orderByEditor = searchPage.getOrderByEditor();
|
||||
const orderByContent = await orderByEditor.textContent();
|
||||
|
||||
// Should not contain the custom ORDER BY from the saved search
|
||||
|
||||
await expect(orderByEditor).not.toHaveText('Body DESC', {
|
||||
timeout: 5000,
|
||||
});
|
||||
await expect(orderByEditor).toHaveText(/(Timestamp|timestamp)/i, {
|
||||
timeout: 5000,
|
||||
});
|
||||
});
|
||||
|
||||
await test.step('Switch back to original source', async () => {
|
||||
await searchPage.sourceDropdown.click();
|
||||
await page
|
||||
.getByRole('option', {
|
||||
name: originalSourceName || '',
|
||||
exact: true,
|
||||
})
|
||||
.click();
|
||||
await page.waitForLoadState('networkidle');
|
||||
await searchPage.table.waitForRowsToPopulate();
|
||||
});
|
||||
|
||||
await test.step('Verify ORDER BY restored to saved search custom value', async () => {
|
||||
const orderByEditor = searchPage.getOrderByEditor();
|
||||
await expect(orderByEditor).toHaveText('Body DESC', { timeout: 5000 });
|
||||
});
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -188,6 +188,13 @@ export class SearchPage {
|
|||
return this.page.locator('.cm-content').first();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get ORDER BY editor (CodeMirror)
|
||||
*/
|
||||
getOrderByEditor() {
|
||||
return this.page.locator('.cm-content').nth(1);
|
||||
}
|
||||
|
||||
/**
|
||||
* Set custom SELECT columns
|
||||
*/
|
||||
|
|
@ -197,6 +204,17 @@ export class SearchPage {
|
|||
await this.page.keyboard.type(selectStatement);
|
||||
}
|
||||
|
||||
/**
|
||||
* Set custom ORDER BY clause
|
||||
*/
|
||||
async setCustomOrderBy(orderByStatement: string) {
|
||||
const orderByEditor = this.getOrderByEditor();
|
||||
await orderByEditor.click({ clickCount: 3 }); // Select all
|
||||
await this.page.keyboard.type(orderByStatement);
|
||||
// CLoses Autocomplete Modal if open
|
||||
await this.page.keyboard.press('Escape');
|
||||
}
|
||||
|
||||
/**
|
||||
* Get histogram chart
|
||||
*/
|
||||
|
|
|
|||
Loading…
Reference in a new issue