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

fixed editor state updates for parent pipeline aggs #22874

Merged
merged 3 commits into from
Sep 12, 2018

Conversation

ppisljar
Copy link
Member

resolves #22833

release notes: editor now correctly shows errors for parent pipeline aggregations

qa: create a vertical bar chart, choose derivative as a metric and add date histogram bucket agg.
after this add another bucket agg, select split rows, terms, any field ... before you wouldn't get an error, you could apply the change and get elasticsearch error. after this nothing worked.

@ppisljar ppisljar added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:fix v7.0.0 v6.5.0 v6.4.1 labels Sep 10, 2018
Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

Pulled PR and tested locally. I can resolve the visualization configuration error now.
Two small comments on the added functional tests.
Approve after changes/comments and CI pass.

test/functional/apps/visualize/_vertical_bar_chart.js Outdated Show resolved Hide resolved

const error = await find.byCssSelector('.vis-editor-agg-error');
const errorMessage = await error.getProperty('innerText');
log.debug(errorMessage);
Copy link
Member

Choose a reason for hiding this comment

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

Could you add this in as a function in the visualize_page

Copy link
Member Author

Choose a reason for hiding this comment

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

i am not a huge fan of putting functions that are never reused in there, but i can move it.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ppisljar ppisljar requested a review from lukeelmers September 10, 2018 11:33
@elasticmachine
Copy link
Contributor

💔 Build Failed

@ppisljar
Copy link
Member Author

UI Functional Tests.test/functional/apps/management/_import_objects·js.management import objects should import saved objects with index patterns when index patterns does not exists

jenkins, test this

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@marius-dr marius-dr left a comment

Choose a reason for hiding this comment

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

LGTM on IE and Firefox

@ppisljar ppisljar force-pushed the fix/pipelineAggEditorState branch from 3d18e83 to fa6b6d9 Compare September 12, 2018 10:26
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@ppisljar ppisljar restored the fix/pipelineAggEditorState branch September 26, 2018 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:fix review v6.4.1 v6.5.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Derivative aggregation gets stuck in error state
4 participants