-
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
[Lens] Editor state 2 #36513
[Lens] Editor state 2 #36513
Changes from 57 commits
6d6f304
44894df
943fca8
731e264
cc1a0db
18f9d83
60cd891
0eb1f31
0f652c3
a21b0ea
34e9328
b124f88
36de942
7445372
0395e9a
8147aaa
4baf6b1
d33576e
033a632
345e20f
48203dd
dece47f
7469315
ee6fdbc
396cf1c
5a21a43
c383c3e
6f8c295
b61008c
cae9fe0
3f48c98
7cdea68
d63d2f9
52cad4f
d776b54
3221830
f05024b
1c24cb5
bc0c85a
fe1356e
a562914
3d055ee
7c3a2c8
849f3a1
ff5092b
7540916
7b8c21a
0c721a7
21c8756
b31032b
fc47428
eb8f2e8
e0be753
e464ce2
7f1d66d
0bfb2d3
0e6fd91
1b1c9b7
8652dd2
082a2d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,43 +9,25 @@ import { mount, ReactWrapper } from 'enzyme'; | |
import { EditorFrame } from './editor_frame'; | ||
import { Visualization, Datasource, DatasourcePublicAPI } from '../../types'; | ||
import { act } from 'react-dom/test-utils'; | ||
import { createMockVisualization, createMockDatasource } from '../mock_extensions'; | ||
|
||
// calling this function will wait for all pending Promises from mock | ||
// datasources to be processed by its callers. | ||
const waitForPromises = () => new Promise(resolve => setImmediate(resolve)); | ||
const waitForPromises = () => new Promise(resolve => setTimeout(resolve)); | ||
|
||
describe('editor_frame', () => { | ||
const getMockVisualization = () => ({ | ||
getMappingOfTableToRoles: jest.fn(), | ||
getPersistableState: jest.fn(), | ||
getSuggestions: jest.fn(), | ||
initialize: jest.fn(), | ||
renderConfigPanel: jest.fn(), | ||
toExpression: jest.fn(), | ||
}); | ||
|
||
const getMockDatasource = () => ({ | ||
getDatasourceSuggestionsForField: jest.fn(), | ||
getDatasourceSuggestionsFromCurrentState: jest.fn(), | ||
getPersistableState: jest.fn(), | ||
getPublicAPI: jest.fn(), | ||
initialize: jest.fn(() => Promise.resolve()), | ||
renderDataPanel: jest.fn(), | ||
toExpression: jest.fn(), | ||
}); | ||
|
||
let mockVisualization: Visualization; | ||
let mockDatasource: Datasource; | ||
|
||
let mockVisualization2: Visualization; | ||
let mockDatasource2: Datasource; | ||
|
||
beforeEach(() => { | ||
mockVisualization = getMockVisualization(); | ||
mockVisualization2 = getMockVisualization(); | ||
mockVisualization = createMockVisualization(); | ||
mockVisualization2 = createMockVisualization(); | ||
|
||
mockDatasource = getMockDatasource(); | ||
mockDatasource2 = getMockDatasource(); | ||
mockDatasource = createMockDatasource(); | ||
mockDatasource2 = createMockDatasource(); | ||
}); | ||
|
||
describe('initialization', () => { | ||
|
@@ -439,4 +421,284 @@ describe('editor_frame', () => { | |
); | ||
}); | ||
}); | ||
|
||
describe('suggestions', () => { | ||
it('should fetch suggestions of currently active datasource', async () => { | ||
mount( | ||
<EditorFrame | ||
visualizationMap={{ | ||
testVis: mockVisualization, | ||
}} | ||
datasourceMap={{ | ||
testDatasource: mockDatasource, | ||
testDatasource2: mockDatasource2, | ||
}} | ||
initialDatasourceId="testDatasource" | ||
initialVisualizationId="testVis" | ||
/> | ||
); | ||
|
||
await waitForPromises(); | ||
|
||
expect(mockDatasource.getDatasourceSuggestionsFromCurrentState).toHaveBeenCalled(); | ||
expect(mockDatasource2.getDatasourceSuggestionsFromCurrentState).not.toHaveBeenCalled(); | ||
}); | ||
|
||
it('should fetch suggestions of all visualizations', async () => { | ||
mount( | ||
<EditorFrame | ||
visualizationMap={{ | ||
testVis: mockVisualization, | ||
testVis2: mockVisualization2, | ||
}} | ||
datasourceMap={{ | ||
testDatasource: mockDatasource, | ||
testDatasource2: mockDatasource2, | ||
}} | ||
initialDatasourceId="testDatasource" | ||
initialVisualizationId="testVis" | ||
/> | ||
); | ||
|
||
await waitForPromises(); | ||
|
||
expect(mockVisualization.getSuggestions).toHaveBeenCalled(); | ||
expect(mockVisualization2.getSuggestions).toHaveBeenCalled(); | ||
}); | ||
|
||
it('should display suggestions in descending order', async () => { | ||
const instance = mount( | ||
<EditorFrame | ||
visualizationMap={{ | ||
testVis: { | ||
...mockVisualization, | ||
getSuggestions: () => [ | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.5, | ||
state: {}, | ||
title: 'Suggestion2', | ||
}, | ||
{ | ||
datasourceSuggestionId: 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're reusing suggestion IDs here, I would expect a uniqueness error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the implementation of getSuggestions of the visualization, not the datasource. These ids are referencing the datatable used for this visualization suggestion. A single visualization can provide multiple suggestions based on a single datatable, this is not an error. Because this concept is pretty misleading, I changed the naming (see your other comment down below) |
||
score: 0.8, | ||
state: {}, | ||
title: 'Suggestion1', | ||
}, | ||
], | ||
}, | ||
testVis2: { | ||
...mockVisualization, | ||
getSuggestions: () => [ | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.4, | ||
state: {}, | ||
title: 'Suggestion4', | ||
}, | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.45, | ||
state: {}, | ||
title: 'Suggestion3', | ||
}, | ||
], | ||
}, | ||
}} | ||
datasourceMap={{ | ||
testDatasource: { | ||
...mockDatasource, | ||
getDatasourceSuggestionsFromCurrentState: () => [{ state: {}, tableColumns: [] }], | ||
}, | ||
}} | ||
initialDatasourceId="testDatasource" | ||
initialVisualizationId="testVis" | ||
/> | ||
); | ||
|
||
await waitForPromises(); | ||
|
||
// TODO why is this necessary? | ||
instance.update(); | ||
const suggestions = instance.find('[data-test-subj="suggestion"]'); | ||
expect(suggestions.map(el => el.text())).toEqual([ | ||
'Suggestion1', | ||
'Suggestion2', | ||
'Suggestion3', | ||
'Suggestion4', | ||
]); | ||
}); | ||
|
||
it('should switch to suggested visualization', async () => { | ||
const newDatasourceState = {}; | ||
const suggestionVisState = {}; | ||
const instance = mount( | ||
<EditorFrame | ||
visualizationMap={{ | ||
testVis: { | ||
...mockVisualization, | ||
getSuggestions: () => [ | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.8, | ||
state: suggestionVisState, | ||
title: 'Suggestion1', | ||
}, | ||
], | ||
}, | ||
testVis2: mockVisualization2, | ||
}} | ||
datasourceMap={{ | ||
testDatasource: { | ||
...mockDatasource, | ||
getDatasourceSuggestionsFromCurrentState: () => [ | ||
{ state: newDatasourceState, tableColumns: [] }, | ||
], | ||
}, | ||
}} | ||
initialDatasourceId="testDatasource" | ||
initialVisualizationId="testVis2" | ||
/> | ||
); | ||
|
||
await waitForPromises(); | ||
|
||
// TODO why is this necessary? | ||
instance.update(); | ||
|
||
act(() => { | ||
instance.find('[data-test-subj="suggestion"]').simulate('click'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would expect a suggestion click to happen inside a modal, is that what this is doing? If not in a modal, what's this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the bottom right suggestions which are always displayed. The modal makes sense when the user drops a field in the workspace. I didn't add the modal yet, atm it is always picking the first suggestion (see https://github.com/elastic/kibana/pull/36513/files#diff-3bc17690d1f0177c324498f168ff529dR63 ) |
||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this need an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It should, but it currently doesn't - this is probably fixed when we upgrade enzyme and enzyme react adapter (should also get rid of the "you did something outside of act" error which get logged to console while running the tests currently) |
||
|
||
expect(mockVisualization.renderConfigPanel).toHaveBeenCalledTimes(1); | ||
expect(mockVisualization.renderConfigPanel).toHaveBeenCalledWith( | ||
expect.any(Element), | ||
expect.objectContaining({ | ||
state: suggestionVisState, | ||
}) | ||
); | ||
expect(mockDatasource.renderDataPanel).toHaveBeenLastCalledWith( | ||
expect.any(Element), | ||
expect.objectContaining({ | ||
state: newDatasourceState, | ||
}) | ||
); | ||
}); | ||
|
||
it('should switch to best suggested visualization on field drop', async () => { | ||
const suggestionVisState = {}; | ||
const instance = mount( | ||
<EditorFrame | ||
visualizationMap={{ | ||
testVis: { | ||
...mockVisualization, | ||
getSuggestions: () => [ | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.2, | ||
state: {}, | ||
title: 'Suggestion1', | ||
}, | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.8, | ||
state: suggestionVisState, | ||
title: 'Suggestion2', | ||
}, | ||
], | ||
}, | ||
testVis2: mockVisualization2, | ||
}} | ||
datasourceMap={{ | ||
testDatasource: { | ||
...mockDatasource, | ||
getDatasourceSuggestionsForField: () => [{ state: {}, tableColumns: [] }], | ||
getDatasourceSuggestionsFromCurrentState: () => [{ state: {}, tableColumns: [] }], | ||
}, | ||
}} | ||
initialDatasourceId="testDatasource" | ||
initialVisualizationId="testVis" | ||
/> | ||
); | ||
|
||
await waitForPromises(); | ||
|
||
// TODO why is this necessary? | ||
instance.update(); | ||
|
||
act(() => { | ||
instance.find('[data-test-subj="lnsDragDrop"]').simulate('drop'); | ||
}); | ||
|
||
expect(mockVisualization.renderConfigPanel).toHaveBeenCalledWith( | ||
expect.any(Element), | ||
expect.objectContaining({ | ||
state: suggestionVisState, | ||
}) | ||
); | ||
}); | ||
|
||
it('should switch to best suggested visualization regardless extension on field drop', async () => { | ||
const suggestionVisState = {}; | ||
const instance = mount( | ||
<EditorFrame | ||
visualizationMap={{ | ||
testVis: { | ||
...mockVisualization, | ||
getSuggestions: () => [ | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.2, | ||
state: {}, | ||
title: 'Suggestion1', | ||
}, | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.6, | ||
state: {}, | ||
title: 'Suggestion2', | ||
}, | ||
], | ||
}, | ||
testVis2: { | ||
...mockVisualization2, | ||
getSuggestions: () => [ | ||
{ | ||
datasourceSuggestionId: 0, | ||
score: 0.8, | ||
state: suggestionVisState, | ||
title: 'Suggestion3', | ||
}, | ||
], | ||
}, | ||
}} | ||
datasourceMap={{ | ||
testDatasource: { | ||
...mockDatasource, | ||
getDatasourceSuggestionsForField: () => [{ state: {}, tableColumns: [] }], | ||
getDatasourceSuggestionsFromCurrentState: () => [{ state: {}, tableColumns: [] }], | ||
}, | ||
}} | ||
initialDatasourceId="testDatasource" | ||
initialVisualizationId="testVis" | ||
/> | ||
); | ||
|
||
await waitForPromises(); | ||
|
||
// TODO why is this necessary? | ||
instance.update(); | ||
|
||
act(() => { | ||
instance.find('[data-test-subj="lnsDragDrop"]').simulate('drop'); | ||
}); | ||
|
||
expect(mockVisualization2.renderConfigPanel).toHaveBeenCalledWith( | ||
expect.any(Element), | ||
expect.objectContaining({ | ||
state: suggestionVisState, | ||
}) | ||
); | ||
}); | ||
}); | ||
}); |
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.
This says that it's fetching suggestions on first render, without user interaction. Why would we want that?
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.
This test loads the editor with an initial datasource and an initial visualization. This means the datasource will be able to provide suggestions for datatables and all visualizations can particiapte in creating suggestions. This is done to fill the bottom right suggestions: