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

[TSVB] Technical debt - remove default_index_pattern and default_timefield from model #95925

Merged
merged 6 commits into from
Apr 7, 2021

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Mar 31, 2021

Summary

After saving TSVB rendering, two extra properties appear in the model: default_index_pattern, default_timefield
This is due to the fact that we get the default index in the parent component and then pass it through the model. It's wrong and this PR fix that.

image

@alexwizp
Copy link
Contributor Author

alexwizp commented Apr 1, 2021

@elasticmachine merge upstream

@alexwizp
Copy link
Contributor Author

alexwizp commented Apr 5, 2021

@elasticmachine merge upstream

Copy link
Contributor

@VladLasitsa VladLasitsa left a comment

Choose a reason for hiding this comment

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

LGTM

@alexwizp alexwizp self-assigned this Apr 5, 2021
@alexwizp alexwizp added v7.13.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:TSVB TSVB (Time Series Visual Builder) labels Apr 5, 2021
@alexwizp alexwizp requested a review from stratoula April 5, 2021 11:57
@alexwizp alexwizp marked this pull request as ready for review April 5, 2021 11:57
@alexwizp alexwizp requested a review from a team April 5, 2021 11:57
@elasticmachine
Copy link
Contributor

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

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

@elasticmachine merge upstream

const visFields = await fetchFields(indexPatterns);

this.setState((state) => ({
defaultIndex: index,
model: {
...state.model,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that now can be simplifed to:

      this.setState({
        defaultIndex: index,
        visFields,
      });

You don't change something to model, so no need to update it, if I understand the code correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! thank you

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

LGTM, tested it locally and it seems that everything works fine

@stratoula
Copy link
Contributor

@elasticmachine run elasticsearch-ci/docs

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visTypeTimeseries 502 503 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
visTypeTimeseries 1.6MB 1.6MB -178.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
visTypeTimeseries 109.3KB 109.3KB +31.0B

History

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

cc @alexwizp

@alexwizp alexwizp merged commit 80e3d21 into elastic:master Apr 7, 2021
alexwizp added a commit to alexwizp/kibana that referenced this pull request Apr 7, 2021
…field from model (elastic#95925)

* [TSVB] Technical debt - remove default_index_pattern and default_timefield from model

* Update vis_editor.tsx

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
alexwizp added a commit that referenced this pull request Apr 7, 2021
…field from model (#95925) (#96385)

* [TSVB] Technical debt - remove default_index_pattern and default_timefield from model

* Update vis_editor.tsx

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:TSVB TSVB (Time Series Visual Builder) release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants