Skip to content

Conversation

@brandon-pereira
Copy link
Member

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

…t values instead

- also adds tests for the scenario

Fixes HDX-3149
@brandon-pereira brandon-pereira requested review from a team and pulpdrew and removed request for a team January 6, 2026 22:31
@changeset-bot
Copy link

changeset-bot bot commented Jan 6, 2026

🦋 Changeset detected

Latest commit: 7c1c27f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Jan 6, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Jan 7, 2026 11:12pm

@claude
Copy link

claude bot commented Jan 6, 2026

Code Review

No critical issues found.

The implementation correctly addresses the saved search state management bug. Key improvements:

  • ✅ Centralized default config logic in defaultSearchConfig memo
  • ✅ Proper invalidation when switching sources (lines 880-886)
  • ✅ Added sync mechanism in DBRowTable for initialSortBy changes (lines 1232-1244)
  • ✅ Fixed useDefaultOrderBy to return undefined when no source (line 761)
  • ✅ Comprehensive E2E test coverage for navigation scenarios

Minor observations (non-blocking):

  • Line 377-379 in test file declares secondSourceName and thirdSourceName but only uses secondSourceName - thirdSourceName can be removed
  • Line 667: The fallback chain is deep (orderBy || defaultSearchConfig?.orderBy || defaultOrderBy) - this is acceptable but worth monitoring for future edge cases

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2026

E2E Test Results

All tests passed • 59 passed • 4 skipped • 740s

Status Count
✅ Passed 59
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 4

Tests ran across 4 shards in parallel.

View full report →

Copy link
Contributor

@pulpdrew pulpdrew left a comment

Choose a reason for hiding this comment

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

LGTM

@kodiakhq kodiakhq bot merged commit 1a9362e into main Jan 8, 2026
14 checks passed
@kodiakhq kodiakhq bot deleted the brandon/search-page-initialization-improvements branch January 8, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants