Repository navigation
Conversation
PR Reviewer Guide 🔍(Review updated until commit 82d92fe)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 82d92fe Explore these optional code suggestions:
Previous suggestionsSuggestions up to commit 3aa8625
Suggestions up to commit 0f070fe
Suggestions up to commit 82d92fe
Suggestions up to commit 2055775
|
Replace _.isEqual(payload.query, currentQuery) with a custom isEqualQuery helper that only compares the query and language fields. The bug: _.isEqual treats an object with dataset: undefined as unequal to one that lacks the dataset key entirely. This caused the Discover tab's refresh button to silently stop triggering refetches after state transitions. Fixes opensearch-project#12522 Co-Authored-By: Bug Analysis by @lbeltran-dev Signed-off-by: Shatrughan Rai <polyglot.dev@outlook.com>
|
Persistent review updated to latest commit 82d92fe |
…lQuery The previous isEqualQuery used === for the query field, which fails when query is a DSL object (two structurally-equal objects are always different). It also ignored dataset and profile fields entirely, meaning changing the dataset while keeping the same query text would not trigger a refresh. - Use _.isEqual() for the query field to handle both string and DSL object bodies correctly - Add normalization for dataset (?? null) so undefined and missing keys are equivalent, but real value differences are still detected - Add normalization for profile (?? false) with the same behavior Addresses PR review feedback for opensearch-project#12562 Signed-off-by: Shatrughan Rai <polyglot.dev@outlook.com>
|
Persistent review updated to latest commit 0f070fe |
…lQuery The previous isEqualQuery used === for the query field, which fails when query is a DSL object (two structurally-equal objects are always different). It also ignored dataset and profile fields entirely, meaning changing the dataset while keeping the same query text would not trigger a refresh. - Use _.isEqual() for the query field to handle both string and DSL object bodies correctly - Add normalization for dataset (?? null) so undefined and missing keys are equivalent, but real value differences are still detected - Add normalization for profile (?? false) with the same behavior Addresses PR review feedback for opensearch-project#12562 Signed-off-by: Shatrughan Rai <polyglot.dev@outlook.com>
|
Persistent review updated to latest commit 3aa8625 |
|
Persistent review updated to latest commit 82d92fe |
|
Replace _.isEqual(payload.query, currentQuery) with a custom isEqualQuery helper that only compares the query and language fields.
The bug: _.isEqual treats an object with dataset: undefined as unequal to one that lacks the dataset key entirely. This caused the Discover tab's refresh button to silently stop triggering refetches after state transitions.
Fixes #12522
Co-Authored-By: Bug Analysis by @lbeltran-dev
Description
Fix the Discover tab's refresh button becoming non-functional after state transitions.
The root cause was _.isEqual(payload.query, currentQuery) returning false when payload.query had dataset: undefined
as an own enumerable property while currentQuery lacked the dataset key entirely. This is a known limitation of
deep equality checks — the presence of an undefined value is treated as structurally different from the absence of
the key.
The fix introduces a custom isEqualQuery helper that explicitly compares only the meaningful fields (query and
language), ignoring structural differences in optional properties. This ensures the refresh button correctly
detects when a query update is needed.
Issues Resolved
Fixes #12522
Screenshot
Testing the changes
Unit tests: Run the new test suite for create_search_bar:
All 8 tests pass, including the regression test for the dataset: undefined edge case.
Full unit test suite:
All 3072 test suites pass (29608 tests), with no regressions from this change.
Integration tests:
Passes (the single pre-existing failure in basic_optimization.test.ts is unrelated to this PR).
Manual verification:
Check List