maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[Lens] Warn if leaving with unsaved visualization (#67689)

Browse source 
* [Lens] Warn if leaving with unsaved visualization

* Made confirmation logic more robust and add title

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Wylie Conlon 2020-06-01 15:05:27 -04:00 committed by GitHub
parent add5b11611
commit b061d85f9a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 208 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

216
x-pack/plugins/lens/public/app_plugin/app.test.tsx
View file

@ -9,6 +9,7 @@ import { ReactWrapper } from 'enzyme';
import { act } from 'react-dom/test-utils';
import { App } from './app';
import { EditorFrameInstance } from '../types';
import { AppMountParameters } from 'kibana/public';
import { Storage } from '../../../../../src/plugins/kibana_utils/public';
import { Document, SavedObjectStore } from '../persistence';
import { mount } from 'enzyme';
@ -111,6 +112,7 @@ describe('Lens App', () => {
newlyCreated?: boolean
) => void;
originatingApp: string | undefined;
onAppLeave: AppMountParameters['onAppLeave'];
}> {
return ({
navigation: navigationStartMock,
@ -153,6 +155,7 @@ describe('Lens App', () => {
newlyCreated?: boolean
) => {}
),
onAppLeave: jest.fn(),
} as unknown) as jest.Mocked<{
navigation: typeof navigationStartMock;
editorFrame: EditorFrameInstance;
@ -168,6 +171,7 @@ describe('Lens App', () => {
newlyCreated?: boolean
) => void;
originatingApp: string | undefined;
onAppLeave: AppMountParameters['onAppLeave'];
}>;
}
@ -357,22 +361,7 @@ describe('Lens App', () => {
newTitle: string;
}
let defaultArgs: jest.Mocked<{
editorFrame: EditorFrameInstance;
navigation: typeof navigationStartMock;
data: typeof dataStartMock;
core: typeof core;
storage: Storage;
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (
id?: string,
returnToOrigin?: boolean,
originatingApp?: string | undefined,
newlyCreated?: boolean
) => void;
originatingApp: string | undefined;
}>;
let defaultArgs: ReturnType<typeof makeDefaultArgs>;
beforeEach(() => {
defaultArgs = makeDefaultArgs();
@ -486,30 +475,6 @@ describe('Lens App', () => {
expect(getButton(instance).disableButton).toEqual(true);
});
it('shows a disabled save button when there are no changes to the document', async () => {
const args = defaultArgs;
(args.docStorage.load as jest.Mock).mockResolvedValue({
id: '1234',
title: 'My cool doc',
expression: '',
} as jest.ResolvedValue<Document>);
args.editorFrame = frame;
instance = mount(<App {...args} />);
expect(getButton(instance).disableButton).toEqual(true);
const onChange = frame.mount.mock.calls[0][1].onChange;
act(() => {
onChange({
filterableIndexPatterns: [],
doc: ({ id: '1234', expression: 'valid expression' } as unknown) as Document,
});
});
instance.update();
expect(getButton(instance).disableButton).toEqual(false);
});
it('shows a save button that is enabled when the frame has provided its state', async () => {
const args = defaultArgs;
args.editorFrame = frame;
@ -691,21 +656,7 @@ describe('Lens App', () => {
});
describe('query bar state management', () => {
let defaultArgs: jest.Mocked<{
editorFrame: EditorFrameInstance;
data: typeof dataStartMock;
navigation: typeof navigationStartMock;
core: typeof core;
storage: Storage;
docId?: string;
docStorage: SavedObjectStore;
redirectTo: (
id?: string,
returnToOrigin?: boolean,
originatingApp?: string | undefined,
newlyCreated?: boolean
) => void;
}>;
let defaultArgs: ReturnType<typeof makeDefaultArgs>;
beforeEach(() => {
defaultArgs = makeDefaultArgs();
@ -1001,4 +952,159 @@ describe('Lens App', () => {
expect(args.core.notifications.toasts.addDanger).toHaveBeenCalled();
});
describe('showing a confirm message when leaving', () => {
let defaultArgs: ReturnType<typeof makeDefaultArgs>;
let defaultLeave: jest.Mock;
let confirmLeave: jest.Mock;
beforeEach(() => {
defaultArgs = makeDefaultArgs();
defaultLeave = jest.fn();
confirmLeave = jest.fn();
(defaultArgs.docStorage.load as jest.Mock).mockResolvedValue({
id: '1234',
title: 'My cool doc',
expression: 'valid expression',
state: {
query: 'kuery',
datasourceMetaData: { filterableIndexPatterns: [{ id: '1', title: 'saved' }] },
},
} as jest.ResolvedValue<Document>);
});
it('should not show a confirm message if there is no expression to save', () => {
instance = mount(<App {...defaultArgs} />);
const lastCall =
defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(defaultLeave).toHaveBeenCalled();
expect(confirmLeave).not.toHaveBeenCalled();
});
it('does not confirm if the user is missing save permissions', () => {
const args = defaultArgs;
args.core.application = {
...args.core.application,
capabilities: {
...args.core.application.capabilities,
visualize: { save: false, saveQuery: false, show: true },
},
};
args.editorFrame = frame;
instance = mount(<App {...args} />);
const onChange = frame.mount.mock.calls[0][1].onChange;
act(() =>
onChange({
filterableIndexPatterns: [],
doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document,
})
);
instance.update();
const lastCall =
defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(defaultLeave).toHaveBeenCalled();
expect(confirmLeave).not.toHaveBeenCalled();
});
it('should confirm when leaving with an unsaved doc', () => {
defaultArgs.editorFrame = frame;
instance = mount(<App {...defaultArgs} />);
const onChange = frame.mount.mock.calls[0][1].onChange;
act(() =>
onChange({
filterableIndexPatterns: [],
doc: ({ id: undefined, expression: 'valid expression' } as unknown) as Document,
})
);
instance.update();
const lastCall =
defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(confirmLeave).toHaveBeenCalled();
expect(defaultLeave).not.toHaveBeenCalled();
});
it('should confirm when leaving with unsaved changes to an existing doc', async () => {
defaultArgs.editorFrame = frame;
instance = mount(<App {...defaultArgs} />);
await act(async () => {
instance.setProps({ docId: '1234' });
});
const onChange = frame.mount.mock.calls[0][1].onChange;
act(() =>
onChange({
filterableIndexPatterns: [],
doc: ({ id: '1234', expression: 'different expression' } as unknown) as Document,
})
);
instance.update();
const lastCall =
defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(confirmLeave).toHaveBeenCalled();
expect(defaultLeave).not.toHaveBeenCalled();
});
it('should not confirm when changes are saved', async () => {
defaultArgs.editorFrame = frame;
instance = mount(<App {...defaultArgs} />);
await act(async () => {
instance.setProps({ docId: '1234' });
});
const onChange = frame.mount.mock.calls[0][1].onChange;
act(() =>
onChange({
filterableIndexPatterns: [],
doc: ({ id: '1234', expression: 'valid expression' } as unknown) as Document,
})
);
instance.update();
const lastCall =
defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(defaultLeave).toHaveBeenCalled();
expect(confirmLeave).not.toHaveBeenCalled();
});
it('should confirm when the latest doc is invalid', async () => {
defaultArgs.editorFrame = frame;
instance = mount(<App {...defaultArgs} />);
await act(async () => {
instance.setProps({ docId: '1234' });
});
const onChange = frame.mount.mock.calls[0][1].onChange;
act(() =>
onChange({
filterableIndexPatterns: [],
doc: ({ id: '1234', expression: null } as unknown) as Document,
})
);
instance.update();
const lastCall =
defaultArgs.onAppLeave.mock.calls[defaultArgs.onAppLeave.mock.calls.length - 1][0];
lastCall({ default: defaultLeave, confirm: confirmLeave });
expect(confirmLeave).toHaveBeenCalled();
expect(defaultLeave).not.toHaveBeenCalled();
});
});
});

57
x-pack/plugins/lens/public/app_plugin/app.tsx
View file

@ -10,7 +10,7 @@ import { I18nProvider } from '@kbn/i18n/react';
import { i18n } from '@kbn/i18n';
import { Query, DataPublicPluginStart } from 'src/plugins/data/public';
import { NavigationPublicPluginStart } from 'src/plugins/navigation/public';
import { AppMountContext, NotificationsStart } from 'kibana/public';
import { AppMountContext, AppMountParameters, NotificationsStart } from 'kibana/public';
import { IStorageWrapper } from 'src/plugins/kibana_utils/public';
import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/public';
import {
@ -57,6 +57,7 @@ export function App({
redirectTo,
originatingAppFromUrl,
navigation,
onAppLeave,
}: {
editorFrame: EditorFrameInstance;
data: DataPublicPluginStart;
@ -72,6 +73,7 @@ export function App({
newlyCreated?: boolean
) => void;
originatingAppFromUrl?: string | undefined;
onAppLeave: AppMountParameters['onAppLeave'];
}) {
const language =
storage.get('kibana.userQueryLanguage') || core.uiSettings.get('search:queryLanguage');
@ -94,6 +96,12 @@ export function App({
const { lastKnownDoc } = state;
const isSaveable =
lastKnownDoc &&
lastKnownDoc.expression &&
lastKnownDoc.expression.length > 0 &&
core.application.capabilities.visualize.save;
useEffect(() => {
// Clear app-specific filters when navigating to Lens. Necessary because Lens
// can be loaded without a full page refresh
@ -123,7 +131,31 @@ export function App({
filterSubscription.unsubscribe();
timeSubscription.unsubscribe();
};
}, []);
}, [data.query.filterManager, data.query.timefilter.timefilter]);
useEffect(() => {
onAppLeave((actions) => {
// Confirm when the user has made any changes to an existing doc
// or when the user has configured something without saving
if (
core.application.capabilities.visualize.save &&
(state.persistedDoc?.expression
? !_.isEqual(lastKnownDoc?.expression, state.persistedDoc.expression)
: lastKnownDoc?.expression)
) {
return actions.confirm(
i18n.translate('xpack.lens.app.unsavedWorkMessage', {
defaultMessage: 'Leave Lens with unsaved work?',
}),
i18n.translate('xpack.lens.app.unsavedWorkTitle', {
defaultMessage: 'Unsaved changes',
})
);
} else {
return actions.default();
}
});
}, [lastKnownDoc, onAppLeave, state.persistedDoc, core.application.capabilities.visualize.save]);
// Sync Kibana breadcrumbs any time the saved document's title changes
useEffect(() => {
@ -144,7 +176,7 @@ export function App({
: i18n.translate('xpack.lens.breadcrumbsCreate', { defaultMessage: 'Create' }),
},
]);
}, [state.persistedDoc && state.persistedDoc.title]);
}, [core.application, core.chrome, core.http.basePath, state.persistedDoc]);
useEffect(() => {
if (docId && (!state.persistedDoc || state.persistedDoc.id !== docId)) {
@ -187,13 +219,16 @@ export function App({
redirectTo();
});
}
}, [docId]);
const isSaveable =
lastKnownDoc &&
lastKnownDoc.expression &&
lastKnownDoc.expression.length > 0 &&
core.application.capabilities.visualize.save;
}, [
core.notifications,
data.indexPatterns,
data.query.filterManager,
docId,
// TODO: These dependencies are changing too often
// docStorage,
// redirectTo,
// state.persistedDoc,
]);
const runSave = (
saveProps: Omit<OnSaveProps, 'onTitleDuplicate' | 'newDescription'> & {
@ -257,7 +292,7 @@ export function App({
core.notifications.toasts.addDanger({
title: e.message,
}),
[]
[core.notifications.toasts]
);
const { TopNavMenu } = navigation.ui;

1
x-pack/plugins/lens/public/app_plugin/mounter.tsx
View file

@ -92,6 +92,7 @@ export async function mountApp(
redirectTo(routeProps, id, returnToOrigin, originatingApp, newlyCreated)
}
originatingAppFromUrl={originatingAppFromUrl}
onAppLeave={params.onAppLeave}
/>
);
};