Skip to content

Commit

Permalink
[data view field editor] Allow editing of DataViewLazy (elastic#186348)
Browse files Browse the repository at this point in the history
## Summary

Data view field editor will now allow editing of fields when provided
with a DataViewLazy object. Previously it required a DataView object.
This change makes it easier for API consumers to move from DataView to
DataViewLazy usage.

Internally the data view field editor still uses DataView objects since
some of the validation code expects a complete field list. The
validation code would need to be rewritten to assume incompete field
lists. There is the potential for a performance hit when loading a large
field list. After the initial load it will be loaded from the browser
cache which should be performant.

Part of elastic#178926
  • Loading branch information
mattkime committed Jun 22, 2024
1 parent 772ace6 commit 74a202a
Show file tree
Hide file tree
Showing 16 changed files with 46 additions and 39 deletions.
6 changes: 3 additions & 3 deletions packages/kbn-unified-data-table/src/components/data_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -650,18 +650,18 @@ export const UnifiedDataTable = ({
const editField = useMemo(
() =>
onFieldEdited
? (fieldName: string) => {
? async (fieldName: string) => {
closeFieldEditor.current =
onFieldEdited &&
services?.dataViewFieldEditor?.openEditor({
(await services?.dataViewFieldEditor?.openEditor({
ctx: {
dataView,
},
fieldName,
onSave: async () => {
await onFieldEdited();
},
});
}));
}
: undefined,
[dataView, onFieldEdited, services?.dataViewFieldEditor]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ const UnifiedFieldListSidebarContainer = memo(
const editField = useMemo(
() =>
dataView && dataViewFieldEditor && searchMode === 'documents' && canEditDataView
? (fieldName?: string) => {
const ref = dataViewFieldEditor.openEditor({
? async (fieldName?: string) => {
const ref = await dataViewFieldEditor.openEditor({
ctx: {
dataView,
},
Expand Down
20 changes: 13 additions & 7 deletions src/plugins/data_view_field_editor/public/open_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,14 @@ import { euiFlyoutClassname } from './constants';
import type { ApiService } from './lib/api';
import type {
DataPublicPluginStart,
DataView,
UsageCollectionStart,
RuntimeType,
DataViewsPublicPluginStart,
FieldFormatsStart,
DataViewField,
DataViewLazy,
} from './shared_imports';
import { DataView } from './shared_imports';
import { createKibanaReactContext } from './shared_imports';
import type { CloseEditor, Field, InternalFieldType, PluginStart } from './types';

Expand All @@ -34,7 +35,7 @@ export interface OpenFieldEditorOptions {
* context containing the data view the field belongs to
*/
ctx: {
dataView: DataView;
dataView: DataView | DataViewLazy;
};
/**
* action to take after field is saved
Expand Down Expand Up @@ -72,7 +73,7 @@ export const getFieldEditorOpener =
usageCollection,
apiService,
}: Dependencies) =>
(options: OpenFieldEditorOptions): CloseEditor => {
async (options: OpenFieldEditorOptions): Promise<CloseEditor> => {
const { uiSettings, overlays, docLinks, notifications, settings, theme } = core;
const { Provider: KibanaReactContextProvider } = createKibanaReactContext({
uiSettings,
Expand All @@ -91,12 +92,12 @@ export const getFieldEditorOpener =
canCloseValidator.current = args.canCloseValidator;
};

const openEditor = ({
const openEditor = async ({
onSave,
fieldName: fieldNameToEdit,
fieldToCreate,
ctx: { dataView },
}: OpenFieldEditorOptions): CloseEditor => {
ctx: { dataView: dataViewLazyOrNot },
}: OpenFieldEditorOptions): Promise<CloseEditor> => {
const closeEditor = () => {
if (overlayRef) {
overlayRef.close();
Expand All @@ -113,7 +114,7 @@ export const getFieldEditorOpener =
};

const getRuntimeField = (name: string) => {
const fld = dataView.getAllRuntimeFields()[name];
const fld = dataViewLazyOrNot.getAllRuntimeFields()[name];
return {
name,
runtimeField: fld,
Expand All @@ -129,6 +130,11 @@ export const getFieldEditorOpener =
};
};

const dataView =
dataViewLazyOrNot instanceof DataView
? dataViewLazyOrNot
: await dataViews.toDataView(dataViewLazyOrNot);

const dataViewField = fieldNameToEdit
? dataView.getFieldByName(fieldNameToEdit) || getRuntimeField(fieldNameToEdit)
: undefined;
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data_view_field_editor/public/plugin.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('DataViewFieldEditorPlugin', () => {
};
const { openEditor } = plugin.start(coreStartMocked, pluginStart);

openEditor({ onSave: onSaveSpy, ctx: { dataView: {} as any } });
await openEditor({ onSave: onSaveSpy, ctx: { dataView: {} as any } });

expect(openFlyout).toHaveBeenCalled();

Expand All @@ -82,7 +82,7 @@ describe('DataViewFieldEditorPlugin', () => {
test('should return a handler to close the flyout', async () => {
const { openEditor } = plugin.start(coreStart, pluginStart);

const closeEditorHandler = openEditor({ onSave: noop, ctx: { dataView: {} as any } });
const closeEditorHandler = await openEditor({ onSave: noop, ctx: { dataView: {} as any } });
expect(typeof closeEditorHandler).toBe('function');
});

Expand Down
8 changes: 3 additions & 5 deletions src/plugins/data_view_field_editor/public/shared_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@

export type { DataPublicPluginStart } from '@kbn/data-plugin/public';

export type {
DataViewsPublicPluginStart,
DataView,
DataViewField,
} from '@kbn/data-views-plugin/public';
export type { DataViewsPublicPluginStart, DataViewField } from '@kbn/data-views-plugin/public';
export { DataView } from '@kbn/data-views-plugin/public';
export type { FieldFormatsStart } from '@kbn/field-formats-plugin/public';

export type { UsageCollectionStart } from '@kbn/usage-collection-plugin/public';
Expand All @@ -24,6 +21,7 @@ export type {
RuntimeFieldSubField,
RuntimeFieldSubFields,
RuntimePrimitiveTypes,
DataViewLazy,
} from '@kbn/data-views-plugin/common';
export { KBN_FIELD_TYPES, ES_FIELD_TYPES } from '@kbn/data-plugin/common';

Expand Down
4 changes: 2 additions & 2 deletions src/plugins/data_view_field_editor/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ export interface PluginStart {
/**
* Method to open the data view field editor fly-out
*/
openEditor(options: OpenFieldEditorOptions): () => void;
openEditor(options: OpenFieldEditorOptions): Promise<CloseEditor>;
/**
* Method to open the data view field delete fly-out
* @param options Configuration options for the fly-out
*/
openDeleteModal(options: OpenFieldDeleteModalOptions): () => void;
openDeleteModal(options: OpenFieldDeleteModalOptions): CloseEditor;
fieldFormatEditors: FormatEditorServiceStart['fieldFormatEditors'];
/**
* Convenience method for user permissions checks
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ export const Tabs: React.FC<TabsProps> = ({
}, []);

const openFieldEditor = useCallback(
(fieldName?: string) => {
closeEditorHandler.current = dataViewFieldEditor.openEditor({
async (fieldName?: string) => {
closeEditorHandler.current = await dataViewFieldEditor.openEditor({
ctx: {
dataView: indexPattern,
},
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data_views/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const createStartContract = (): Start => {
getIdsWithTitle: jest.fn(),
getFieldsForIndexPattern: jest.fn(),
create: jest.fn().mockReturnValue(Promise.resolve({})),
toDataView: jest.fn().mockReturnValue(Promise.resolve({})),
} as unknown as jest.Mocked<DataViewsContract>;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export const DiscoverTopNav = ({
? async (fieldName?: string, uiAction: 'edit' | 'add' = 'edit') => {
if (dataView?.id) {
const dataViewInstance = await data.dataViews.get(dataView.id);
closeFieldEditor.current = dataViewFieldEditor.openEditor({
closeFieldEditor.current = await dataViewFieldEditor.openEditor({
ctx: {
dataView: dataViewInstance,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ export function getActions(
),
type: 'icon',
icon: 'indexEdit',
onClick: (item: FieldVisConfig) => {
dataViewEditorRef.current = services.dataViewFieldEditor?.openEditor({
onClick: async (item: FieldVisConfig) => {
dataViewEditorRef.current = await services.dataViewFieldEditor?.openEditor({
ctx: { dataView },
fieldName: item.fieldName,
onSave: refreshPage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ export function DataVisualizerDataViewManagement(props: DataVisualizerDataViewMa
return null;
}

const addField = () => {
closeFieldEditor.current = dataViewFieldEditor.openEditor({
const addField = async () => {
closeFieldEditor.current = await dataViewFieldEditor.openEditor({
ctx: {
dataView: currentDataView,
},
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/lens/public/app_plugin/lens_top_nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ export const LensTopNavMenu = ({
? async (fieldName?: string, _uiAction: 'edit' | 'add' = 'edit') => {
if (currentIndexPattern?.id) {
const indexPatternInstance = await data.dataViews.get(currentIndexPattern?.id);
closeFieldEditor.current = dataViewFieldEditor.openEditor({
closeFieldEditor.current = await dataViewFieldEditor.openEditor({
ctx: {
dataView: indexPatternInstance,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ export const InnerFormBasedDataPanel = function InnerFormBasedDataPanel({
editPermission
? async (fieldName?: string, uiAction: 'edit' | 'add' = 'edit') => {
const indexPatternInstance = await dataViews.get(currentIndexPattern?.id);
closeFieldEditor.current = indexPatternFieldEditor.openEditor({
closeFieldEditor.current = await indexPatternFieldEditor.openEditor({
ctx: {
dataView: indexPatternInstance,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ export const DataViewPicker = memo(() => {

const { dataViewId } = useSelector(sourcererAdapterSelector);

const createNewDataView = useCallback(() => {
closeDataViewEditor.current = dataViewEditor.openEditor({
const createNewDataView = useCallback(async () => {
closeDataViewEditor.current = await dataViewEditor.openEditor({
// eslint-disable-next-line no-console
onSave: () => console.log('new data view saved'),
allowAdHocDataView: true,
Expand All @@ -58,7 +58,7 @@ export const DataViewPicker = memo(() => {
}

const dataViewInstance = await data.dataViews.get(dataViewId);
closeFieldEditor.current = dataViewFieldEditor.openEditor({
closeFieldEditor.current = await dataViewFieldEditor.openEditor({
ctx: {
dataView: dataViewInstance,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ describe('useFieldBrowserOptions', () => {
it('should dispatch the proper action when a new field is saved', async () => {
let onSave: ((field: DataViewField[]) => void) | undefined;
useKibanaMock().services.data.dataViews.get = () => Promise.resolve({} as DataView);
useKibanaMock().services.dataViewFieldEditor.openEditor = (options) => {
useKibanaMock().services.dataViewFieldEditor.openEditor = async (options) => {
onSave = options.onSave;
return () => {};
};
Expand Down Expand Up @@ -198,7 +198,7 @@ describe('useFieldBrowserOptions', () => {
it('should dispatch the proper actions when a field is edited', async () => {
let onSave: ((field: DataViewField[]) => void) | undefined;
useKibanaMock().services.data.dataViews.get = () => Promise.resolve({} as DataView);
useKibanaMock().services.dataViewFieldEditor.openEditor = (options) => {
useKibanaMock().services.dataViewFieldEditor.openEditor = async (options) => {
onSave = options.onSave;
return () => {};
};
Expand Down Expand Up @@ -266,7 +266,7 @@ describe('useFieldBrowserOptions', () => {
it("should store 'closeEditor' in the actions ref when editor is open by create button", async () => {
const mockCloseEditor = jest.fn();
useKibanaMock().services.data.dataViews.get = () => Promise.resolve({} as DataView);
useKibanaMock().services.dataViewFieldEditor.openEditor = () => mockCloseEditor;
useKibanaMock().services.dataViewFieldEditor.openEditor = async () => mockCloseEditor;

const editorActionsRef: FieldEditorActionsRef = React.createRef();

Expand All @@ -280,6 +280,7 @@ describe('useFieldBrowserOptions', () => {
expect(editorActionsRef?.current).toBeNull();

getByRole('button').click();
await runAllPromises();

expect(mockCloseEditor).not.toHaveBeenCalled();
expect(editorActionsRef?.current?.closeEditor).toBeDefined();
Expand All @@ -293,7 +294,7 @@ describe('useFieldBrowserOptions', () => {
it("should store 'closeEditor' in the actions ref when editor is open by edit button", async () => {
const mockCloseEditor = jest.fn();
useKibanaMock().services.data.dataViews.get = () => Promise.resolve({} as DataView);
useKibanaMock().services.dataViewFieldEditor.openEditor = () => mockCloseEditor;
useKibanaMock().services.dataViewFieldEditor.openEditor = async () => mockCloseEditor;

const editorActionsRef: FieldEditorActionsRef = React.createRef();

Expand All @@ -311,6 +312,7 @@ describe('useFieldBrowserOptions', () => {
expect(editorActionsRef?.current).toBeNull();

getByTestId('actionEditRuntimeField').click();
await runAllPromises();

expect(mockCloseEditor).not.toHaveBeenCalled();
expect(editorActionsRef?.current?.closeEditor).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ export const useFieldBrowserOptions: UseFieldBrowserOptions = ({
}, [selectedDataViewId, missingPatterns, dataViews]);

const openFieldEditor = useCallback<OpenFieldEditor>(
(fieldName) => {
async (fieldName) => {
if (dataView && selectedDataViewId) {
const closeFieldEditor = dataViewFieldEditor.openEditor({
const closeFieldEditor = await dataViewFieldEditor.openEditor({
ctx: { dataView },
fieldName,
onSave: async (savedFields: DataViewField[]) => {
Expand Down

0 comments on commit 74a202a

Please sign in to comment.