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

Possibility to filter when testing scripted fields (#35379) #44220

Merged
merged 16 commits into from
Dec 27, 2019

Conversation

friol
Copy link
Contributor

@friol friol commented Aug 28, 2019

Summary

As asked in issue #35379 , I've added the possibility to perform a filter query when testing your scripted field, since not always the first 10 records are significant.

@elasticmachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@spalger
Copy link
Contributor

spalger commented Aug 28, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger
Copy link
Contributor

spalger commented Aug 28, 2019

retest

@lukeelmers lukeelmers added review Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Aug 30, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@lukeelmers lukeelmers changed the title Possibility to filter when testing scripted fields (#35379) [release_note:enhancement] Possibility to filter when testing scripted fields (#35379) Aug 30, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@TinaHeiligers
Copy link
Contributor

@friol could you please update this PR by rebasing against master?

@spalger
Copy link
Contributor

spalger commented Sep 16, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@friol
Copy link
Contributor Author

friol commented Nov 28, 2019

@spalger : Can anybody please take a look at this one? This is useful! Thanks :)

@spalger spalger requested a review from nreese December 4, 2019 07:37
@nreese
Copy link
Contributor

nreese commented Dec 4, 2019

@elasticmachine merge upstream

Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

@friol thanks for making this PR! 🎉

I'm part of the Kibana Design team and I've justed created a PR with a few design changes:

  • We normally use a space of 16px in between form rows
  • We use our Elastic UI variables to address paddings and margins instead of pixels
  • I changed one of the class names to adhere to the BEM principle

Here's the PR: friol#5

Test script design improvement
@friol
Copy link
Contributor Author

friol commented Dec 12, 2019

@friol thanks for making this PR! 🎉

I'm part of the Kibana Design team and I've justed created a PR with a few design changes:

  • We normally use a space of 16px in between form rows
  • We use our Elastic UI variables to address paddings and margins instead of pixels
  • I changed one of the class names to adhere to the BEM principle

Here's the PR: friol#5

@miukimiu done :)

Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

Thanks @friol! LGTM! 🎉

@nreese
Copy link
Contributor

nreese commented Dec 12, 2019

@elasticmachine merge upstream

@nreese
Copy link
Contributor

nreese commented Dec 13, 2019

retest

@nreese
Copy link
Contributor

nreese commented Dec 13, 2019

@friol CI is failing for this PR. You will need to mock out TestScript in https://github.com/elastic/kibana/blob/master/src/legacy/ui/public/field_editor/components/scripting_help/help_flyout.test.js#L28 by adding the following.

jest.mock('./test_script', () => ({
  TestScript: () => {
    return (<div>mockTestScript</div>);
  }
}));

Then run node scripts/jest help_flyout -u to update the snapshot since the mocked TestScript component does not set the default prop name.

@friol
Copy link
Contributor Author

friol commented Dec 22, 2019

@nreese : I aligned my local repository with this branch and recompiled/restarted. Now, when I access Kibana, I get this error:

