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

[Fix for Vis Editor] Revert setting time field to empty string when it's undefined #58873

Merged
merged 7 commits into from
Mar 5, 2020

Conversation

maryia-lapata
Copy link
Contributor

Fixes #58862.

The issue is if a date histogram aggregation is configured in the vis based on an index pattern without a time filter, errors are logged into the console and the page is empty.

image

Steps to reproduce:

  1. Create an index pattern without a time filter
  2. Create new visualization
  3. Select date_histogram aggregation as X-Axis buckets
  4. See logged errors and empty page.

The reason is that when an index pattern is created with I don't want to use the Time Filter time filter option, the selectedTimeField is set as an empty string instead of undefined due to recent changes in #56987. And in agg_config.ts
https://github.com/mattkime/kibana/blob/0adfdcafe137692750118277927f9a3ccbf26faa/src/legacy/core_plugins/data/public/search/aggs/agg_config.ts#L190-L191
val is "" and val == nul is false, which leads to calling deserialize function when it shouldn't.

@maryia-lapata maryia-lapata added Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Feb 28, 2020
@elasticmachine
Copy link
Contributor

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

@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

@maryia-lapata maryia-lapata requested review from mattkime and flash1293 and removed request for mattkime March 2, 2020 07:23
@maryia-lapata maryia-lapata marked this pull request as ready for review March 2, 2020 07:23
@maryia-lapata maryia-lapata requested a review from a team as a code owner March 2, 2020 07:23
@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

@maryia-lapata
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

Bug is resolved, but can we add a test to cover this case?

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

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

LGTM once green

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@maryia-lapata maryia-lapata merged commit 61a8b78 into elastic:master Mar 5, 2020
@maryia-lapata maryia-lapata deleted the fix-date-histogram branch March 5, 2020 11:15
maryia-lapata added a commit that referenced this pull request Mar 5, 2020
…t's undefined (#58873) (#59407)

* Revert setting time field to empty string when it's undefined

* Add unit test

* Mock timeFields

* Update step_time_field.test.tsx

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Mar 5, 2020
…re/files-and-filetree

* 'master' of github.com:elastic/kibana: (254 commits)
  Convert discover_page to ts, remove redundunt methods (elastic#59312)
  [Fix for Vis Editor] Revert setting time field to empty string when it's undefined (elastic#58873)
  Delete legacy search endpoint (elastic#59341)
  [Uptime] Improve duration chart (elastic#58404)
  [Snapshot & Restore] NP migration (elastic#59109)
  [ML] Add support for date_nanos time field in anomaly job wizard (elastic#59017)
  Revert "Makes alerting and actions optional properties for interface RequestH… (elastic#59264)"
  Change remote_clusters ID to remoteClusters (elastic#59246)
  Makes alerting and actions optional properties for interface RequestH… (elastic#59264)
  Clean up date histogram agg type. (elastic#58805)
  [ML] Management: fix license unsubscribe (elastic#59365)
  Remove documentation for server.cors settings (elastic#59096)
  Edit alert flyout (elastic#58964)
  [SIEM] Fix rule delete/duplicate actions (elastic#59306)
  move mouse to close obstructing tooltip (elastic#59214)
  Reset page after deleting (elastic#59310)
  Make sure phrases input filter triggers autosuggestons (elastic#59299)
  Add loading count source for http requests (elastic#59245)
  Revert "[ML] Transforms: Deprecate custom KibanaContext. (elastic#59133)"
  Expose metrics service to public API (elastic#59294)
  ...

# Conflicts:
#	src/plugins/console/public/application/containers/editor/legacy/console_editor/editor.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console errors on selecting date histogram when index pattern is created without time filter
4 participants