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 adding a filter for scripted fields via Document Explorer cell actions #130895

Merged

Conversation

jughosta
Copy link
Contributor

Closes #130751

Summary

This PR fixes adding filters when user clicks on "Filter for"/"Filter out" buttons in the new Document Explorer grid. Now field types will be considered and scripted fields will be properly handled in the search request. A similar implementation for the legacy table view can be found here.

Apr-25-2022 15-13-51

How to test:

  1. Add a scripted field (Stack Management > Data view > Scripted fields (for a data view) > Create a scripted field) with return "test"; value
  2. Navigate back to Discover
  3. Check +/- cell actions for Document Explorer and the legacy table view. It should trigger ES queries with script filter. Same behaviour should be for +/- in Document Explorer flyout.

Checklist

@jughosta jughosta self-assigned this Apr 25, 2022
@jughosta jughosta added Feature:Discover Discover Application Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. bug Fixes for quality problems that affect the customer experience release_note:fix v8.3.0 auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 labels Apr 25, 2022
@jughosta jughosta force-pushed the 130751-document-explorer-scripted-field branch from 4c67cc5 to 3c79c71 Compare April 25, 2022 13:40
@jughosta jughosta marked this pull request as ready for review April 25, 2022 14:01
@jughosta jughosta requested a review from a team as a code owner April 25, 2022 14:01
@elasticmachine
Copy link
Contributor

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

@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 407.5KB 407.5KB +5.0B

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

cc @jughosta

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, thx for fixing this so quickly 🚅 ! Even if we are fixing 🚂 . Tested with cloud deployment, works as expected now 👍

mode: '+' | '-'
) {
const row = context.rows[rowIndex];
const flattened = flattenHit(row, context.indexPattern);
Copy link
Member

@kertal kertal Apr 26, 2022

Choose a reason for hiding this comment

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

just a side note, unrelated to this PR: we should not flatten our hits in the UI, too much flattening is being done a various places in Discover. We should migrate this to something like

data being fetched -> data being prepared (flattened) -> UI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kertal Yes, I remember we were discussing it. Created a ticket for it #130990

Copy link
Member

Choose a reason for hiding this comment

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

great, thanks!

@jughosta jughosta merged commit 60049fd into elastic:main Apr 26, 2022
@jughosta jughosta deleted the 130751-document-explorer-scripted-field branch April 26, 2022 15:25
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.2 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 130895

Questions ?

Please refer to the Backport tool documentation

jughosta added a commit to jughosta/kibana that referenced this pull request Apr 26, 2022
…ent Explorer view (elastic#130895)

(cherry picked from commit 60049fd)

# Conflicts:
#	src/plugins/discover/public/components/discover_grid/discover_grid_cell_actions.tsx
jughosta added a commit that referenced this pull request Apr 26, 2022
… Document Explorer view (#130895) (#130996)

* [Discover] Fix adding/removing a filter for a scripted field in Document Explorer view (#130895)

(cherry picked from commit 60049fd)

# Conflicts:
#	src/plugins/discover/public/components/discover_grid/discover_grid_cell_actions.tsx

* [Discover] Revert paths for backporting
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
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 bug Fixes for quality problems that affect the customer experience 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. v8.2.0 v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discover] Adding filter from scripted field no longer works in new doc table
5 participants