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

Convert value provided to PhraseValueInput to string to prevent Exception with numeric values #45259

Merged

Conversation

kertal
Copy link
Member

@kertal kertal commented Sep 10, 2019

Summary

Even when a field's format in an Indexpattern is string, a field can contain a number value in _source. Because in _source the value is persisted like it was sent to Elasticsearch. When you add a filter by clicking e.g. in the DocViewer, the value of the field in _source is used for the filter and this causes an Exception when trying to edit the filter (A string value is expected).

image

Test

Add mapping and data for a new index in Kibana's Console:

PUT test-number
{
  "mappings": {
   
      "properties": {
        "number": {
          "type": "keyword"
        }
      }
    
  }
}

PUT test-number/_doc/1
{
  "number": 123
}

Add an IndexPattern for the test-number index. Switch to Discover, use the new IndexPattern

Add a filter by the number property

Bildschirmfoto 2019-09-10 um 19 03 33

Edit the filter.
Bildschirmfoto 2019-09-10 um 19 06 42

It works, without this PR, you'd get an Exception

Fixes #30245
Fixes #45010

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials
- [ ] Unit or functional tests were updated or added to match the most common scenarios
- [ ] This was checked for keyboard-only and screenreader accessibility

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately

@kertal kertal self-assigned this Sep 10, 2019
@kertal kertal marked this pull request as ready for review September 10, 2019 17:17
@kertal kertal added release_note:fix v8.0.0 v7.5.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 10, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@kertal kertal changed the title Convert value provided to PhraseValueInput to string to catch Exception Convert value provided to PhraseValueInput to string to prevent Exception with numeric values Sep 10, 2019
Copy link
Contributor

@Bargs Bargs left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Tested in Chrome, works as expected.
Code LGTM.

@kertal
Copy link
Member Author

kertal commented Sep 11, 2019

thx @TinaHeiligers & @Bargs, here's my personal award to honor your performance in the not yet olympic discipline of synchronous reviewing 🏆

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal kertal merged commit 745d76d into elastic:master Sep 11, 2019
kertal added a commit to kertal/kibana that referenced this pull request Sep 11, 2019
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 11, 2019
…-to-np-ready

* 'master' of github.com:elastic/kibana: (25 commits)
  [ML] Fixes display of matching modules in index data visualizer (elastic#45261)
  [Console] Update indentation behaviour (elastic#45249)
  Convert value provided to PhraseValueInput to string to catch Exception (elastic#45259)
  [Region Map] Fix loading default vector map and base layer setting (elastic#43858)
  [ML] Fixing empty time range when cloning jobs (elastic#45286)
  [ML] Fixing wizard validation delay (elastic#45265)
  [Logs UI] Interpret finished analysis jobs as healthy (elastic#45268)
  [Console] SQL template with triple quote in completion (elastic#45248)
  [ML] Data Frames: Cards as links (elastic#45254)
  fix(code/frontend): should show updating instead of cloning when updating (elastic#45238)
  fix(code/frontend): fix document search result from (elastic#45236)
  disable another flaky suite (elastic#45323) (elastic#45330)
  disable flaky suite (elastic#45105)
  skip flaky suite (elastic#43069)
  skip flaky suite (elastic#45089)
  disable jest suite that has no enabled tests (elastic#44250)
  disable flaky test (elastic#45317)
  disable flaky test (elastic#45315)
  [DOCS] Creates developer folder (elastic#45280)
  [SIEM] Changes ML conditional links to use tabs, fixes a small bug with null filterQuery   (elastic#45218)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 11, 2019
…ditor

* 'master' of github.com:elastic/kibana: (76 commits)
  Upgrade EUI to 13.8.1 (elastic#45052)
  [ML] Add multi metric job wizard test (elastic#45279)
  [SIEM] Inject/apply KQL changed in refresh button (elastic#45065)
  [Graph] Type persistence (elastic#44985)
  Functional tests: convert more test/services to TS (elastic#45176)
  [ML] Fixes display of matching modules in index data visualizer (elastic#45261)
  [Console] Update indentation behaviour (elastic#45249)
  Convert value provided to PhraseValueInput to string to catch Exception (elastic#45259)
  [Region Map] Fix loading default vector map and base layer setting (elastic#43858)
  [ML] Fixing empty time range when cloning jobs (elastic#45286)
  [ML] Fixing wizard validation delay (elastic#45265)
  [Logs UI] Interpret finished analysis jobs as healthy (elastic#45268)
  [Console] SQL template with triple quote in completion (elastic#45248)
  [ML] Data Frames: Cards as links (elastic#45254)
  fix(code/frontend): should show updating instead of cloning when updating (elastic#45238)
  fix(code/frontend): fix document search result from (elastic#45236)
  disable another flaky suite (elastic#45323) (elastic#45330)
  disable flaky suite (elastic#45105)
  skip flaky suite (elastic#43069)
  skip flaky suite (elastic#45089)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Filters release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.5.0 v8.0.0
Projects
None yet
4 participants