Skip to content
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

[data] updates query and lang if language is not supported by query data #8749

Merged

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Oct 29, 2024

Description

If the query currently has a language set that is not valid for that dataset type then update the language and get the initial query for that language and dataset and then send the query.

The scenario we can see this is when we are on an index pattern, and DQL selected. Then from recent datasets select an S3 connection. The dataset updates but the language does not update so therefore the query is invalid.

Issues Resolved

n/a

Screenshot

Testing the changes

Changelog

  • fix: updates query and language if language is not supported by query data

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

If the query currently has a language set that is not valid for that dataset
type then update the language and get the initial query for that language
and dataset and then send the query.

The scenario we can see this is when we are on an index pattern, and DQL selected.
Then from recent datasets select an S3 connection. The dataset updates but the
language does not update so therefore the query is invalid.

Issue:
n/a

Signed-off-by: Kawika Avilla <[email protected]>
Copy link
Contributor

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 60.76%. Comparing base (48bfe2c) to head (096d189).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
.../public/query/query_string/query_string_manager.ts 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8749      +/-   ##
==========================================
+ Coverage   60.75%   60.76%   +0.01%     
==========================================
  Files        3798     3798              
  Lines       90684    90690       +6     
  Branches    14272    14274       +2     
==========================================
+ Hits        55091    55111      +20     
+ Misses      32094    32080      -14     
  Partials     3499     3499              
Flag Coverage Δ
Linux_1 29.05% <14.28%> (-0.01%) ⬇️
Linux_2 56.39% <ø> (ø)
Linux_3 37.61% <85.71%> (+0.01%) ⬆️
Linux_4 29.82% <14.28%> (-0.01%) ⬇️
Windows_1 29.09% <14.28%> (+0.01%) ⬆️
Windows_2 56.34% <ø> (ø)
Windows_3 37.61% <85.71%> (+<0.01%) ⬆️
Windows_4 29.82% <14.28%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kavilla kavilla added backport 2.x discover for discover reinvent labels Oct 29, 2024
Copy link
Contributor

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

ruanyl
ruanyl previously approved these changes Oct 30, 2024
});

// Show warning about language change
showWarning(this.notifications, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this just be an info?

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

it could be show warning is the helper function for this manager that passes an id that prevents duplicate notifications from popping up. so if the user constantly switches between recent data and somehow the current language is always invalid it won't keep adding a notification on the right side and keep pushing the duplicate ones upwards. it will just have a single notification available.

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot 2024-10-30 at 12 59 26 PM

@virajsanghvi
Copy link
Collaborator

Btw, looks like there's a cypress test failure related to discover

Signed-off-by: Kawika Avilla <[email protected]>
…penSearch-Dashboards-1 into kavilla/defaultlangaugefordata
@sejli
Copy link
Member

sejli commented Oct 30, 2024

Does this need a backport 2.18 label?

@ananzh ananzh added the v2.18.0 label Oct 30, 2024
if (supportedLanguages && !supportedLanguages.includes(newQuery.language)) {
// Get initial query with first supported language and new dataset
newQuery = this.getInitialQuery({
language: supportedLanguages[0],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it okay if language is undefined?

Copy link
Member Author

Choose a reason for hiding this comment

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

it shouldn't but with new features i'd imagine there prolly is some way a plugin developer can attempt. will add an issue

// Show warning about language change
showWarning(this.notifications, {
title: i18n.translate('data.languageChangeTitle', {
defaultMessage: 'Language Changed',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Language Changed',
defaultMessage: 'Language changed',

Copy link
Member Author

Choose a reason for hiding this comment

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

i'm not sure if this will retrigger requiring approvals so will do this in fast follow! thanks

@kavilla kavilla merged commit a70a7b8 into opensearch-project:main Oct 30, 2024
72 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 30, 2024
…ata (#8749)

* [data] updates query and lang if language is not supported by query data

If the query currently has a language set that is not valid for that dataset
type then update the language and get the initial query for that language
and dataset and then send the query.

The scenario we can see this is when we are on an index pattern, and DQL selected.
Then from recent datasets select an S3 connection. The dataset updates but the
language does not update so therefore the query is invalid.

Issue:
n/a

Signed-off-by: Kawika Avilla <[email protected]>

* Changeset file for PR #8749 created/updated

* update string in notification

Signed-off-by: Kawika Avilla <[email protected]>

---------

Signed-off-by: Kawika Avilla <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit a70a7b8)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.18 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.18 2.18
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.18
# Create a new branch
git switch --create backport/backport-8749-to-2.18
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 a70a7b83b0165c66391433ba98d4c09ad5127902
# Push it to GitHub
git push --set-upstream origin backport/backport-8749-to-2.18
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.18

Then, create a pull request where the base branch is 2.18 and the compare/head branch is backport/backport-8749-to-2.18.

ananzh pushed a commit that referenced this pull request Oct 31, 2024
…ata (#8749) (#8768)

* [data] updates query and lang if language is not supported by query data

If the query currently has a language set that is not valid for that dataset
type then update the language and get the initial query for that language
and dataset and then send the query.

The scenario we can see this is when we are on an index pattern, and DQL selected.
Then from recent datasets select an S3 connection. The dataset updates but the
language does not update so therefore the query is invalid.

Issue:
n/a



* Changeset file for PR #8749 created/updated

* update string in notification



---------



(cherry picked from commit a70a7b8)

Signed-off-by: Kawika Avilla <[email protected]>
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>
amsiglan pushed a commit to amsiglan/OpenSearch-Dashboards that referenced this pull request Nov 1, 2024
…ata (opensearch-project#8749)

* [data] updates query and lang if language is not supported by query data

If the query currently has a language set that is not valid for that dataset
type then update the language and get the initial query for that language
and dataset and then send the query.

The scenario we can see this is when we are on an index pattern, and DQL selected.
Then from recent datasets select an S3 connection. The dataset updates but the
language does not update so therefore the query is invalid.

Issue:
n/a

Signed-off-by: Kawika Avilla <[email protected]>

* Changeset file for PR opensearch-project#8749 created/updated

* update string in notification

Signed-off-by: Kawika Avilla <[email protected]>

---------

Signed-off-by: Kawika Avilla <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
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.

7 participants