-
Notifications
You must be signed in to change notification settings - Fork 920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove unsupported languages for dataset #8100
Remove unsupported languages for dataset #8100
Conversation
@@ -36,6 +38,9 @@ export const QueryLanguageSelector = (props: QueryLanguageSelectorProps) => { | |||
const queryString = getQueryService().queryString; | |||
const languageService = queryString.getLanguageService(); | |||
|
|||
const type = queryString.getDatasetService().getType(props.dataset!.type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to avoid non-null assertion as it's one of the major cause of runtime exception.
const type = queryString.getDatasetService().getType(props.dataset!.type); | |
const type = props.dataset ? queryString.getDatasetService().getType(props.dataset.type) : undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use conditional operator
languageService.getUserQueryLanguageBlocklist().includes(language?.id) | ||
) | ||
return; | ||
languageOptions.unshift(mapExternalLanguageToOptions(language!)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems you don't need non-null assertion here
languageOptions.unshift(mapExternalLanguageToOptions(language!)); | |
languageOptions.unshift(mapExternalLanguageToOptions(language)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8100 +/- ##
==========================================
+ Coverage 63.71% 63.84% +0.13%
==========================================
Files 3738 3738
Lines 88682 88702 +20
Branches 13788 13795 +7
==========================================
+ Hits 56501 56631 +130
+ Misses 31593 31474 -119
- Partials 588 597 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
b2eeb68
to
1dafc7e
Compare
|
||
export interface QueryLanguageSelectorProps { | ||
query: Query; | ||
onSelectLanguage: (newLanguage: string) => void; | ||
anchorPosition?: PopoverAnchorPosition; | ||
appName?: string; | ||
dataset?: Dataset; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesnt the query object already contain the dataset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, removed extra prop
.forEach((language) => { | ||
if ( | ||
(language && props.appName && !language.editorSupportedAppNames?.includes(props.appName)) || | ||
languageService.getUserQueryLanguageBlocklist().includes(language?.id) | ||
) | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be in filter()
, since it's basically just filtering it again but in the forEach()
loop? Then, we can pass the mapExternalLanguageOptions()
function into a .map()
instead of a for loop. Not a blocking comment, just something that might make it a bit easier more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combined forEach and filter
Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com>
* remove unsupported languages Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> * Changeset file for PR #8100 created/updated * address comments Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> * address comments Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> --------- Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 42317f1) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* remove unsupported languages * Changeset file for PR #8100 created/updated * address comments * address comments --------- (cherry picked from commit 42317f1) Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Co-authored-by: Qingyang(Abby) Hu <abigailhu2000@gmail.com>
* remove unsupported languages Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> * Changeset file for PR #8100 created/updated * address comments Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> * address comments Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> --------- Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit 42317f1) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* remove unsupported languages * Changeset file for PR #8100 created/updated * address comments * address comments --------- (cherry picked from commit 42317f1) Signed-off-by: abbyhu2000 <abigailhu2000@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
…8100) (opensearch-project#8262)" This reverts commit cb3b4f6.
Description
Remove unsupported languages for dataset in language selector
Issues Resolved
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration