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

[Visualization] Get rid of saved object loader and use savedObjectClient resolve #113121

Merged
merged 38 commits into from
Oct 11, 2021
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
277f7ae
First step: create saved_visualize_utils, starting use new get/save m…
VladLasitsa Sep 20, 2021
2424506
Use new util methods in embeddable
VladLasitsa Sep 20, 2021
cd3c1a9
move findListItem in utils
VladLasitsa Sep 20, 2021
1560e50
some clean up
VladLasitsa Sep 21, 2021
7cd5973
clean up
VladLasitsa Sep 23, 2021
cda436d
Some fixes
VladLasitsa Sep 23, 2021
6bbcf88
Merge remote-tracking branch 'upstream/master' into get_rid_of_saved_…
VladLasitsa Sep 24, 2021
ab285e1
Fix saved object tags
VladLasitsa Sep 24, 2021
58f3c58
Some types fixes
VladLasitsa Sep 24, 2021
7af8213
Fix unit tests
VladLasitsa Sep 27, 2021
af5fa95
Clean up code
VladLasitsa Sep 28, 2021
facd0ad
Add unit tests for new utils
VladLasitsa Sep 28, 2021
e01d539
Fix lint
VladLasitsa Sep 28, 2021
e916aaf
Fix tagging
VladLasitsa Sep 29, 2021
196e3d0
Add unit tests
VladLasitsa Sep 29, 2021
c0c81e1
Some fixes
VladLasitsa Sep 29, 2021
733e4db
Clean up code
VladLasitsa Sep 30, 2021
dd6d5e2
Fix lint
VladLasitsa Sep 30, 2021
c5d1dc1
Fix types
VladLasitsa Sep 30, 2021
9197198
Merge branch 'master' into get_rid_of_saved_object_loader
kibanamachine Sep 30, 2021
620dced
put new methods in start contract
VladLasitsa Oct 1, 2021
0debc03
Fix imports
VladLasitsa Oct 1, 2021
5eb0c8a
Merge branch 'master' into get_rid_of_saved_object_loader
kibanamachine Oct 1, 2021
b87b393
Merge branch 'master' into get_rid_of_saved_object_loader
kibanamachine Oct 1, 2021
6993014
Merge branch 'master' into get_rid_of_saved_object_loader
kibanamachine Oct 4, 2021
2c89e2e
Fix lint
VladLasitsa Oct 4, 2021
66d31f5
Fix comments
VladLasitsa Oct 4, 2021
ef1c959
Fix lint
VladLasitsa Oct 4, 2021
321849d
Merge remote-tracking branch 'upstream/master' into get_rid_of_saved_…
VladLasitsa Oct 5, 2021
3a28397
Fix CI
VladLasitsa Oct 5, 2021
5715b47
use local url instead of full path
VladLasitsa Oct 5, 2021
c9707bd
Fix unit test
VladLasitsa Oct 5, 2021
0522ded
Merge branch 'master' into get_rid_of_saved_object_loader
kibanamachine Oct 6, 2021
bfd5947
Some clean up
VladLasitsa Oct 6, 2021
ff5463b
Merge branch 'master' into get_rid_of_saved_object_loader
kibanamachine Oct 7, 2021
91b0e71
Fix nits
VladLasitsa Oct 7, 2021
2679dca
fix types
VladLasitsa Oct 7, 2021
ca40c99
Merge branch 'master' into get_rid_of_saved_object_loader
kibanamachine Oct 10, 2021
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
2 changes: 1 addition & 1 deletion src/plugins/visualizations/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"inspector",
"savedObjects"
],
"optionalPlugins": ["usageCollection"],
"optionalPlugins": ["usageCollection", "spaces", "savedObjectsTaggingOss"],
"requiredBundles": ["kibanaUtils", "discover"],
"extraPublicDirs": ["common/constants", "common/prepare_log_table", "common/expression_functions"],
"owner": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,10 @@ import {
AttributeService,
} from '../../../../plugins/embeddable/public';
import { DisabledLabEmbeddable } from './disabled_lab_embeddable';
import {
getSavedVisualizationsLoader,
getUISettings,
getHttp,
getTimeFilter,
getCapabilities,
} from '../services';
import { getUISettings, getHttp, getTimeFilter, getCapabilities } from '../services';
import { urlFor } from '../utils/saved_visualize_utils';
import { VisualizeEmbeddableFactoryDeps } from './visualize_embeddable_factory';
import { VISUALIZE_ENABLE_LABS_SETTING } from '../../common/constants';
import { SavedVisualizationsLoader } from '../saved_visualizations';
import { IndexPattern } from '../../../data/public';
import { createVisualizeEmbeddableAsync } from './visualize_embeddable_async';

