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

[Vega] Add Filter custom label for opensearchDashboardsAddFilter #3640

Merged
merged 3 commits into from
Mar 24, 2023

Conversation

aswath86
Copy link
Contributor

@aswath86 aswath86 commented Mar 21, 2023

Description

Vega 'opensearchDashboardsAddFilter' allows to add OpenSearch Query DSL and index. The implementation buildQueryFilter takes in third parameter to set a custom label in 'Add Filter'. At the moment, opensearchDashboardsAddFilter can only take query and index parameters.

Issues Resolved

This PR adds in the third 'alias' parameter to set the custom label from Vega

buildQueryFilter = (query: QueryStringFilter['query'], index: string, alias: string)

Below image is self explanatory. The first filter in the image is without custom label (ugly, in my opinion), and the second filter is with custom label set in Vega.

Vega_OSDAddFilter

Check List

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

Signed-off-by: Aswath <it.aswath@gmail.com>
@aswath86 aswath86 requested a review from a team as a code owner March 21, 2023 10:28
Signed-off-by: Aswath <it.aswath@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2023

Codecov Report

Merging #3640 (8dbdf1b) into main (de06344) will increase coverage by 0.00%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #3640   +/-   ##
=======================================
  Coverage   66.45%   66.46%           
=======================================
  Files        3208     3209    +1     
  Lines       61593    61610   +17     
  Branches     9502     9505    +3     
=======================================
+ Hits        40932    40947   +15     
- Misses      18384    18385    +1     
- Partials     2277     2278    +1     
Flag Coverage Δ
Linux 66.40% <0.00%> (+<0.01%) ⬆️
Windows 66.41% <0.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...s/vis_type_vega/public/vega_view/vega_base_view.js 56.09% <0.00%> (ø)

... and 4 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

joshuarrrr
joshuarrrr previously approved these changes Mar 23, 2023
Signed-off-by: Aswath <it.aswath@gmail.com>

Co-authored-by: Josh Romero <rmerqg@amazon.com>
Copy link
Member

@joshuarrrr joshuarrrr left a comment

Choose a reason for hiding this comment

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

Love to see this addition. Thanks @aswath86 !

@joshuarrrr
Copy link
Member

Ideally, we'd love a functional or unit test for this, but there are no existing unit tests for vega_base_view.js. And while we could add a test case to

it('should set filter by calling "opensearchDashboardsAddFilter" expression', async () => {
await fillSpecAndGo(
getTestSpec('opensearchDashboardsAddFilter({ query_string: { query: "response:200" }})')
);
expect(await filterBar.getFilterCount()).to.be(1);
});

there's little benefit, because our testing service for the filterBar has no mechanisms for retrieving the custom label/filter alias we're now able to set: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/test/functional/services/filter_bar.ts

@joshuarrrr joshuarrrr merged commit 276fae9 into opensearch-project:main Mar 24, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 24, 2023
* [Vega] Add Filter custom label for opensearchDashboardsAddFilter

Signed-off-by: Aswath <it.aswath@gmail.com>

Co-authored-by: Josh Romero <rmerqg@amazon.com>

---------

Signed-off-by: Aswath <it.aswath@gmail.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit 276fae9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
joshuarrrr added a commit that referenced this pull request Mar 28, 2023
…sAddFilter (#3690)

* [Vega] Add Filter custom label for opensearchDashboardsAddFilter (#3640)

* [Vega] Add Filter custom label for opensearchDashboardsAddFilter

Signed-off-by: Aswath <it.aswath@gmail.com>

Co-authored-by: Josh Romero <rmerqg@amazon.com>

---------

Signed-off-by: Aswath <it.aswath@gmail.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit 276fae9)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* add changelog

Signed-off-by: Josh Romero <rmerqg@amazon.com>

---------

Signed-off-by: Aswath <it.aswath@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Signed-off-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
sikhote pushed a commit to sikhote/OpenSearch-Dashboards that referenced this pull request Apr 24, 2023
…nsearch-project#3640)

* [Vega] Add Filter custom label for opensearchDashboardsAddFilter

Signed-off-by: Aswath <it.aswath@gmail.com>

Co-authored-by: Josh Romero <rmerqg@amazon.com>

---------

Signed-off-by: Aswath <it.aswath@gmail.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: David Sinclair <david@sinclair.tech>
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.

4 participants