Skip to content

Commit b66561a

Browse files
committed
optimize code
Signed-off-by: Qxisylolo <qianxisy@amazon.com>
1 parent 5766222 commit b66561a

24 files changed

Lines changed: 277 additions & 289 deletions

src/plugins/explore/public/application/in_context_vis_editor/component/save_vis_button.test.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,26 @@ jest.mock('./save_vis_modal', () => ({
3232
SaveVisModal: () => <div data-test-subj="save-vis-modal">Modal</div>,
3333
}));
3434

35+
jest.mock('../../../components/data_transformations', () => {
36+
const mockObs = {
37+
subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }),
38+
getValue: jest.fn().mockReturnValue([]),
39+
};
40+
const mockMapObs = {
41+
subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }),
42+
getValue: jest.fn().mockReturnValue(new Map()),
43+
};
44+
return {
45+
useTransformationService: jest.fn().mockReturnValue({
46+
clearPipeline: jest.fn(),
47+
pipeline$: mockObs,
48+
getPipeline$: () => mockObs,
49+
stageSchemas$: mockMapObs,
50+
}),
51+
TransformPanel: () => null,
52+
};
53+
});
54+
3555
jest.mock('@osd/i18n', () => ({
3656
i18n: {
3757
translate: jest.fn((key, options) => options.defaultMessage),
@@ -67,6 +87,7 @@ const buildVisualizationBuilder = () => ({
6787
visualizationBuilderForEditor: {
6888
visConfig$: { value: { type: 'bar', styles: {}, axesMapping: { x: 'field' } } },
6989
isVisDirty$: new BehaviorSubject(false),
90+
getTransformationService: jest.fn().mockReturnValue({ pipeline$: new BehaviorSubject([]) }),
7091
},
7192
});
7293

src/plugins/explore/public/application/in_context_vis_editor/component/save_vis_button.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import { useCurrentExploreId } from '../hooks/use_explore_id';
2525
import { useVisualizationBuilder } from '../hooks/use_visualization_builder';
2626
import { EditorMode } from '../../utils/state_management/types';
2727
import { ContainerState, CONTAINER_URL_KEY } from '../types';
28-
import { useTransformationService } from '../hooks/use_transformation_service';
2928

3029
export interface OnSaveProps {
3130
savedExplore: SavedExplore;

src/plugins/explore/public/application/in_context_vis_editor/component/visualization_editor_bottom_left_container.test.tsx

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import React from 'react';
76
import { render, screen } from '@testing-library/react';
87
import { ResizableQueryPanelAndVisualization } from './visualization_editor_bottom_left_container';
98
import { QueryExecutionStatus } from '../../utils/state_management/types';
@@ -12,6 +11,26 @@ import { useVisualizationBuilder } from '../hooks/use_visualization_builder';
1211

1312
jest.mock('../hooks/use_query_builder_state', () => ({ useQueryBuilderState: jest.fn() }));
1413
jest.mock('../hooks/use_visualization_builder', () => ({ useVisualizationBuilder: jest.fn() }));
14+
15+
jest.mock('../../../components/data_transformations', () => {
16+
const mockObs = {
17+
subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }),
18+
getValue: jest.fn().mockReturnValue([]),
19+
};
20+
const mockMapObs = {
21+
subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }),
22+
getValue: jest.fn().mockReturnValue(new Map()),
23+
};
24+
return {
25+
useTransformationService: jest.fn().mockReturnValue({
26+
clearPipeline: jest.fn(),
27+
pipeline$: mockObs,
28+
getPipeline$: () => mockObs,
29+
stageSchemas$: mockMapObs,
30+
}),
31+
TransformPanel: () => null,
32+
};
33+
});
1534
jest.mock('../../../components/query_panel/utils/use_search_context', () => ({
1635
useSearchContext: jest.fn().mockReturnValue({}),
1736
}));
@@ -44,8 +63,12 @@ jest.mock('@osd/i18n', () => ({
4463
}));
4564

4665
const mockQueryEditorState$ = { getValue: jest.fn() };
66+
const mockResultState$ = { subscribe: jest.fn().mockReturnValue({ unsubscribe: jest.fn() }) };
4767
const mockQueryBuilder = {
4868
queryEditorState$: mockQueryEditorState$,
69+
resultState$: mockResultState$,
70+
updateQueryEditorState: jest.fn(),
71+
setOnDatasetChanged: jest.fn(),
4972
};
5073

5174
const buildState = (options: Record<string, any>) => ({

src/plugins/explore/public/application/in_context_vis_editor/component/visualization_editor_bottom_left_container.tsx

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import { useVisualizationBuilder } from '../hooks/use_visualization_builder';
3131

3232
import '../visualization_editor.scss';
3333
import {
34-
TransformationService,
34+
ITransformationService,
3535
useTransformationService,
3636
TransformPanel,
3737
} from '../../../components/data_transformations';
@@ -69,6 +69,12 @@ export const ResizableQueryPanelAndVisualization = () => {
6969
}, [queryBuilder]),
7070
});
7171

