Skip to content
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

Add "Popular fields" to Lens as part of the "Unified Field List" initiative #147704

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/data_views_service/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export function convertDataViewIntoLensIndexPattern(
// Convert the getters on the index pattern service into plain JSON
const base = {
name: field.name,
count: field.count,
displayName: field.displayName,
type: field.type,
aggregatable: field.aggregatable,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -345,13 +345,19 @@ export const InnerFormBasedDataPanel = function InnerFormBasedDataPanel({
[core.uiSettings]
);

const popularFieldsLimit = useMemo(
() => core.uiSettings.get('fields:popularLimit'),
[core.uiSettings]
);

const fieldListGroupedProps = useGroupedFields<IndexPatternField>({
dataViewId: currentIndexPatternId,
allFields,
services: {
dataViews,
},
fieldsExistenceReader,
popularFieldsLimit: popularFieldsLimit ?? 0,
isAffectedByGlobalFilter: Boolean(filters.length),
onFilterField,
onSupportedFieldFilter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { LayerPanel } from './layer_panel';
import { coreMock } from '@kbn/core/public/mocks';
import { UiActionsStart } from '@kbn/ui-actions-plugin/public';
import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
import { generateId } from '../../../id_generator';
import { mountWithProvider } from '../../../mocks';
import { LayerTypes } from '@kbn/expression-xy-plugin/public';
Expand Down Expand Up @@ -135,6 +136,7 @@ describe('ConfigPanel', () => {
isFullscreen: false,
toggleFullscreen: jest.fn(),
uiActions,
dataViewsService: dataViewPluginMocks.createStartContract(),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { FramePublicAPI, Visualization, VisualizationConfigProps } from '../../.
import { LayerPanel } from './layer_panel';
import { ChildDragDropProvider, DragDrop } from '../../../drag_drop';
import { coreMock } from '@kbn/core/public/mocks';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
import { generateId } from '../../../id_generator';
import {
createMockVisualization,
Expand Down Expand Up @@ -113,6 +114,7 @@ describe('LayerPanel', () => {
onEmptyDimensionAdd: jest.fn(),
onChangeIndexPattern: jest.fn(),
indexPatternService: createIndexPatternServiceMock(),
dataViewsService: dataViewPluginMocks.createStartContract(),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import {
import { onDropForVisualization, shouldRemoveSource } from './buttons/drop_targets_utils';
import { getSharedActions } from './layer_actions/layer_actions';
import { FlyoutContainer } from './flyout_container';
import { popularizeField } from '../../../utils';

// hide the random sampling settings from the UI
const DISPLAY_RANDOM_SAMPLING_SETTINGS = false;
Expand Down Expand Up @@ -118,6 +119,7 @@ export function LayerPanel(
visualizationState,
onChangeIndexPattern,
core,
indexPatternService,
} = props;

const datasourceStates = useLensSelector(selectDatasourceStates);
Expand Down Expand Up @@ -214,6 +216,18 @@ export function LayerPanel(
);
}
if (hasDropSucceeded) {
if (dropType === 'field_replace' || dropType === 'field_add') {
popularizeField(
source.indexPatternId as string,
source.id,
props.dataViewsService,
core.application.capabilities,
indexPatternService,
framePublicAPI.dataViews.indexPatterns,
datasourceStates,
props.datasourceMap
);
}
activeVisualization.onDrop = activeVisualization.onDrop?.bind(activeVisualization);

updateVisualization(
Expand Down Expand Up @@ -249,6 +263,9 @@ export function LayerPanel(
activeVisualization,
updateVisualization,
props,
core.application.capabilities,
datasourceStates,
indexPatternService,
]);

const isDimensionPanelOpen = Boolean(activeId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';
import { UiActionsStart } from '@kbn/ui-actions-plugin/public';
import { IndexPatternServiceAPI } from '../../../data_views_service/service';

Expand All @@ -23,6 +24,7 @@ export interface ConfigPanelWrapperProps {
core: DatasourceDimensionEditorProps['core'];
indexPatternService: IndexPatternServiceAPI;
uiActions: UiActionsStart;
dataViewsService: DataViewsPublicPluginStart;
}

export interface LayerPanelProps {
Expand All @@ -31,6 +33,7 @@ export interface LayerPanelProps {
activeVisualization: Visualization;
framePublicAPI: FramePublicAPI;
core: DatasourceDimensionEditorProps['core'];
dataViewsService: DataViewsPublicPluginStart;
}

export interface LayerDatasourceDropProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import type { LensInspector } from '../../lens_inspector_service';
import { ErrorBoundary, showMemoizedErrorNotification } from '../../lens_ui_errors';
import { IndexPatternServiceAPI } from '../../data_views_service/service';
import { popularizeField } from '../../utils';

export interface EditorFrameProps {
datasourceMap: DatasourceMap;
Expand Down Expand Up @@ -85,11 +86,30 @@ export function EditorFrame(props: EditorFrameProps) {
(field) => {
const suggestion = getSuggestionForField.current!(field);
if (suggestion) {
popularizeField(
field.indexPatternId,
field.id,
props.plugins.dataViews,
props.core.application.capabilities,
props.indexPatternService,
framePublicAPI.dataViews.indexPatterns,
datasourceStates,
datasourceMap
);
trackUiCounterEvents('drop_onto_workspace');
switchToSuggestion(dispatchLens, suggestion, { clearStagedPreview: true });
}
},
[getSuggestionForField, dispatchLens]
[
getSuggestionForField,
dispatchLens,
props.plugins.dataViews,
props.core.application.capabilities,
props.indexPatternService,
framePublicAPI.dataViews.indexPatterns,
datasourceStates,
datasourceMap,
]
);

const onError = useCallback((error: Error) => {
Expand Down Expand Up @@ -139,6 +159,7 @@ export function EditorFrame(props: EditorFrameProps) {
framePublicAPI={framePublicAPI}
uiActions={props.plugins.uiActions}
indexPatternService={props.indexPatternService}
dataViewsService={props.plugins.dataViews}
/>
</ErrorBoundary>
)
Expand All @@ -156,6 +177,7 @@ export function EditorFrame(props: EditorFrameProps) {
visualizationMap={visualizationMap}
framePublicAPI={framePublicAPI}
getSuggestionForField={getSuggestionForField.current}
indexPatternService={props.indexPatternService}
/>
</ErrorBoundary>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { UiActionsStart } from '@kbn/ui-actions-plugin/public';
import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks';
import { TriggerContract } from '@kbn/ui-actions-plugin/public/triggers';
import { VIS_EVENT_TO_TRIGGER } from '@kbn/visualizations-plugin/public/embeddable';
import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks';
import {
applyChanges,
setState,
Expand All @@ -44,6 +45,7 @@ import { getLensInspectorService } from '../../../lens_inspector_service';
import { inspectorPluginMock } from '@kbn/inspector-plugin/public/mocks';
import { disableAutoApply, enableAutoApply } from '../../../state_management/lens_slice';
import { Ast, toExpression } from '@kbn/interpreter';
import { createIndexPatternServiceMock } from '../../../mocks/data_views_service_mock';

const defaultPermissions: Record<string, Record<string, boolean | Record<string, boolean>>> = {
navLinks: { management: true },
Expand All @@ -67,10 +69,12 @@ const defaultProps = {
plugins: {
uiActions: uiActionsPluginMock.createStartContract(),
data: mockDataPlugin(),
dataViews: dataViewPluginMocks.createStartContract(),
},
getSuggestionForField: () => undefined,
lensInspector: getLensInspectorService(inspectorPluginMock.createStartContract()),
toggleFullscreen: jest.fn(),
indexPatternService: createIndexPatternServiceMock(),
};

const toExpr = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@ import type {
ExpressionRenderError,
ReactExpressionRendererType,
} from '@kbn/expressions-plugin/public';
import type { DataViewsPublicPluginStart } from '@kbn/data-views-plugin/public';
import type { UiActionsStart } from '@kbn/ui-actions-plugin/public';
import { VIS_EVENT_TO_TRIGGER } from '@kbn/visualizations-plugin/public';
import type { DefaultInspectorAdapters } from '@kbn/expressions-plugin/common';
import type { Datatable } from '@kbn/expressions-plugin/public';
import { DropIllustration } from '@kbn/chart-icons';
import { trackUiCounterEvents } from '../../../lens_ui_telemetry';
import { getSearchWarningMessages } from '../../../utils';
import { getSearchWarningMessages, popularizeField } from '../../../utils';
import {
FramePublicAPI,
isLensBrushEvent,
Expand All @@ -48,7 +49,7 @@ import {
Suggestion,
DatasourceLayers,
} from '../../../types';
import { DragDrop, DragContext, DragDropIdentifier } from '../../../drag_drop';
import { DragDrop, DragContext, DragDropIdentifier, DragContextState } from '../../../drag_drop';
import { switchToSuggestion } from '../suggestion_helpers';
import { buildExpression } from '../expression_helpers';
import { WorkspacePanelWrapper } from './workspace_panel_wrapper';
Expand Down Expand Up @@ -84,16 +85,22 @@ import {
import type { LensInspector } from '../../../lens_inspector_service';
import { inferTimeField, DONT_CLOSE_DIMENSION_CONTAINER_ON_CLICK_CLASS } from '../../../utils';
import { setChangesApplied } from '../../../state_management/lens_slice';
import { IndexPatternServiceAPI } from '../../../data_views_service/service';

export interface WorkspacePanelProps {
visualizationMap: VisualizationMap;
datasourceMap: DatasourceMap;
framePublicAPI: FramePublicAPI;
ExpressionRenderer: ReactExpressionRendererType;
core: CoreStart;
plugins: { uiActions?: UiActionsStart; data: DataPublicPluginStart };
plugins: {
uiActions?: UiActionsStart;
data: DataPublicPluginStart;
dataViews: DataViewsPublicPluginStart;
};
getSuggestionForField: (field: DragDropIdentifier) => Suggestion | undefined;
lensInspector: LensInspector;
indexPatternService: IndexPatternServiceAPI;
}

interface WorkspaceState {
Expand Down Expand Up @@ -137,7 +144,11 @@ export const WorkspacePanel = React.memo(function WorkspacePanel(props: Workspac
);

return (
<InnerWorkspacePanel {...restProps} suggestionForDraggedField={suggestionForDraggedField} />
<InnerWorkspacePanel
{...restProps}
suggestionForDraggedField={suggestionForDraggedField}
dragDropCtx={dragDropContext}
/>
);
});

Expand All @@ -151,8 +162,11 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({
ExpressionRenderer: ExpressionRendererComponent,
suggestionForDraggedField,
lensInspector,
indexPatternService,
dragDropCtx,
}: Omit<WorkspacePanelProps, 'getSuggestionForField'> & {
suggestionForDraggedField: Suggestion | undefined;
dragDropCtx: DragContextState;
}) {
const dispatchLens = useLensDispatch();
const isFullscreen = useLensSelector(selectIsFullscreenDatasource);
Expand Down Expand Up @@ -475,11 +489,31 @@ export const InnerWorkspacePanel = React.memo(function InnerWorkspacePanel({
}, [expressionExists, localState.expressionBuildError]);

const onDrop = useCallback(() => {
if (suggestionForDraggedField) {
if (suggestionForDraggedField && dragDropCtx.dragging) {
popularizeField(
dragDropCtx.dragging.indexPatternId as string,
dragDropCtx.dragging.id,
plugins.dataViews,
core.application.capabilities,
indexPatternService,
framePublicAPI.dataViews.indexPatterns,
datasourceStates,
datasourceMap
);
trackUiCounterEvents('drop_onto_workspace');
switchToSuggestion(dispatchLens, suggestionForDraggedField, { clearStagedPreview: true });
}
}, [suggestionForDraggedField, dispatchLens]);
}, [
suggestionForDraggedField,
dragDropCtx.dragging,
plugins.dataViews,
core.application.capabilities,
dispatchLens,
datasourceStates,
indexPatternService,
framePublicAPI.dataViews.indexPatterns,
datasourceMap,
]);

const IS_DARK_THEME = core.uiSettings.get('theme:darkMode');

Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/lens/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,10 @@ export type DatasourceDimensionEditorProps<T = unknown> = DatasourceDimensionPro
forceRender?: boolean;
}
>;
core: Pick<CoreStart, 'http' | 'notifications' | 'uiSettings' | 'overlays' | 'theme'>;
core: Pick<
CoreStart,
'http' | 'notifications' | 'uiSettings' | 'overlays' | 'theme' | 'application'
>;
dateRange: DateRange;
dimensionGroups: VisualizationDimensionGroupConfig[];
toggleFullscreen: () => void;
Expand Down
53 changes: 51 additions & 2 deletions x-pack/plugins/lens/public/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@ import moment from 'moment-timezone';
import type { Serializable } from '@kbn/utility-types';

import type { TimefilterContract } from '@kbn/data-plugin/public';
import type { IUiSettingsClient, SavedObjectReference } from '@kbn/core/public';
import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/public';
import type { Capabilities, IUiSettingsClient, SavedObjectReference } from '@kbn/core/public';
import type {
DataView,
DataViewsContract,
DataViewsPublicPluginStart,
} from '@kbn/data-views-plugin/public';
import type { DatatableUtilitiesService } from '@kbn/data-plugin/common';
import { BrushTriggerEvent, ClickTriggerEvent } from '@kbn/charts-plugin/public';
import { RequestAdapter } from '@kbn/inspector-plugin/common';
Expand Down Expand Up @@ -328,3 +332,48 @@ export const getSearchWarningMessages = (

return [...warningsMap.values()].flat();
};

export async function popularizeField(
indexPatternId: string,
fieldName: string,
dataViewsService: DataViewsPublicPluginStart,
capabilities: Capabilities,
indexPatternService: IndexPatternServiceAPI,
indexPatternsCache: IndexPatternMap,
datasourceStates: DatasourceStates,
datasourceMap: DatasourceMap
) {
if (!indexPatternId || !capabilities?.indexPatterns?.save) return;
const dataView = await dataViewsService.get(indexPatternId);
const field = dataView.fields.getByName(fieldName);
if (!field) {
return;
}

field.count++;

const refreshIndexPatternsListArgs = {
activeDatasources: Object.keys(datasourceStates).reduce(
(acc, datasourceId) => ({
...acc,
[datasourceId]: datasourceMap[datasourceId],
}),
{}
),
indexPatternId,
indexPatternService,
indexPatternsCache,
};

if (!dataView.isPersisted()) {
refreshIndexPatternsList(refreshIndexPatternsListArgs);
return;
}

// Catch 409 errors caused by user adding columns in a higher frequency that the changes can be persisted to Elasticsearch
try {
await dataViewsService.updateSavedObject(dataView, 0, true);
refreshIndexPatternsList(refreshIndexPatternsListArgs);
// eslint-disable-next-line no-empty
} catch {}
Comment on lines +346 to +378
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would refactor a bit here to do the async work only in case of success for a persisted dataView:

Suggested change
if (!indexPatternId || !capabilities?.indexPatterns?.save) return;
const dataView = await dataViewsService.get(indexPatternId);
const field = dataView.fields.getByName(fieldName);
if (!field) {
return;
}
field.count++;
const refreshIndexPatternsListArgs = {
activeDatasources: Object.keys(datasourceStates).reduce(
(acc, datasourceId) => ({
...acc,
[datasourceId]: datasourceMap[datasourceId],
}),
{}
),
indexPatternId,
indexPatternService,
indexPatternsCache,
};
if (!dataView.isPersisted()) {
refreshIndexPatternsList(refreshIndexPatternsListArgs);
return;
}
// Catch 409 errors caused by user adding columns in a higher frequency that the changes can be persisted to Elasticsearch
try {
await dataViewsService.updateSavedObject(dataView, 0, true);
refreshIndexPatternsList(refreshIndexPatternsListArgs);
// eslint-disable-next-line no-empty
} catch {}
if (!indexPatternId || !capabilities?.indexPatterns?.save) return;
const lensDataView = indexPatternsCache[indexPatternId];
if (!lensDataView.getFieldByName(fieldName)) {
return;
}
if (lensDataView.isPersisted) {
// Catch 409 errors caused by user adding columns in a higher frequency that the changes can be persisted to Elasticsearch
try {
const dataView = await dataViewsService.get(indexPatternId);
const field = dataView.fields.getByName(fieldName);
if (field) {
field.count++;
await dataViewsService.updateSavedObject(dataView, 0, true);
}
// eslint-disable-next-line no-empty
} catch {}
}
const refreshIndexPatternsListArgs = {
activeDatasources: Object.keys(datasourceStates).reduce(
(acc, datasourceId) => ({
...acc,
[datasourceId]: datasourceMap[datasourceId],
}),
{}
),
indexPatternId,
indexPatternService,
indexPatternsCache,
};
refreshIndexPatternsList(refreshIndexPatternsListArgs);

It is a little bit more verbose but it saves some work for adHoc dataviews

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @dej611, I will update my function. About batch requests I don't think that is a good idea. I call refreshIndexPatternsList here to show correct state of fields after updating popularity. In you proposal we should call refreshIndexPatternsList after some timeout which lead to showing correct list of popular fields with delay.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dej611, also for adHoc dataviews we also should use this field.count++; not only for usual dataviews.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wait for the main decision on the feature first.
But for the adHoc I think that operating on the cached version should be ok, no?

}