Version: 8.0.0
Build: 9007199254740991
TypeError: Cannot read property 'SearchBar' of undefined
at Object../src/legacy/ui/public/field_editor/components/scripting_help/test_script.js (http://localhost:5601/jmq/bundles/kibana.bundle.js:19120:34)
at webpack_require (http://localhost:5601/jmq/bundles/kibana.bundle.js:93:30)
at Object../src/legacy/ui/public/field_editor/components/scripting_help/help_flyout.js (http://localhost:5601/jmq/bundles/kibana.bundle.js:18811:20)
at webpack_require (http://localhost:5601/jmq/bundles/kibana.bundle.js:93:30)
at Object../src/legacy/ui/public/field_editor/components/scripting_help/index.js (http://localhost:5601/jmq/bundles/kibana.bundle.js:18883:20)
at webpack_require (http://localhost:5601/jmq/bundles/kibana.bundle.js:93:30)
at Object../src/legacy/ui/public/field_editor/field_editor.js (http://localhost:5601/jmq/bundles/kibana.bundle.js:19437:23)
at webpack_require (http://localhost:5601/jmq/bundles/kibana.bundle.js:93:30)
at Object../src/legacy/ui/public/field_editor/index.js (http://localhost:5601/jmq/bundles/kibana.bundle.js:20273:21)
at webpack_require (http://localhost:5601/jmq/bundles/kibana.bundle.js:93:30)

Why?

@nreese
Copy link
Contributor

nreese commented Dec 26, 2019

Looks like SearchBar was migrated to Kibana's new platform and is now imported from npStart.plugins.data.ui. Here is the PR that made the change - #52622.

@friol
Copy link
Contributor Author

friol commented Dec 26, 2019

@nreese: ok, done, and also, updated the tests. Thanks!

@nreese
Copy link
Contributor

nreese commented Dec 26, 2019

@elasticmachine merge upstream

@nreese
Copy link
Contributor

nreese commented Dec 27, 2019

retest

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@elastic elastic deleted a comment from elasticmachine Dec 27, 2019
@nreese nreese merged commit 3ed5264 into elastic:master Dec 27, 2019
@nreese
Copy link
Contributor

nreese commented Dec 27, 2019

@friol Thanks for the contribution. This is a great enhancement.

nreese added a commit to nreese/kibana that referenced this pull request Dec 27, 2019
…lastic#44220)

* Possibility to filter when testing scripted fields

* Possibility to filter when testing scripted fields

* Now the i18n test should pass

* use SearchBar instead of EuiFieldText input

* clean up

* test script design improvement

* Fixed SearchBar reference and updated the help_flyout tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Nathan Reese <reese.nathan@gmail.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
@nreese
Copy link
Contributor

nreese commented Dec 27, 2019

7.6 #53813

@friol
Copy link
Contributor Author

friol commented Dec 27, 2019

@friol Thanks for the contribution. This is a great enhancement.

@nreese : this was my 1st PR to Kibana and the one that made me start contributing to the project.
Thanks for your support and for making this real even after 4 months :)

nreese added a commit that referenced this pull request Dec 27, 2019
…53813)

* Possibility to filter when testing scripted fields

* Possibility to filter when testing scripted fields

* Now the i18n test should pass

* use SearchBar instead of EuiFieldText input

* clean up

* test script design improvement

* Fixed SearchBar reference and updated the help_flyout tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Nathan Reese <reese.nathan@gmail.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>

Co-authored-by: friol <dantonag@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 27, 2019
* master:
  Possibility to filter when testing scripted fields (elastic#35379) (elastic#44220)
  Update maps telemetry mappings to account for recent updates (elastic#53803)
  [Maps] Only show legend when layer is visible (elastic#53781)
  remove use of experimental fs.promises api (elastic#53346)
  [APM] Add log statements for flaky test (elastic#53775)
jloleysens added a commit to jloleysens/kibana that referenced this pull request Dec 30, 2019
…le-saved-objects

* 'master' of github.com:elastic/kibana: (250 commits)
  Allow chromeless applications to render via non-/app routes (elastic#51527)
  Add server rendering service to enable standalone route rendering (elastic#52161)
  Possibility to filter when testing scripted fields (elastic#35379) (elastic#44220)
  Update maps telemetry mappings to account for recent updates (elastic#53803)
  [Maps] Only show legend when layer is visible (elastic#53781)
  remove use of experimental fs.promises api (elastic#53346)
  [APM] Add log statements for flaky test (elastic#53775)
  [APM] Transaction page throws unhandled exception if transactions doesn't have  `http.request` (elastic#53760)
  Licensing plugin functional tests (elastic#53580)
  [Lens] Disable saving visualization until there are no changes to the document (elastic#52982)
  [Monitoring] Added safeguard for some EUI components (elastic#53318)
  [Vega] Shim new platform - cleanup vega_visualization dependencies (elastic#53605)
  Display changed field formats without requiring hard page refresh. (elastic#53746)
  Upgrade EUI to v17.3.1 (elastic#53655)
  [APM] Fix missing apm indicies (elastic#53541)
  Disable inspector for timelion (elastic#53747)
  Clean up search servie (elastic#53701)
  [Dashboard] Grid: removing double handler (elastic#53707)
  Remove SavedObjectRegistryProvider from codebase (elastic#53455)
  Move ui/courier into data shim plugin (elastic#52359)
  ...
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Jan 8, 2020
…lastic#44220)

* Possibility to filter when testing scripted fields

* Possibility to filter when testing scripted fields

* Now the i18n test should pass

* use SearchBar instead of EuiFieldText input

* clean up

* test script design improvement

* Fixed SearchBar reference and updated the help_flyout tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Nathan Reese <reese.nathan@gmail.com>
Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:enhancement review Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.6.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants