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

Forbid specifying the same percentile multiple times #57444

Closed
timroes opened this issue Feb 12, 2020 · 1 comment · Fixed by #58299
Closed

Forbid specifying the same percentile multiple times #57444

timroes opened this issue Feb 12, 2020 · 1 comment · Fixed by #58299
Assignees
Labels
Feature:Vis Editor Visualization editor issues good first issue low hanging fruit Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@timroes
Copy link
Contributor

timroes commented Feb 12, 2020

With elastic/elasticsearch#52257 Elasticsearch removes the ability to specify the same percentile value multiple times in a percentile aggregation. This will go into 8.0 since it's a breaking change. We should make sure we're also not allowing the user to specify two times the same percentile value in the visualize editor, since otherwise that request would start failing and the user doesn't have a clear error anymore what's happening. Instead the editor should show a clear error message on that, and make sure we can't save that state.

@timroes timroes added good first issue low hanging fruit Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Vis Editor Visualization editor issues labels Feb 12, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@ThomThomson ThomThomson linked a pull request Feb 24, 2020 that will close this issue
6 tasks
ThomThomson added a commit that referenced this issue Mar 2, 2020
Added an optional validation step to the number_list component to disallow duplicates, Reworked and consolidated number_list component validations into one method and enabled this option in the percentiles editor. Added unit / integration tests
gmmorris added a commit to gmmorris/kibana that referenced this issue Mar 3, 2020
* master: (26 commits)
  [Endpoint] Alert Details Overview (elastic#58412)
  Service map language icons (elastic#58633)
  [SIEM] [Case] Comments to case view (elastic#58315)
  Remove appBasePath from docs + add mock for AppMountParameters (elastic#58775)
  [kbn/optimizer] fix ui/* url rewrites in dist (elastic#58627)
  Dashboard a11y tests (elastic#58122)
  Downgrade "setting up plugin" log to debug (elastic#58776)
  [CI] Pipeline refactoring (elastic#56447)
  [Advanced Settings] Fix a11y of unsaved indicator (elastic#58511)
  put params into short url instead of behind it (elastic#58846)
  show timepicker in timelion and tsvb (elastic#58857)
  improve graph missing workspace error message (elastic#58876)
  [Maps] direct Discover "visualize" to open Maps application (elastic#58549)
  Disallow duplicate percentiles (elastic#57444) (elastic#58299)
  removing references to visTypes uiExports (elastic#58337)
  [SIEM] Default the Timeline events filter to show All events (elastic#58953)
  [Remote clusters] Add indexManagement as required plugin (elastic#58915)
  [DOCS] Rework of main get started page (elastic#58260)
  [Endpoint] [Tests] fixes elastic#57946 flaky endpoint policy list test (elastic#58348)
  [Endpoint] add resolver middleware (elastic#58288)
  ...
ThomThomson added a commit that referenced this issue Mar 5, 2020
Added an optional validation step to the number_list component to disallow duplicates, Reworked and consolidated number_list component validations into one method and enabled this option in the percentiles editor. Added unit / integration tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Vis Editor Visualization editor issues good first issue low hanging fruit Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants