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

[Discover] Fix csv report for filtered discover result on dashboard #119583

Merged

Conversation

dimaanj
Copy link
Contributor

@dimaanj dimaanj commented Nov 24, 2021

Summary

Fixes #119452

This PR takes into consideration discover filters when generating csv report.

Test notes

  • Open discover
  • Create saved search with columns, apply filter
  • Create dashboard with saved search
  • Generate csv report via panel actions
  • Result should be filtered

@dimaanj dimaanj added Feature:Discover Discover Application (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:fix and removed (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead labels Nov 24, 2021
@dimaanj dimaanj marked this pull request as ready for review November 24, 2021 16:01
@dimaanj dimaanj requested a review from a team as a code owner November 24, 2021 16:01
@dimaanj dimaanj requested a review from tsullivan November 24, 2021 16:01
@dimaanj
Copy link
Contributor Author

dimaanj commented Nov 24, 2021

@elasticmachine merge upstream

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, will test it later on. Could you add a test for it? A functional for this one would be 🍰

@dimaanj dimaanj requested review from a team as code owners November 25, 2021 12:21
@dimaanj dimaanj requested a review from kertal November 25, 2021 14:29
@kertal
Copy link
Member

kertal commented Nov 29, 2021

@elasticmachine merge upstream

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested a-la-carte using Chrome, created a saved search with filter, added to a dashboard, exported CSV. Result was as expected, filter added to saved search was considered in the CSV. Nice work! 👍

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

LGTM, did not test locally

? data.query.timefilter.timefilter.createFilter(index)
: data.query.timefilter.timefilter.createRelativeFilter(index);

searchSource.setField('filter', filter);
if (existingFilter && timeFilter) {
searchSource.setField(
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder how common this append to filter approach is. Perhaps it should be part of search source @elastic/kibana-app-services ?

Copy link
Member

Choose a reason for hiding this comment

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

…ilters were applied for non time based data views
@dimaanj
Copy link
Contributor Author

dimaanj commented Nov 30, 2021

@elasticmachine merge upstream

@dimaanj dimaanj added auto-backport Deprecated - use backport:version if exact versions are needed and removed v7.15.1 labels Nov 30, 2021
@dimaanj dimaanj added the Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. label Nov 30, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@kertal
Copy link
Member

kertal commented Dec 1, 2021

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
discover 312.2KB 312.4KB +204.0B
Unknown metric groups

References to deprecated APIs

id before after diff
discover 1824 1830 +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @dmitriynj

@dimaanj dimaanj merged commit 79079a7 into elastic:main Dec 1, 2021
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.16 Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 119583

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 3, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 119583 or prevent reminders by adding the backport:skip label.

tsullivan pushed a commit to tsullivan/kibana that referenced this pull request Dec 3, 2021
…lastic#119583)

* [Discover] fix csv report for filtered discover result on dashboard

* [Discover] fix tests

* [Discover] add functional test

* [Discover] fix the case when timeFilter was not applied, but others filters were applied for non time based data views

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	src/plugins/discover/public/application/apps/main/utils/get_sharing_data.ts
tsullivan pushed a commit to tsullivan/kibana that referenced this pull request Dec 3, 2021
…lastic#119583)

* [Discover] fix csv report for filtered discover result on dashboard

* [Discover] fix tests

* [Discover] add functional test

* [Discover] fix the case when timeFilter was not applied, but others filters were applied for non time based data views

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	src/plugins/discover/public/utils/get_sharing_data.ts
tsullivan added a commit that referenced this pull request Dec 3, 2021
…119583) (#120418)

* [Discover] fix csv report for filtered discover result on dashboard

* [Discover] fix tests

* [Discover] add functional test

* [Discover] fix the case when timeFilter was not applied, but others filters were applied for non time based data views

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	src/plugins/discover/public/application/apps/main/utils/get_sharing_data.ts

Co-authored-by: Dmitry Tomashevich <39378793+Dmitriynj@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 3, 2021
tsullivan added a commit that referenced this pull request Dec 3, 2021
…119583) (#120419)

* [Discover] fix csv report for filtered discover result on dashboard

* [Discover] fix tests

* [Discover] add functional test

* [Discover] fix the case when timeFilter was not applied, but others filters were applied for non time based data views

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
# Conflicts:
#	src/plugins/discover/public/utils/get_sharing_data.ts

Co-authored-by: Dmitry Tomashevich <39378793+Dmitriynj@users.noreply.github.com>
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
…lastic#119583)

* [Discover] fix csv report for filtered discover result on dashboard

* [Discover] fix tests

* [Discover] add functional test

* [Discover] fix the case when timeFilter was not applied, but others filters were applied for non time based data views

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Discover Discover Application release_note:fix Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v7.16.1 v7.17.0 v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard][Discover] Filters saved with Saved Searches are not respected when downloading CSV files
7 participants