-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 validation: Allow numeric values for axes #63553
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Pinging @elastic/kibana-app (Team:KibanaApp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code change LGTM, although I did not test clicking the link generated by the Metrics Explorer, I looked at how metrics explorer generates links. They are generating URLs that are parsed by us, which is why the string validation seems to be failing. For example, vis:(aggs:!(),params:(axis_formatter:number,axis_min:0,
was being parsed as a number instead of string.
axis_max: stringOptionalNullable, | ||
axis_min: stringOptionalNullable, | ||
axis_max: stringOrNumberOptionalNullable, | ||
axis_min: stringOrNumberOptionalNullable, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing that we are still using a mix of both types within our own code, for example: seriesGroup.axis_min = seriesGroup.axis_min || 0;
@elasticmachine merge upstream |
…nto tsvb-validation-fix-3
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…bana into pipeline-editor-part-mvp-2 * 'feature/ingest-node-pipelines' of github.com:elastic/kibana: (152 commits) [Ingest pipelines] Simulate pipeline (elastic#64223) Ability to get scoped call cluster from alerting and action executors (elastic#64432) Add editApp and editPath to embeddable (elastic#64297) TSVB validation: Allow numeric values for axes (elastic#63553) [ML] Fixing optional plugin dependency types (elastic#64450) [Logs UI] Alerting (elastic#62806) [Endpoint] Show Policy Status on Host Details using Policy Response API (elastic#64116) [Maps] update LayerWizard previewLayer to take layerDescriptor instead of ISource (elastic#64461) Remove SO root property index signature (elastic#64434) [ML] Functional tests - stabilize job row details validations (elastic#64503) [Ingest] Add Global settings flyout (elastic#64276) Bump cypress dev-dependency from 4.2.0 to 4.4.1 (elastic#64408) Migrate saved object of type url to kibana platform (elastic#64043) [NP] Migrate ui capabilities (elastic#64185) Bump karma-mocha dev-dependency from 1.3.0 to 2.0.0 (elastic#64407) Migrate kql_telemetry saved object registration to Kibana platform (elastic#64149) Remove SO autocreateindex error and error page (elastic#64037) Fix issue with yarn.lock (elastic#64496) Bump @hapi/boom dependency from 7.4.2 to 7.4.11 (elastic#64433) Bump gonzales-pe dev-dependency from 4.2.4 to 4.3.0 (elastic#64401) ... # Conflicts: # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx # x-pack/plugins/ingest_pipelines/public/shared_imports.ts
Fixes #63081
For some parameters the TSVB UI always sets strings, but numbers should also be allowed and work just fine.
This PR relaxes the types a little in this regard.
It also resets the failed validation counter for 7.7.1