Skip to content

Commit

Permalink
[NP] Graph: get rid of saved objects class wrapper (#59917)
Browse files Browse the repository at this point in the history
* Implement find saved workspace

* Implement get saved workspace

* Create helper function as applyESResp

* Fix eslint

* Implement savedWorkspaceLoader.get()

* Implement deleteWS

* Implement saveWS

* Remove applyESRespUtil

* Refactoring

* Refactoring

* Remove savedWorkspaceLoader

* Update unit test

* Fix merge conflicts

* Add unit tests for saveWithConfirmation

* Fix TS

* Move saveWithConfirmation to a separate file

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
maryia-lapata and elasticmachine authored Mar 24, 2020
1 parent 3cb7053 commit 78e0bdf
Show file tree
Hide file tree
Showing 20 changed files with 478 additions and 180 deletions.
8 changes: 7 additions & 1 deletion src/plugins/saved_objects/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ import { SavedObjectsPublicPlugin } from './plugin';

export { OnSaveProps, SavedObjectSaveModal, SaveResult, showSaveModal } from './save_modal';
export { getSavedObjectFinder, SavedObjectFinderUi, SavedObjectMetaData } from './finder';
export { SavedObjectLoader, createSavedObjectClass } from './saved_object';
export {
SavedObjectLoader,
createSavedObjectClass,
checkForDuplicateTitle,
saveWithConfirmation,
isErrorNonFatal,
} from './saved_object';
export { SavedObjectSaveOpts, SavedObjectKibanaServices, SavedObject } from './types';

export const plugin = () => new SavedObjectsPublicPlugin();
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { checkForDuplicateTitle } from './check_for_duplicate_title';
* @param error {Error} the error
* @return {boolean}
*/
function isErrorNonFatal(error: { message: string }) {
export function isErrorNonFatal(error: { message: string }) {
if (!error) return false;
return error.message === OVERWRITE_REJECTED || error.message === SAVE_DUPLICATE_REJECTED;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { SavedObjectAttributes, SavedObjectsCreateOptions, OverlayStart } from 'kibana/public';
import { SavedObjectsClientContract } from '../../../../../core/public';
import { saveWithConfirmation } from './save_with_confirmation';
import * as deps from './confirm_modal_promise';
import { OVERWRITE_REJECTED } from '../../constants';

describe('saveWithConfirmation', () => {
const savedObjectsClient: SavedObjectsClientContract = {} as SavedObjectsClientContract;
const overlays: OverlayStart = {} as OverlayStart;
const source: SavedObjectAttributes = {} as SavedObjectAttributes;
const options: SavedObjectsCreateOptions = {} as SavedObjectsCreateOptions;
const savedObject = {
getEsType: () => 'test type',
title: 'test title',
displayName: 'test display name',
};

beforeEach(() => {
savedObjectsClient.create = jest.fn();
jest.spyOn(deps, 'confirmModalPromise').mockReturnValue(Promise.resolve({} as any));
});

test('should call create of savedObjectsClient', async () => {
await saveWithConfirmation(source, savedObject, options, { savedObjectsClient, overlays });
expect(savedObjectsClient.create).toHaveBeenCalledWith(
savedObject.getEsType(),
source,
options
);
});

test('should call confirmModalPromise when such record exists', async () => {
savedObjectsClient.create = jest
.fn()
.mockImplementation((type, src, opt) =>
opt && opt.overwrite ? Promise.resolve({} as any) : Promise.reject({ res: { status: 409 } })
);

await saveWithConfirmation(source, savedObject, options, { savedObjectsClient, overlays });
expect(deps.confirmModalPromise).toHaveBeenCalledWith(
expect.any(String),
expect.any(String),
expect.any(String),
overlays
);
});

test('should call create of savedObjectsClient when overwriting confirmed', async () => {
savedObjectsClient.create = jest
.fn()
.mockImplementation((type, src, opt) =>
opt && opt.overwrite ? Promise.resolve({} as any) : Promise.reject({ res: { status: 409 } })
);

await saveWithConfirmation(source, savedObject, options, { savedObjectsClient, overlays });
expect(savedObjectsClient.create).toHaveBeenLastCalledWith(savedObject.getEsType(), source, {
overwrite: true,
...options,
});
});

test('should reject when overwriting denied', async () => {
savedObjectsClient.create = jest.fn().mockReturnValue(Promise.reject({ res: { status: 409 } }));
jest.spyOn(deps, 'confirmModalPromise').mockReturnValue(Promise.reject());

expect.assertions(1);
await expect(
saveWithConfirmation(source, savedObject, options, {
savedObjectsClient,
overlays,
})
).rejects.toThrow(OVERWRITE_REJECTED);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { get } from 'lodash';
import { i18n } from '@kbn/i18n';
import {
SavedObjectAttributes,
SavedObjectsCreateOptions,
OverlayStart,
SavedObjectsClientContract,
} from 'kibana/public';
import { OVERWRITE_REJECTED } from '../../constants';
import { confirmModalPromise } from './confirm_modal_promise';

/**
* Attempts to create the current object using the serialized source. If an object already
* exists, a warning message requests an overwrite confirmation.
* @param source - serialized version of this object what will be indexed into elasticsearch.
* @param savedObject - a simple object that contains properties title and displayName, and getEsType method
* @param options - options to pass to the saved object create method
* @param services - provides Kibana services savedObjectsClient and overlays
* @returns {Promise} - A promise that is resolved with the objects id if the object is
* successfully indexed. If the overwrite confirmation was rejected, an error is thrown with
* a confirmRejected = true parameter so that case can be handled differently than
* a create or index error.
* @resolved {SavedObject}
*/
export async function saveWithConfirmation(
source: SavedObjectAttributes,
savedObject: {
getEsType(): string;
title: string;
displayName: string;
},
options: SavedObjectsCreateOptions,
services: { savedObjectsClient: SavedObjectsClientContract; overlays: OverlayStart }
) {
const { savedObjectsClient, overlays } = services;
try {
return await savedObjectsClient.create(savedObject.getEsType(), source, options);
} catch (err) {
// record exists, confirm overwriting
if (get(err, 'res.status') === 409) {
const confirmMessage = i18n.translate(
'savedObjects.confirmModal.overwriteConfirmationMessage',
{
defaultMessage: 'Are you sure you want to overwrite {title}?',
values: { title: savedObject.title },
}
);

const title = i18n.translate('savedObjects.confirmModal.overwriteTitle', {
defaultMessage: 'Overwrite {name}?',
values: { name: savedObject.displayName },
});
const confirmButtonText = i18n.translate('savedObjects.confirmModal.overwriteButtonLabel', {
defaultMessage: 'Overwrite',
});

return confirmModalPromise(confirmMessage, title, confirmButtonText, overlays)
.then(() =>
savedObjectsClient.create(savedObject.getEsType(), source, {
overwrite: true,
...options,
})
)
.catch(() => Promise.reject(new Error(OVERWRITE_REJECTED)));
}
return await Promise.reject(err);
}
}
3 changes: 3 additions & 0 deletions src/plugins/saved_objects/public/saved_object/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@

export { createSavedObjectClass } from './saved_object';
export { SavedObjectLoader } from './saved_object_loader';
export { checkForDuplicateTitle } from './helpers/check_for_duplicate_title';
export { saveWithConfirmation } from './helpers/save_with_confirmation';
export { isErrorNonFatal } from './helpers/save_saved_object';
26 changes: 19 additions & 7 deletions x-pack/plugins/graph/public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ import { asAngularSyncedObservable } from './helpers/as_observable';
import { colorChoices } from './helpers/style_choices';
import { createGraphStore, datasourceSelector, hasFieldsSelector } from './state_management';
import { formatHttpError } from './helpers/format_http_error';
import {
findSavedWorkspace,
getSavedWorkspace,
deleteSavedWorkspace,
} from './helpers/saved_workspace_utils';

export function initGraphApp(angularModule, deps) {
const {
Expand All @@ -42,7 +47,6 @@ export function initGraphApp(angularModule, deps) {
getBasePath,
data,
config,
savedWorkspaceLoader,
capabilities,
coreStart,
storage,
Expand Down Expand Up @@ -112,15 +116,21 @@ export function initGraphApp(angularModule, deps) {
$location.url(getNewPath());
};
$scope.find = search => {
return savedWorkspaceLoader.find(search, $scope.listingLimit);
return findSavedWorkspace(
{ savedObjectsClient, basePath: coreStart.http.basePath },
search,
$scope.listingLimit
);
};
$scope.editItem = workspace => {
$location.url(getEditPath(workspace));
};
$scope.getViewUrl = workspace => getEditUrl(addBasePath, workspace);
$scope.delete = workspaces => {
return savedWorkspaceLoader.delete(workspaces.map(({ id }) => id));
};
$scope.delete = workspaces =>
deleteSavedWorkspace(
savedObjectsClient,
workspaces.map(({ id }) => id)
);
$scope.capabilities = capabilities;
$scope.initialFilter = $location.search().filter || '';
$scope.coreStart = coreStart;
Expand All @@ -133,7 +143,7 @@ export function initGraphApp(angularModule, deps) {
resolve: {
savedWorkspace: function($rootScope, $route, $location) {
return $route.current.params.id
? savedWorkspaceLoader.get($route.current.params.id).catch(function(e) {
? getSavedWorkspace(savedObjectsClient, $route.current.params.id).catch(function(e) {
toastNotifications.addError(e, {
title: i18n.translate('xpack.graph.missingWorkspaceErrorMessage', {
defaultMessage: "Couldn't load graph with ID",
Expand All @@ -146,7 +156,7 @@ export function initGraphApp(angularModule, deps) {
// return promise that never returns to prevent the controller from loading
return new Promise();
})
: savedWorkspaceLoader.get();
: getSavedWorkspace(savedObjectsClient);
},
indexPatterns: function() {
return savedObjectsClient
Expand Down Expand Up @@ -283,6 +293,8 @@ export function initGraphApp(angularModule, deps) {
},
notifications: coreStart.notifications,
http: coreStart.http,
overlays: coreStart.overlays,
savedObjectsClient,
showSaveModal,
setWorkspaceInitialized: () => {
$scope.workspaceInitialized = true;
Expand Down
11 changes: 1 addition & 10 deletions x-pack/plugins/graph/public/application.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import { Plugin as DataPlugin, IndexPatternsContract } from '../../../../src/plu
import { LicensingPluginSetup } from '../../licensing/public';
import { checkLicense } from '../common/check_license';
import { NavigationPublicPluginStart as NavigationStart } from '../../../../src/plugins/navigation/public';
import { createSavedWorkspacesLoader } from './services/persistence/saved_workspace_loader';
import { Storage } from '../../../../src/plugins/kibana_utils/public';
import {
addAppRedirectMessageToUrl,
Expand Down Expand Up @@ -87,15 +86,7 @@ export const renderApp = ({ appBasePath, element, ...deps }: GraphDependencies)
}
});

const savedWorkspaceLoader = createSavedWorkspacesLoader({
chrome: deps.coreStart.chrome,
indexPatterns: deps.data.indexPatterns,
overlays: deps.coreStart.overlays,
savedObjectsClient: deps.coreStart.savedObjects.client,
basePath: deps.coreStart.http.basePath,
});

initGraphApp(graphAngularModule, { ...deps, savedWorkspaceLoader });
initGraphApp(graphAngularModule, deps);
const $injector = mountGraphApp(appBasePath, element);
return () => {
licenseSubscription.unsubscribe();
Expand Down
Loading

0 comments on commit 78e0bdf

Please sign in to comment.