Skip to content

Commit

Permalink
[Lens] Switch to SavedObjectClient.resolve (#110059)
Browse files Browse the repository at this point in the history
* Step 2: Update client code to use resolve() method instead of get()

Following sharing Saved Objects developer guide: Step 2
This step demonstrates the changes to update client code to use the new
SavedObjectsClient `resolve()` method instead of `get()`.

* Step 3 Lens
  • Loading branch information
mbondyra authored Sep 3, 2021
1 parent 9a45980 commit d4c03eb
Show file tree
Hide file tree
Showing 23 changed files with 466 additions and 108 deletions.
3 changes: 2 additions & 1 deletion x-pack/plugins/lens/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
"usageCollection",
"taskManager",
"globalSearch",
"savedObjectsTagging"
"savedObjectsTagging",
"spaces"
],
"configPath": [
"xpack",
Expand Down
31 changes: 31 additions & 0 deletions x-pack/plugins/lens/public/app_plugin/app.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,9 @@ describe('Lens App', () => {
savedObjectId: savedObjectId || 'aaa',
}));
services.attributeService.unwrapAttributes = jest.fn().mockResolvedValue({
sharingSavedObjectProps: {
outcome: 'exactMatch',
},
savedObjectId: initialSavedObjectId ?? 'aaa',
references: [],
state: {
Expand Down Expand Up @@ -1256,4 +1259,32 @@ describe('Lens App', () => {
expect(defaultLeave).not.toHaveBeenCalled();
});
});
it('should display a conflict callout if saved object conflicts', async () => {
const history = createMemoryHistory();
const { services } = await mountWith({
props: {
...makeDefaultProps(),
history: {
...history,
location: {
...history.location,
search: '?_g=test',
},
},
},
preloadedState: {
persistedDoc: defaultDoc,
sharingSavedObjectProps: {
outcome: 'conflict',
aliasTargetId: '2',
},
},
});
expect(services.spaces.ui.components.getLegacyUrlConflict).toHaveBeenCalledWith({
currentObjectId: '1234',
objectNoun: 'Lens visualization',
otherObjectId: '2',
otherObjectPath: '#/edit/2?_g=test',
});
});
});
28 changes: 28 additions & 0 deletions x-pack/plugins/lens/public/app_plugin/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
runSaveLensVisualization,
} from './save_modal_container';
import { getLensInspectorService, LensInspector } from '../lens_inspector_service';
import { getEditPath } from '../../common';

