diff --git a/.changeset/eighty-walls-wait.md b/.changeset/eighty-walls-wait.md new file mode 100644 index 000000000..834428f7a --- /dev/null +++ b/.changeset/eighty-walls-wait.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": patch +--- + +Fix bug where loading saved search from another page might use default values instead diff --git a/packages/app/src/DBSearchPage.tsx b/packages/app/src/DBSearchPage.tsx index 45b5585ae..2391a1f51 100644 --- a/packages/app/src/DBSearchPage.tsx +++ b/packages/app/src/DBSearchPage.tsx @@ -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, +) { 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() { { 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', () => { diff --git a/packages/app/src/components/DBRowTable.tsx b/packages/app/src/components/DBRowTable.tsx index 7e3d2131e..823195ced 100644 --- a/packages/app/src/components/DBRowTable.tsx +++ b/packages/app/src/components/DBRowTable.tsx @@ -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), diff --git a/packages/app/tests/e2e/features/search/saved-search.spec.ts b/packages/app/tests/e2e/features/search/saved-search.spec.ts index 1df1c0002..807864b59 100644 --- a/packages/app/tests/e2e/features/search/saved-search.spec.ts +++ b/packages/app/tests/e2e/features/search/saved-search.spec.ts @@ -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 }); + }); + }, + ); }); diff --git a/packages/app/tests/e2e/page-objects/SearchPage.ts b/packages/app/tests/e2e/page-objects/SearchPage.ts index 115228608..56518be58 100644 --- a/packages/app/tests/e2e/page-objects/SearchPage.ts +++ b/packages/app/tests/e2e/page-objects/SearchPage.ts @@ -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 */