[Lens] Extend Datasource props validation with VisualizationGroups #82607
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lay-error-workspace
…lay-error-workspace
…it as building state
…/datasource-groups-validation
…/datasource-groups-validation
…source-groups-validation
flash1293
reviewed
Nov 5, 2020
x-pack/plugins/lens/public/types.ts
Outdated
| getErrorMessages: (state: T) => Array<{ shortMessage: string; longMessage: string }> | undefined; | ||
| getErrorMessages: ( | ||
| state: T, | ||
| groups?: VisualizationDimensionGroupConfig[] |
There was a problem hiding this comment.
You added the groups as an optional parameter here, but in the places getErrorMessages is actually called they are not passed in (in workspace and suggestion panel)
flash1293
added
Feature:Lens
Team:Visualizations
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
flash1293
added
the
release_note:skip
1 task
flash1293
requested changes
Nov 6, 2020
| @@ -104,8 +104,18 @@ export const validateDatasourceAndVisualization = ( | |||
| longMessage: string; | |||
| }> | |||
| | undefined => { | |||
| const layersGroups = | |||
| currentVisualizationState && | |||
| currentVisualization?.getLayerIds(currentVisualizationState).map((layerId) => { | |||
There was a problem hiding this comment.
This has to be a map (with the keys being the layer id) - otherwise the datasource won't be able to map the right group arrays back to the right internal layer representation.
The type should be
getErrorMessages: (
state: T,
layersGroups?: Record<string, VisualizationDimensionGroupConfig[]>
) => Array<{ shortMessage: string; longMessage: string }> | undefined;
There was a problem hiding this comment.
It is an array, so always in order, so it shouldn't be a problem: probably the implementation will need to identify layers index anyway internally to output a better contextual message.
I understand it may be also useful to have a lookup by layerId, I'll update.
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]async chunks size
History
To update your PR or re-run it, just comment with: |
flash1293
approved these changes
Nov 9, 2020
dej611
added a commit
to dej611/kibana
that referenced
this pull request
Nov 9, 2020
…lastic#82607) * ✨ First pass with visualization validation + error messages * 🔥 Remove indexpattern error handling for now * 🏷️ Fix type issues * ✅ Add getErrorMessage test for data table * ✅ Add tests for pie and metric error messages * 🌐 Fix i18n checks issues * 🐛 Fix last issue * ✅ Add more tests for the XY visualization validation code * 👌 Included all feedback from first review * ✏️ Off by one message * 🌐 Fix i18n duplicate id * 🌐 Fix last i18n issue * 🐛 Fixed a hook reflow issue * ♻️+✅ Reworked validation flow + tests * 🏷️ Fix type issue * 🐛 Improved XY corner cases validation logic * 🐛 Fix empty datatable scenario * ✨ + ✅ Improved error messages for invalid datasources + tests * 🌐 Add missing i18n translation * 🏷️ Fix type issues * 🌐 Fix i18n issues * ✨ Filter out suggestions which fail to build * 🚚 Migrate datatable validation logic to the building phase, handling it as building state * 🏷️ Fix type issue * ✏️ Add comment for future enhancements * ✏️ Updated comment * :world_with_meridians: Refactor axis labels * 🌐 Reworked few validation messages * 🐛 Fix break down validation + percentage charts * ✅ Align tests with new validation logic * ♻️ Fix suggestion panel validation to match main panel * 🌐 Fix i18n issues * 🔧 Fix some refs for validation checks in suggestions * 🐛 Fix missing key prop in multiple errors scenario * 🐛 Fix swtich issue from XY to partition * 🌐 Fix i18n messages and aligned tests * 🐛 Fix suggestions switching bug * :refactor: Add more validation + refactored validation logic in a single place * ✏️ Add note about lint hooks disable rule * 🚨 Fix linting issue * 🏗️ Add infra API for datasource advanced validation * ✅ Align tests with new API * ✅ Fix type issues in tests * 👌 Early exists added * ✨ Add layers groups to the API * ✅ Fix some broken test after the validation change * 👌 Move to disctionary shape Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
dej611
added a commit
that referenced
this pull request
Nov 9, 2020
…ups (#82607) (#82958) * ✨ First pass with visualization validation + error messages * 🔥 Remove indexpattern error handling for now * 🏷️ Fix type issues * ✅ Add getErrorMessage test for data table * ✅ Add tests for pie and metric error messages * 🌐 Fix i18n checks issues * 🐛 Fix last issue * ✅ Add more tests for the XY visualization validation code * 👌 Included all feedback from first review * ✏️ Off by one message * 🌐 Fix i18n duplicate id * 🌐 Fix last i18n issue * 🐛 Fixed a hook reflow issue * ♻️+✅ Reworked validation flow + tests * 🏷️ Fix type issue * 🐛 Improved XY corner cases validation logic * 🐛 Fix empty datatable scenario * ✨ + ✅ Improved error messages for invalid datasources + tests * 🌐 Add missing i18n translation * 🏷️ Fix type issues * 🌐 Fix i18n issues * ✨ Filter out suggestions which fail to build * 🚚 Migrate datatable validation logic to the building phase, handling it as building state * 🏷️ Fix type issue * ✏️ Add comment for future enhancements * ✏️ Updated comment * :world_with_meridians: Refactor axis labels * 🌐 Reworked few validation messages * 🐛 Fix break down validation + percentage charts * ✅ Align tests with new validation logic * ♻️ Fix suggestion panel validation to match main panel * 🌐 Fix i18n issues * 🔧 Fix some refs for validation checks in suggestions * 🐛 Fix missing key prop in multiple errors scenario * 🐛 Fix swtich issue from XY to partition * 🌐 Fix i18n messages and aligned tests * 🐛 Fix suggestions switching bug * :refactor: Add more validation + refactored validation logic in a single place * ✏️ Add note about lint hooks disable rule * 🚨 Fix linting issue * 🏗️ Add infra API for datasource advanced validation * ✅ Align tests with new API * ✅ Fix type issues in tests * 👌 Early exists added * ✨ Add layers groups to the API * ✅ Fix some broken test after the validation change * 👌 Move to disctionary shape 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR is the second part of #80127, built on top of #81439, so it requires it to be merged before merging this one.
The purpose of this PR is to create a basic infrastructure for future advanced checks on the Dimension Editor in Lens.