Expand All @@ -38,24 +32,19 @@ export const createVisEmbeddableFromObject =
async (
vis: Vis,
input: Partial<VisualizeInput> & { id: string },
savedVisualizationsLoader?: SavedVisualizationsLoader,
attributeService?: AttributeService<
VisualizeSavedObjectAttributes,
VisualizeByValueInput,
VisualizeByReferenceInput
>,
parent?: IContainer
): Promise<VisualizeEmbeddable | ErrorEmbeddable | DisabledLabEmbeddable> => {
const savedVisualizations = getSavedVisualizationsLoader();

try {
const visId = vis.id as string;

const editPath = visId ? savedVisualizations.urlFor(visId) : '#/edit_by_value';
const editPath = visId ? urlFor(visId) : '#/edit_by_value';

const editUrl = visId
? getHttp().basePath.prepend(`/app/visualize${savedVisualizations.urlFor(visId)}`)
: '';
const editUrl = visId ? getHttp().basePath.prepend(`/app/visualize${urlFor(visId)}`) : '';
const isLabsEnabled = getUISettings().get<boolean>(VISUALIZE_ENABLE_LABS_SETTING);

if (!isLabsEnabled && vis.type.stage === 'experimental') {
Expand Down Expand Up @@ -87,7 +76,6 @@ export const createVisEmbeddableFromObject =
},
input,
attributeService,
savedVisualizationsLoader,
parent
);
} catch (e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import { getExpressions, getUiActions } from '../services';
import { VIS_EVENT_TO_TRIGGER } from './events';
import { VisualizeEmbeddableFactoryDeps } from './visualize_embeddable_factory';
import { SavedObjectAttributes } from '../../../../core/types';
import { SavedVisualizationsLoader } from '../saved_visualizations';
import { getSavedVisualization } from '../utils/saved_visualize_utils';
import { VisSavedObject } from '../types';
import { toExpressionAst } from './to_ast';

Expand Down Expand Up @@ -108,7 +108,6 @@ export class VisualizeEmbeddable
VisualizeByValueInput,
VisualizeByReferenceInput
>;
private savedVisualizationsLoader?: SavedVisualizationsLoader;

constructor(
timefilter: TimefilterContract,
Expand All @@ -119,7 +118,6 @@ export class VisualizeEmbeddable
VisualizeByValueInput,
VisualizeByReferenceInput
>,
savedVisualizationsLoader?: SavedVisualizationsLoader,
parent?: IContainer
) {
super(
Expand All @@ -144,7 +142,6 @@ export class VisualizeEmbeddable
this.vis.uiState.on('change', this.uiStateChangeHandler);
this.vis.uiState.on('reload', this.reload);
this.attributeService = attributeService;
this.savedVisualizationsLoader = savedVisualizationsLoader;

if (this.attributeService) {
const isByValue = !this.inputIsRefType(initialInput);
Expand Down Expand Up @@ -455,7 +452,15 @@ export class VisualizeEmbeddable
};

getInputAsRefType = async (): Promise<VisualizeByReferenceInput> => {
const savedVis = await this.savedVisualizationsLoader?.get({});
const { savedObjectsClient, data, spaces, savedObjectsTaggingOss } = await this.deps.start()
.plugins;
const savedVis = await getSavedVisualization({
savedObjectsClient,
search: data.search,
dataViews: data.dataViews,
spaces,
savedObjectsTagging: savedObjectsTaggingOss?.getTaggingApi(),
});
if (!savedVis) {
throw new Error('Error creating a saved vis object');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,20 @@ import type {
import { VISUALIZE_EMBEDDABLE_TYPE } from './constants';
import type { SerializedVis, Vis } from '../vis';
import { createVisAsync } from '../vis_async';
import {
getCapabilities,
getTypes,
getUISettings,
getSavedVisualizationsLoader,
} from '../services';
import { getCapabilities, getTypes, getUISettings } from '../services';
import { showNewVisModal } from '../wizard';
import { convertToSerializedVis } from '../saved_visualizations/_saved_vis';
import {
convertToSerializedVis,
getSavedVisualization,
saveVisualization,
getFullPath,
} from '../utils/saved_visualize_utils';
import {
extractControlsReferences,
extractTimeSeriesReferences,
injectTimeSeriesReferences,
injectControlsReferences,
} from '../saved_visualizations/saved_visualization_references';
} from '../utils/saved_visualization_references';
import { createVisEmbeddableFromObject } from './create_vis_embeddable_from_object';
import { VISUALIZE_ENABLE_LABS_SETTING } from '../../common/constants';
import { checkForDuplicateTitle } from '../../../saved_objects/public';
Expand All @@ -59,7 +59,15 @@ interface VisualizationAttributes extends SavedObjectAttributes {

export interface VisualizeEmbeddableFactoryDeps {
start: StartServicesGetter<
Pick<VisualizationsStartDeps, 'inspector' | 'embeddable' | 'savedObjectsClient'>
Pick<
VisualizationsStartDeps,
| 'inspector'
| 'embeddable'
| 'savedObjectsClient'
| 'data'
| 'savedObjectsTaggingOss'
| 'spaces'
>
>;
}

Expand Down Expand Up @@ -147,17 +155,36 @@ export class VisualizeEmbeddableFactory
input: Partial<VisualizeInput> & { id: string },
parent?: IContainer
): Promise<VisualizeEmbeddable | ErrorEmbeddable | DisabledLabEmbeddable> {
const savedVisualizations = getSavedVisualizationsLoader();
const startDeps = await this.deps.start();

try {
const savedObject = await savedVisualizations.get(savedObjectId);
const savedObject = await getSavedVisualization(
{
savedObjectsClient: startDeps.core.savedObjects.client,
search: startDeps.plugins.data.search,
dataViews: startDeps.plugins.data.dataViews,
spaces: startDeps.plugins.spaces,
savedObjectsTagging: startDeps.plugins.savedObjectsTaggingOss?.getTaggingApi(),
},
savedObjectId
);

if (savedObject.sharingSavedObjectProps?.outcome === 'conflict') {
return new ErrorEmbeddable(
i18n.translate('visualizations.embeddable.legacyURLConflict.errorMessage', {
defaultMessage: `This visualization has the same URL as a legacy alias. Disable the alias to resolve this error : {json}`,
values: { json: savedObject.sharingSavedObjectProps?.errorJSON },
}),
input,
parent
);
}
Comment on lines +172 to +181
Copy link
Contributor

@jportner jportner Oct 10, 2021

Choose a reason for hiding this comment

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

As @flash1293 mentioned in #113121 (comment), the Spaces plugin now provides a component for this. Whether you want to change it in this PR or leave it for a follow-on PR, I just wanted to leave a note and let you know that I made some additional changes to it in #114172.
You can load this component with spaces.ui.components.getEmbeddableLegacyUrlConflict; you can see how Lens consumes it here:

if (sharingSavedObjectProps?.outcome === 'conflict' && this.deps.spaces) {
const conflictError = {
shortMessage: i18n.translate('xpack.lens.embeddable.legacyURLConflict.shortMessage', {
defaultMessage: `You've encountered a URL conflict`,
}),
longMessage: (
<this.deps.spaces.ui.components.getEmbeddableLegacyUrlConflict
targetType={DOC_TYPE}
sourceId={sharingSavedObjectProps.sourceId!}
/>
),
};
this.errors = this.errors ? [...this.errors, conflictError] : [conflictError];
}

The sourceId is the ID of the saved object.

const visState = convertToSerializedVis(savedObject);
const vis = await createVisAsync(savedObject.visState.type, visState);

return createVisEmbeddableFromObject(this.deps)(
vis,
input,
savedVisualizations,
await this.getAttributeService(),
parent
);
Expand All @@ -173,11 +200,9 @@ export class VisualizeEmbeddableFactory
if (input.savedVis) {
const visState = input.savedVis;
const vis = await createVisAsync(visState.type, visState);
const savedVisualizations = getSavedVisualizationsLoader();
return createVisEmbeddableFromObject(this.deps)(
vis,
input,
savedVisualizations,
await this.getAttributeService(),
parent
);
Expand All @@ -201,9 +226,9 @@ export class VisualizeEmbeddableFactory
confirmOverwrite: false,
returnToOrigin: true,
isTitleDuplicateConfirmed: true,
copyOnSave: false,
};
savedVis.title = title;
savedVis.copyOnSave = false;
savedVis.description = '';
savedVis.searchSourceFields = visObj?.data.searchSource?.getSerializedFields();
const serializedVis = (visObj as unknown as Vis).serialize();
Expand All @@ -217,14 +242,20 @@ export class VisualizeEmbeddableFactory
if (visObj) {
savedVis.uiStateJSON = visObj?.uiState.toString();
}
const id = await savedVis.save(saveOptions);
const { core, plugins } = await this.deps.start();
const id = await saveVisualization(savedVis, saveOptions, {
savedObjectsClient: core.savedObjects.client,
overlays: core.overlays,
savedObjectsTagging: plugins.savedObjectsTaggingOss?.getTaggingApi(),
});
if (!id || id === '') {
throw new Error(
i18n.translate('visualizations.savingVisualizationFailed.errorMsg', {
defaultMessage: 'Saving a visualization failed',
})
);
}
core.chrome.recentlyAccessed.add(getFullPath(id), savedVis.title, String(id));
return { id };
} catch (error) {
throw error;
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/visualizations/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export {
VisToExpressionAst,
VisToExpressionAstParams,
VisEditorOptionsProps,
GetVisOptions,
} from './types';
export { VisualizationListItem, VisualizationStage } from './vis_types/vis_type_alias_registry';
export { VISUALIZE_ENABLE_LABS_SETTING } from '../common/constants';
Expand All @@ -49,3 +50,4 @@ export {
FakeParams,
HistogramParams,
} from '../common/expression_functions/xy_dimension';
export { urlFor, getFullPath } from './utils/saved_visualize_utils';
7 changes: 7 additions & 0 deletions src/plugins/visualizations/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { PluginInitializerContext } from '../../../core/public';
import { Schema, VisualizationsSetup, VisualizationsStart } from './';
import { Schemas } from './vis_types';
import { VisualizationsPlugin } from './plugin';
import { spacesPluginMock } from '../../../../x-pack/plugins/spaces/public/mocks';
import { coreMock, applicationServiceMock } from '../../../core/public/mocks';
import { embeddablePluginMock } from '../../../plugins/embeddable/public/mocks';
import { expressionsPluginMock } from '../../../plugins/expressions/public/mocks';
Expand All @@ -18,6 +19,7 @@ import { usageCollectionPluginMock } from '../../../plugins/usage_collection/pub
import { uiActionsPluginMock } from '../../../plugins/ui_actions/public/mocks';
import { inspectorPluginMock } from '../../../plugins/inspector/public/mocks';
import { savedObjectsPluginMock } from '../../../plugins/saved_objects/public/mocks';
import { savedObjectTaggingOssPluginMock } from '../../saved_objects_tagging_oss/public/mocks';

const createSetupContract = (): VisualizationsSetup => ({
createBaseVisualization: jest.fn(),
Expand All @@ -34,6 +36,9 @@ const createStartContract = (): VisualizationsStart => ({
savedVisualizationsLoader: {
get: jest.fn(),
} as any,
getSavedVisualization: jest.fn(),
saveVisualization: jest.fn(),
findListItems: jest.fn(),
showNewVisModal: jest.fn(),
createVis: jest.fn(),
convertFromSerializedVis: jest.fn(),
Expand Down Expand Up @@ -61,9 +66,11 @@ const createInstance = async () => {
uiActions: uiActionsPluginMock.createStartContract(),
application: applicationServiceMock.createStartContract(),
embeddable: embeddablePluginMock.createStartContract(),
spaces: spacesPluginMock.createStartContract(),
getAttributeService: jest.fn(),
savedObjectsClient: coreMock.createStart().savedObjects.client,
savedObjects: savedObjectsPluginMock.createStartContract(),
savedObjectsTaggingOss: savedObjectTaggingOssPluginMock.createStart(),
});

return {
Expand Down
Loading