72+
useEffect(() => {
73+
queryBuilder.setOnDatasetChanged(() => {
74+
transformServices.clearPipeline();
75+
});
76+
}, [queryBuilder, transformServices]);
77+
7278
useEffect(() => {
7379
queryBuilder.updateQueryEditorState({ activeBottomPanelTab: activeTab });
7480
}, [activeTab, queryBuilder]);
@@ -164,10 +170,7 @@ export const ResizableQueryPanelAndVisualization = () => {
164170
{activeTab === 'QUERY_TAB' ? (
165171
<QueryPanel queryEditorState$={queryBuilder.queryEditorState$} />
166172
) : (
167-
<TransformPanel
168-
transformationService={transformServices}
169-
visualizationBuilder={visualizationBuilderForEditor}
170-
/>
173+
<TransformPanel transformationService={transformServices} />
171174
)}
172175
</EuiPanel>
173176
</EuiResizablePanel>
@@ -179,7 +182,7 @@ export const ResizableQueryPanelAndVisualization = () => {
179182
};
180183

181184
export const VisualizationContainer = React.memo(
182-
({ transformationService }: { transformationService: TransformationService }) => {
185+
({ transformationService }: { transformationService: ITransformationService }) => {
183186
const searchContext = useSearchContext();
184187

185188
const { visualizationBuilderForEditor: visualizationBuilder } = useVisualizationBuilder();

src/plugins/explore/public/application/in_context_vis_editor/hooks/use_initial_save_explore.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,14 @@ export const useInitialSaveExplore = () => {
2424
}, [savedExplore]);
2525

2626
// parse saved vis state and transformation from saved explore
27-
const { savedVisConfig } = useMemo(() => {
28-
if (!savedExplore?.visualization)
29-
return { savedVisConfig: undefined, savedTransformationPipeline: undefined };
27+
const savedVisConfig = useMemo(() => {
28+
if (!savedExplore?.visualization) return undefined;
3029
try {
3130
const parsedVisualization = JSON.parse(savedExplore.visualization);
3231

33-
return {
34-
savedVisConfig: parsedVisualization,
35-
};
32+
return parsedVisualization;
3633
} catch {
37-
return { savedVisConfig: undefined, savedTransformationPipeline: undefined };
34+
return undefined;
3835
}
3936
}, [savedExplore]);
4037

src/plugins/explore/public/application/in_context_vis_editor/hooks/use_transformation_service.ts

Lines changed: 0 additions & 71 deletions
This file was deleted.

src/plugins/explore/public/application/in_context_vis_editor/query_builder/query_builder.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ export class QueryBuilder {
136136
private getServices: () => ExploreServices;
137137
private interpolationService?: IVariableInterpolationService;
138138
public lastExecutedInterpolatedQuery?: string;
139+
private onDatasetChangedCallback?: () => void;
139140

140141
constructor(getServices: () => ExploreServices) {
141142
this.getServices = getServices;
@@ -296,6 +297,9 @@ export class QueryBuilder {
296297
// sync dataset change
297298
// check isLanguageChanged and isInitialized for the initial sync
298299
if (isDatasetChanged || isLanguageChanged || !this.isInitialized) {
300+
if (isDatasetChanged && this.isInitialized) {
301+
this.onDatasetChangedCallback?.();
302+
}
299303
this.datasetView$.next({ ...this.datasetView$.getValue(), isLoading: true });
300304
return from(this.handleDatasetChange(newQuery.dataset));
301305
}
@@ -610,6 +614,12 @@ export class QueryBuilder {
610614
return this.editorRef;
611615
}
612616

617+
// register a callback that fires when the dataset changes,
618+
// for example, used to clear the transformation pipeline on dataset switch.
619+
setOnDatasetChanged(callback: () => void) {
620+
this.onDatasetChangedCallback = callback;
621+
}
622+
613623
getQueryEditorState() {
614624
return this.queryEditorState$.value;
615625
}

src/plugins/explore/public/application/in_context_vis_editor/visualization_editor_page.tsx

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import { getBreadcrumbs } from './utils';
3030
import { useInitialContainerContext } from './hooks/use_initial_container_context';
3131
import { useVariables } from './hooks/use_variables';
3232
import { DashboardSelectionModal } from './component/dashboard_selection_modal';
33-
import { cleanupGlobalTransformationService } from './hooks/use_transformation_service';
3433

3534
export const VisualizationEditorPage = ({
3635
setHeaderActionMenu,
@@ -156,7 +155,6 @@ export const VisualizationEditorPage = ({
156155
return () => {
157156
queryBuilder.reset();
158157
visualizationBuilderForEditor.reset();
159-
cleanupGlobalTransformationService();
160158
};
161159
}, [queryBuilder, visualizationBuilderForEditor]);
162160

src/plugins/explore/public/components/data_transformations/transform_panel.tsx

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,40 +27,42 @@ import { TransformationInstance, FieldSchema } from './types';
2727
import { TransformationService } from './transformation_service';
2828
import { FIELD_TYPE_MAP } from '../visualizations/constants';
2929
import { VisFieldType } from '../visualizations/types';
30-
import { VisualizationBuilder } from '../visualizations/visualization_builder';
3130

3231
const DROPPABLE_ID = 'transformationPipelineDroppable';
3332

3433
export interface TransformPanelProps {
3534
transformationService: TransformationService;
36-
visualizationBuilder: VisualizationBuilder;
3735
}
3836

39-
export const TransformPanel = ({
40-
transformationService,
41-
visualizationBuilder,
42-
}: TransformPanelProps) => {
37+
export const TransformPanel = ({ transformationService }: TransformPanelProps) => {
4338
const pipeline = useObservable(
4439
transformationService.pipeline$,
4540
transformationService.pipeline$.getValue()
4641
);
4742

48-
const stageSchemas = useObservable(
49-
visualizationBuilder.stageSchemas$,
50-
visualizationBuilder.stageSchemas$.getValue()
43+
const stageSchemasMap = useObservable(
44+
transformationService.stageSchemas$,
45+
transformationService.stageSchemas$.getValue()
5146
);
5247

53-
const availableFieldsByStage = useMemo(() => {
54-
return stageSchemas.map((raw) =>
55-
raw.map((field) => ({
56-
name: field.name || '',
57-
visFieldType: FIELD_TYPE_MAP[field.type || ''] || VisFieldType.Unknown,
58-
}))
59-
);
60-
}, [stageSchemas]);
61-
62-
const getAvailableFieldsForIndex = (index: number): FieldSchema[] => {
63-
return availableFieldsByStage[index] ?? [];
48+
// stageSchemas$ is updated asynchronously via handleData
49+
// among this gap, each editor still use its last own stage schemas
50+
const availableFieldsForInstanceId = useMemo(() => {
51+
const result = new Map<string, FieldSchema[]>();
52+
stageSchemasMap.forEach((raw, instanceId) => {
53+
result.set(
54+
instanceId,
55+
raw.map((field) => ({
56+
name: field.name || '',
57+
visFieldType: FIELD_TYPE_MAP[field.type || ''] || VisFieldType.Unknown,
58+
}))
59+
);
60+
});
61+
return result;
62+
}, [stageSchemasMap]);
63+
64+
const getAvailableFieldsForInstance = (instanceId: string): FieldSchema[] => {
65+
return availableFieldsForInstanceId.get(instanceId) ?? [];
6466
};
6567

6668
const onSelectTransformation = (id: string) => {
@@ -142,7 +144,7 @@ export const TransformPanel = ({
142144
onConfigChange={onConfigChange}
143145
onToggleHide={onToggleHide}
144146
dragHandleProps={provided.dragHandleProps}
145-
availableFields={getAvailableFieldsForIndex(index)}
147+
availableFields={getAvailableFieldsForInstance(instance.instance_id)}
146148
/>
147149
)}
148150
</EuiDraggable>
@@ -204,7 +206,7 @@ const TransformationCard = ({
204206
</EuiFlexGroup>
205207
}
206208
extraAction={
207-
<EuiFlexGroup gutterSize="xs" alignItems="center" responsive={false}>
209+
<EuiFlexGroup gutterSize="xs" alignItems="center">
208210
<EuiFlexItem grow={false}>
209211
<EuiButtonIcon
210212
iconType={instance.hide ? 'eyeClosed' : 'eye'}
@@ -226,8 +228,6 @@ const TransformationCard = ({
226228
{...dragHandleProps}
227229
style={{
228230
cursor: 'grab',
229-
display: 'flex',
230-
alignItems: 'center',
231231
marginLeft: '10px',
232232
}}
233233
>

0 commit comments

Comments
 (0)