Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
fix: trim orderByExpression and harden e2e test
- Trim whitespace from orderByExpression before using it as the
  default ORDER BY to prevent whitespace-only values from producing
  invalid queries
- Use DEFAULT_LOGS_SOURCE_NAME for deterministic source selection
  in e2e test
- Wrap e2e test body in try/finally so cleanup always runs even
  if assertions fail

Made-with: Cursor
  • Loading branch information
dhable committed Mar 6, 2026
commit ee435c73d6acacddfc7657d30ad2f0534bf9d09d
3 changes: 2 additions & 1 deletion packages/app/src/DBSearchPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,8 @@ export function useDefaultOrderBy(sourceID: string | undefined | null) {
return useMemo(() => {
// If no source, return undefined so that the orderBy is not set incorrectly
if (!source) return undefined;
if (source.orderByExpression) return source.orderByExpression;
const trimmedOrderBy = source.orderByExpression?.trim();
if (trimmedOrderBy) return trimmedOrderBy;
return optimizeDefaultOrderBy(
source?.timestampValueExpression ?? '',
source?.displayedTimestampValueExpression,
Expand Down
80 changes: 39 additions & 41 deletions packages/app/tests/e2e/features/sources.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,55 +178,53 @@ test.describe('Sources Functionality', { tag: ['@sources'] }, () => {
async ({ page }) => {
const API_URL = getApiUrl();
const logSources = await getSources(page, 'log');
expect(logSources.length).toBeGreaterThan(0);
const source = logSources.find(
(s: any) => s.name === DEFAULT_LOGS_SOURCE_NAME,
);
expect(source).toBeDefined();

const source = logSources[0];
const sourceId = source._id;
const customOrderBy = 'Timestamp ASC';

await test.step('Set custom orderByExpression on the source', async () => {
const updateResponse = await page.request.put(
`${API_URL}/sources/${sourceId}`,
{
data: {
...source,
id: sourceId,
orderByExpression: customOrderBy,
try {
await test.step('Set custom orderByExpression on the source', async () => {
const updateResponse = await page.request.put(
`${API_URL}/sources/${sourceId}`,
{
data: {
...source,
id: sourceId,
orderByExpression: customOrderBy,
},
},
},
);
expect(updateResponse.ok()).toBeTruthy();
});

await test.step('Verify orderByExpression is persisted', async () => {
const updatedSources = await getSources(page, 'log');
const updatedSource = updatedSources.find(
(s: any) => s._id === sourceId,
);
expect(updatedSource).toBeDefined();
expect(updatedSource.orderByExpression).toBe(customOrderBy);
});
);
expect(updateResponse.ok()).toBeTruthy();
});

await test.step('Verify search results load with custom ORDER BY', async () => {
await searchPage.goto();
await searchPage.selectSource(source.name);
await searchPage.submitEmptySearch();
await expect(searchPage.table.firstRow).toBeVisible();
});
await test.step('Verify orderByExpression is persisted', async () => {
const updatedSources = await getSources(page, 'log');
const updatedSource = updatedSources.find(
(s: any) => s._id === sourceId,
);
expect(updatedSource).toBeDefined();
expect(updatedSource.orderByExpression).toBe(customOrderBy);
});

await test.step('Clean up: reset orderByExpression', async () => {
const resetResponse = await page.request.put(
`${API_URL}/sources/${sourceId}`,
{
data: {
...source,
id: sourceId,
orderByExpression: '',
},
await test.step('Verify search results load with custom ORDER BY', async () => {
await searchPage.goto();
await searchPage.selectSource(source.name);
await searchPage.submitEmptySearch();
await expect(searchPage.table.firstRow).toBeVisible();
});
} finally {
await page.request.put(`${API_URL}/sources/${sourceId}`, {
data: {
...source,
id: sourceId,
orderByExpression: '',
},
);
expect(resetResponse.ok()).toBeTruthy();
});
});
}
},
);
});
Copy link

Choose a reason for hiding this comment

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

The cleanup step is only reached if all prior steps pass. If an earlier step fails, the source's orderByExpression remains set and can affect other tests. Consider using an afterEach hook or wrapping the API calls in a try/finally block to guarantee cleanup.

Loading