Skip to content

[Vislib] Use timestamp on brush event instead of iso dates#91483

Merged
stratoula merged 4 commits intoelastic:masterfrom
stratoula:vislib-brush-timestamp
Feb 17, 2021
Merged

[Vislib] Use timestamp on brush event instead of iso dates#91483
stratoula merged 4 commits intoelastic:masterfrom
stratoula:vislib-brush-timestamp

Conversation

@stratoula
Copy link
Copy Markdown
Contributor

@stratoula stratoula commented Feb 16, 2021

Summary

Closes #90923.
Url drilldowns on brush event for all visualizations emit timestamps instead of the vislib xy axis that emit ISO strings. We want to change this behavior and emit timestamps instead in order to be in line with the rest.

Url drilldowns are in beta and introduced on 7.10 so we are not going to proceed with migrating the existing URL templates.

How to test it

Create a URL drilldown (range) and add the following URL template
{{kibanaUrl}}/app/discover#/?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:'{{date event.from}}',to:'{{date event.to}}'))&_a=(columns:!(_source),filters:{{rison context.panel.filters}},index:'{{context.panel.indexPatternId}}',interval:auto,query:(language:{{context.panel.query.language}},query:'{{context.panel.query.query}}'),sort:!())

It should work correctly now with the date function

@stratoula stratoula requested review from Dosant and ppisljar February 16, 2021 15:30
@stratoula stratoula marked this pull request as ready for review February 16, 2021 15:53
@stratoula stratoula requested a review from a team February 16, 2021 15:53
@stratoula stratoula added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Feb 16, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Looks good.

Could you please also tweak the docs 🙏
I think it makes sense to tweak as part of this:

https://github.com/elastic/kibana/blob/master/docs/user/dashboard/url-drilldown.asciidoc

from and to values of selected range. Depending on your data, could be either a date or number.

---> to probably something like:

from and to values of the selected range as numbers

There is also a functional test that should have caught this "regression", could you please tweak an assertion so we would catch it? Probably we need also to check that time range in discover is not now-15min

https://github.com/elastic/kibana/blob/master/x-pack/test/functional/apps/dashboard/drilldowns/dashboard_to_url_drilldown.ts#L76

@stratoula
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

Copy link
Copy Markdown
Contributor

@ppisljar ppisljar 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

Copy link
Copy Markdown
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Thanks for the docs and test changes 🚀

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 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
visTypeVislib 622.9KB 623.1KB +186.0B

History

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

@mbondyra
Copy link
Copy Markdown
Contributor

Code LGTM, tested on Chrome, all works wonderfully with the example drilldown:

{{kibanaUrl}}/app/discover#/?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:'{{date event.from}}',to:'{{date event.to}}'))&_a=(columns:!(_source),filters:{{rison context.panel.filters}},index:'{{context.panel.indexPatternId}}',interval:auto,query:(language:{{context.panel.query.language}},query:'{{context.panel.query.query}}'),sort:!())

@stratoula stratoula merged commit c9f7c2e into elastic:master Feb 17, 2021
stratoula added a commit to stratoula/kibana that referenced this pull request Feb 17, 2021
…1483)

* [Vislib] Use timestamp on brush event instead of iso dates

* Fix functional test and update documentation

* Update documentation

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
stratoula added a commit to stratoula/kibana that referenced this pull request Feb 17, 2021
…1483)

* [Vislib] Use timestamp on brush event instead of iso dates

* Fix functional test and update documentation

* Update documentation

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 17, 2021
* master: (157 commits)
  [DOCS] Adds machine learning to the security section of alerting (elastic#91501)
  [Uptime] Ping list step screenshot caption formatting (elastic#91403)
  [Vislib] Use timestamp on brush event instead of iso dates (elastic#91483)
  [Application Usage] Remove deprecated & unused legacy.appChanged API (elastic#91464)
  Migrate logstash, monitoring, url_drilldowns, xpack_legacy to ts projects (elastic#91194)
  [APM] Wrap Elasticsearch client errors (elastic#91125)
  [APM] Fix optimize-tsconfig script (elastic#91487)
  [Discover][docs] Add searchFieldsFromSource description (elastic#90980)
  Adds support for 'ip' data type (elastic#85087)
  [Detection Rules] Add updates from 7.11.2 rules (elastic#91553)
  [SECURITY SOLUTION] Eql in timeline (elastic#90816)
  [APM] Correlations Beta (elastic#86477) (elastic#89952)
  [Security Solutions][Detection Engine] Adds a warning banner when the alerts data has not been migrated yet. (elastic#90258)
  [Security Solution] [Timeline] Endpoint row renderers (2nd batch) (elastic#91446)
  skip flaky suite (elastic#91450)
  skip flaky suite (elastic#91592)
  [Security Solution][Endpoint][Admin] Endpoint Details UX Enhancements (elastic#90870)
  [ML] Add better UI support for runtime fields Transforms  (elastic#90363)
  [Security Solution] [Detections] Replace 'partial failure' with 'warning' for rule statuses (elastic#91167)
  [Security Solution][Detections] Adds Indicator path config for indicator match rules (elastic#91260)
  ...
stratoula added a commit that referenced this pull request Feb 17, 2021
…91623)

* [Vislib] Use timestamp on brush event instead of iso dates

* Fix functional test and update documentation

* Update documentation

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
stratoula added a commit that referenced this pull request Feb 17, 2021
…91622)

* [Vislib] Use timestamp on brush event instead of iso dates

* Fix functional test and update documentation

* Update documentation

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Vislib Vislib chart implementation release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.11.2 v7.12.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[URL Drilldown] Range selection over date x-axis timestamp range instead of iso range

6 participants