export type SaveProps = Omit<OnSaveProps, 'onTitleDuplicate' | 'newDescription'> & {
returnToOrigin: boolean;
Expand Down Expand Up @@ -70,6 +71,8 @@ export function App({
notifications,
savedObjectsTagging,
getOriginatingAppName,
spaces,
http,
// Temporarily required until the 'by value' paradigm is default.
dashboardFeatureFlag,
} = lensAppServices;
Expand All @@ -82,6 +85,7 @@ export function App({

const {
persistedDoc,
sharingSavedObjectProps,
isLinkedToOriginatingApp,
searchSessionId,
isLoading,
Expand Down Expand Up @@ -166,6 +170,28 @@ export function App({
});
}, [onAppLeave, lastKnownDoc, isSaveable, persistedDoc, application.capabilities.visualize.save]);

const getLegacyUrlConflictCallout = useCallback(() => {
// This function returns a callout component *if* we have encountered a "legacy URL conflict" scenario
if (spaces && sharingSavedObjectProps?.outcome === 'conflict' && persistedDoc?.savedObjectId) {
// We have resolved to one object, but another object has a legacy URL alias associated with this ID/page. We should display a
// callout with a warning for the user, and provide a way for them to navigate to the other object.
const currentObjectId = persistedDoc.savedObjectId;
const otherObjectId = sharingSavedObjectProps?.aliasTargetId!; // This is always defined if outcome === 'conflict'
const otherObjectPath = http.basePath.prepend(
`${getEditPath(otherObjectId)}${history.location.search}`
);
return spaces.ui.components.getLegacyUrlConflict({
objectNoun: i18n.translate('xpack.lens.appName', {
defaultMessage: 'Lens visualization',
}),
currentObjectId,
otherObjectId,
otherObjectPath,
});
}
return null;
}, [persistedDoc, sharingSavedObjectProps, spaces, http, history]);

// Sync Kibana breadcrumbs any time the saved document's title changes
useEffect(() => {
const isByValueMode = getIsByValueMode();
Expand Down Expand Up @@ -273,6 +299,8 @@ export function App({
title={persistedDoc?.title}
lensInspector={lensInspector}
/>

{getLegacyUrlConflictCallout()}
{(!isLoading || persistedDoc) && (
<MemoizedEditorFrameWrapper
editorFrame={editorFrame}
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/lens/public/app_plugin/mounter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export async function getLensServices(
savedObjectsTagging,
usageCollection,
fieldFormats,
spaces,
} = startDependencies;

const storage = new Storage(localStorage);
Expand Down Expand Up @@ -87,6 +88,7 @@ export async function getLensServices(
},
// Temporarily required until the 'by value' paradigm is default.
dashboardFeatureFlag: startDependencies.dashboard.dashboardFeatureFlagConfig,
spaces,
};
}

Expand Down Expand Up @@ -203,7 +205,9 @@ export async function mountApp(
trackUiEvent('loaded');
const initialInput = getInitialInput(props.id, props.editByValue);

lensStore.dispatch(loadInitial({ redirectCallback, initialInput, emptyState }));
lensStore.dispatch(
loadInitial({ redirectCallback, initialInput, emptyState, history: props.history })
);

return (
<Provider store={lensStore}>
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/lens/public/app_plugin/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type { History } from 'history';
import type { OnSaveProps } from 'src/plugins/saved_objects/public';
import { SpacesApi } from '../../../spaces/public';
import type {
ApplicationStart,
AppMountParameters,
Expand Down Expand Up @@ -116,6 +117,8 @@ export interface LensAppServices {
savedObjectsTagging?: SavedObjectTaggingPluginStart;
getOriginatingAppName: () => string | undefined;
presentationUtil: PresentationUtilPluginStart;
spaces: SpacesApi;

// Temporarily required until the 'by value' paradigm is default.
dashboardFeatureFlag: DashboardFeatureFlagConfig;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export interface WorkspacePanelProps {
interface WorkspaceState {
expressionBuildError?: Array<{
shortMessage: string;
longMessage: string;
longMessage: React.ReactNode;
fixAction?: DatasourceFixAction<unknown>;
}>;
expandError: boolean;
Expand Down Expand Up @@ -416,10 +416,10 @@ export const VisualizationWrapper = ({
localState: WorkspaceState & {
configurationValidationError?: Array<{
shortMessage: string;
longMessage: string;
longMessage: React.ReactNode;
fixAction?: DatasourceFixAction<unknown>;
}>;
missingRefsErrors?: Array<{ shortMessage: string; longMessage: string }>;
missingRefsErrors?: Array<{ shortMessage: string; longMessage: React.ReactNode }>;
};
ExpressionRendererComponent: ReactExpressionRendererType;
application: ApplicationStart;
Expand Down Expand Up @@ -454,7 +454,7 @@ export const VisualizationWrapper = ({
validationError:
| {
shortMessage: string;
longMessage: string;
longMessage: React.ReactNode;
fixAction?: DatasourceFixAction<unknown>;
}
| undefined
Expand Down Expand Up @@ -499,7 +499,7 @@ export const VisualizationWrapper = ({
.map((validationError) => (
<>
<p
key={validationError.longMessage}
key={validationError.shortMessage}
className="eui-textBreakWord"
data-test-subj="configuration-failure-error"
>
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/lens/public/editor_frame_service/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ export type TableInspectorAdapter = Record<string, Datatable>;

export interface ErrorMessage {
shortMessage: string;
longMessage: string;
longMessage: React.ReactNode;
type?: 'fixable' | 'critical';
}
56 changes: 53 additions & 3 deletions x-pack/plugins/lens/public/embeddable/embeddable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
LensByReferenceInput,
LensSavedObjectAttributes,
LensEmbeddableInput,
ResolvedLensSavedObjectAttributes,
} from './embeddable';
import { ReactExpressionRendererProps } from 'src/plugins/expressions/public';
import { Query, TimeRange, Filter, IndexPatternsContract } from 'src/plugins/data/public';
Expand Down Expand Up @@ -68,12 +69,17 @@ const options = {
const attributeServiceMockFromSavedVis = (document: Document): LensAttributeService => {
const core = coreMock.createStart();
const service = new AttributeService<
LensSavedObjectAttributes,
ResolvedLensSavedObjectAttributes,
LensByValueInput,
LensByReferenceInput
>('lens', jest.fn(), core.i18n.Context, core.notifications.toasts, options);
service.unwrapAttributes = jest.fn((input: LensByValueInput | LensByReferenceInput) => {
return Promise.resolve({ ...document } as LensSavedObjectAttributes);
return Promise.resolve({
...document,
sharingSavedObjectProps: {
outcome: 'exactMatch',
},
} as ResolvedLensSavedObjectAttributes);
});
service.wrapAttributes = jest.fn();
return service;
Expand All @@ -86,7 +92,7 @@ describe('embeddable', () => {
let trigger: { exec: jest.Mock };
let basePath: IBasePath;
let attributeService: AttributeService<
LensSavedObjectAttributes,
ResolvedLensSavedObjectAttributes,
LensByValueInput,
LensByReferenceInput
>;
Expand Down Expand Up @@ -223,6 +229,50 @@ describe('embeddable', () => {
expect(expressionRenderer).toHaveBeenCalledTimes(0);
});

it('should not render the vis if loaded saved object conflicts', async () => {
attributeService.unwrapAttributes = jest.fn(
(input: LensByValueInput | LensByReferenceInput) => {
return Promise.resolve({
...savedVis,
sharingSavedObjectProps: {
outcome: 'conflict',
errorJSON: '{targetType: "lens", sourceId: "1", targetSpace: "space"}',
aliasTargetId: '2',
},
} as ResolvedLensSavedObjectAttributes);
}
);
const embeddable = new Embeddable(
{
timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter,
attributeService,
inspector: inspectorPluginMock.createStartContract(),
expressionRenderer,
basePath,
indexPatternService: {} as IndexPatternsContract,
capabilities: {
canSaveDashboards: true,
canSaveVisualizations: true,
},
getTrigger,
documentToExpression: () =>
Promise.resolve({
ast: {
type: 'expression',
chain: [
{ type: 'function', function: 'my', arguments: {} },
{ type: 'function', function: 'expression', arguments: {} },
],
},
errors: undefined,
}),
},
{} as LensEmbeddableInput
);
await embeddable.initializeSavedVis({} as LensEmbeddableInput);
expect(expressionRenderer).toHaveBeenCalledTimes(0);
});

it('should initialize output with deduped list of index patterns', async () => {
attributeService = attributeServiceMockFromSavedVis({
...savedVis,
Expand Down
27 changes: 21 additions & 6 deletions x-pack/plugins/lens/public/embeddable/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ import {
ReferenceOrValueEmbeddable,
} from '../../../../../src/plugins/embeddable/public';
import { Document, injectFilterReferences } from '../persistence';
import { ExpressionWrapper, ExpressionWrapperProps } from './expression_wrapper';
import {
ExpressionWrapper,
ExpressionWrapperProps,
savedObjectConflictError,
} from './expression_wrapper';
import { UiActionsStart } from '../../../../../src/plugins/ui_actions/public';
import {
isLensBrushEvent,
Expand All @@ -58,8 +62,12 @@ import { IBasePath } from '../../../../../src/core/public';
import { LensAttributeService } from '../lens_attribute_service';
import type { ErrorMessage } from '../editor_frame_service/types';
import { getLensInspectorService, LensInspector } from '../lens_inspector_service';
import { SharingSavedObjectProps } from '../types';

export type LensSavedObjectAttributes = Omit<Document, 'savedObjectId' | 'type'>;
export interface ResolvedLensSavedObjectAttributes extends LensSavedObjectAttributes {
sharingSavedObjectProps?: SharingSavedObjectProps;
}

interface LensBaseEmbeddableInput extends EmbeddableInput {
filters?: Filter[];
Expand All @@ -76,7 +84,7 @@ interface LensBaseEmbeddableInput extends EmbeddableInput {
}

export type LensByValueInput = {
attributes: LensSavedObjectAttributes;
attributes: ResolvedLensSavedObjectAttributes;
} & LensBaseEmbeddableInput;

export type LensByReferenceInput = SavedObjectEmbeddableInput & LensBaseEmbeddableInput;
Expand Down Expand Up @@ -253,24 +261,31 @@ export class Embeddable
}

async initializeSavedVis(input: LensEmbeddableInput) {
const attributes:
| LensSavedObjectAttributes
const attrs:
| ResolvedLensSavedObjectAttributes
| false = await this.deps.attributeService.unwrapAttributes(input).catch((e: Error) => {
this.onFatalError(e);
return false;
});
if (!attributes || this.isDestroyed) {
if (!attrs || this.isDestroyed) {
return;
}

const { sharingSavedObjectProps, ...attributes } = attrs;

this.savedVis = {
...attributes,
type: this.type,
savedObjectId: (input as LensByReferenceInput)?.savedObjectId,
};
const { ast, errors } = await this.deps.documentToExpression(this.savedVis);
this.errors = errors;
if (sharingSavedObjectProps?.outcome === 'conflict') {
const conflictError = savedObjectConflictError(sharingSavedObjectProps.errorJSON!);
this.errors = this.errors ? [...this.errors, conflictError] : [conflictError];
}
this.expression = ast ? toExpression(ast) : null;
if (errors) {
if (this.errors) {
this.logError('validation');
}
await this.initializeOutput();
Expand Down
Loading

0 comments on commit d4c03eb

Please sign in to comment.