Select the correct language when a different one is chosen by the dataset selector #8671
Conversation
…aset selector Authored-by: Suchit Sahoo <suchsah@amazon.com> Signed-off-by: Miki <miki@amazon.com>
AMoo-Miki
added
backport 2.x
discover
AMoo-Miki
requested review from
ananzh,
kavilla,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs,
ZilongX,
Flyingliuhub,
curq,
bandinib-amzn,
SuZhou-Joe,
ruanyl,
BionIT,
xinruiba,
zhyuanqi,
mengweieric,
LDrago27,
virajsanghvi,
sejli,
joshuali925 and
huyaboo
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8671 +/- ##
=======================================
Coverage 60.85% 60.85%
=======================================
Files 3793 3793
Lines 90368 90368
Branches 14181 14181
=======================================
Hits 54997 54997
- Misses 31893 31894 +1
+ Partials 3478 3477 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| const newQuery = this.queryString.getInitialQuery(); | ||
| const newQueryString = newQuery.query; | ||
| if (this.inputRef.getValue() !== newQueryString) { | ||
| const newQueryString = this.props.query.query; |
There was a problem hiding this comment.
i think now when i select the dataset for the new language the query string is not updated with the updated dataset.
There was a problem hiding this comment.
recreation steps:
- have an index pattern and navigate to discover. load results
- select an index from a cluster.
- notice how the query (if for sql and ppl) still references the old dataset
| @@ -44,7 +44,7 @@ export const QueryLanguageSelector = (props: QueryLanguageSelectorProps) => { | |||
| : undefined; | |||
|
|
|||
| useEffect(() => { | |||
| const subscription = queryString.getUpdates$().subscribe((query: Query) => { | |||
| const subscription = queryString.getUpdates$(0).subscribe((query: Query) => { | |||
There was a problem hiding this comment.
is it potentially just a race condition or something?
could we verify if the props.query.language is the current query always. and this component gets re-rendered. i need to verify since last checking these things out. or if you have the insight already.
because what im thinking is that potentially the dataset is checking which languages are supported and adding to a list while this is listening to changes and before the language is added this setter is called but set to the first language it found.
there's a couple of places i think were we need loading spinners. shooting in the dark perhaps we just need to check if everything is done loading before setting the current language?
There was a problem hiding this comment.
or opt for just disabling unsupported languages and setting the current value vs removing them from view
|
Marking as draft because we have another fix. Will close once Miki confirms that there is nothing in this PR thats needs to be picked up |
Description
Select the correct language when a different one is chosen by the dataset selector
Changelog
Check List
yarn test:jestyarn test:jest_integration