Skip to content

Commit

Permalink
[Fix] Lose OriginatingApp Connection on Save As (#72725) (#73090)
Browse files Browse the repository at this point in the history
Reset originatingApp in lens and visualize when return to dashboard on saving is false
  • Loading branch information
ThomThomson authored Jul 23, 2020
1 parent 20a8415 commit 605046a
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export const VisualizeEditor = () => {
isEmbeddableRendered={isEmbeddableRendered}
hasUnappliedChanges={hasUnappliedChanges}
originatingApp={originatingApp}
setOriginatingApp={setOriginatingApp}
savedVisInstance={savedVisInstance}
stateContainer={appState}
visualizationIdFromUrl={visualizationIdFromUrl}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ interface VisualizeTopNavProps {
setHasUnsavedChanges: (value: boolean) => void;
hasUnappliedChanges: boolean;
originatingApp?: string;
setOriginatingApp?: (originatingApp: string | undefined) => void;
savedVisInstance: SavedVisInstance;
stateContainer: VisualizeAppStateContainer;
visualizationIdFromUrl?: string;
Expand All @@ -53,6 +54,7 @@ const TopNav = ({
setHasUnsavedChanges,
hasUnappliedChanges,
originatingApp,
setOriginatingApp,
savedVisInstance,
stateContainer,
visualizationIdFromUrl,
Expand Down Expand Up @@ -86,6 +88,7 @@ const TopNav = ({
hasUnappliedChanges,
openInspector,
originatingApp,
setOriginatingApp,
savedVisInstance,
stateContainer,
visualizationIdFromUrl,
Expand All @@ -100,6 +103,7 @@ const TopNav = ({
hasUnappliedChanges,
openInspector,
originatingApp,
setOriginatingApp,
savedVisInstance,
stateContainer,
visualizationIdFromUrl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ interface TopNavConfigParams {
setHasUnsavedChanges: (value: boolean) => void;
openInspector: () => void;
originatingApp?: string;
setOriginatingApp?: (originatingApp: string | undefined) => void;
hasUnappliedChanges: boolean;
savedVisInstance: SavedVisInstance;
stateContainer: VisualizeAppStateContainer;
Expand All @@ -51,6 +52,7 @@ export const getTopNavConfig = (
setHasUnsavedChanges,
openInspector,
originatingApp,
setOriginatingApp,
hasUnappliedChanges,
savedVisInstance: { embeddableHandler, savedVis, vis },
stateContainer,
Expand Down Expand Up @@ -112,6 +114,9 @@ export const getTopNavConfig = (
application.navigateToApp(originatingApp);
}
} else {
if (setOriginatingApp && originatingApp && savedVis.copyOnSave) {
setOriginatingApp(undefined);
}
chrome.docTitle.change(savedVis.lastSavedTitle);
chrome.setBreadcrumbs(getEditBreadcrumbs(savedVis.lastSavedTitle));

Expand Down
12 changes: 12 additions & 0 deletions test/functional/apps/dashboard/edit_embeddable_redirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,17 @@ export default function ({ getService, getPageObjects }) {
const titles = await PageObjects.dashboard.getPanelTitles();
expect(titles.indexOf(newTitle)).to.not.be(-1);
});

it('loses originatingApp connection after save as when redirectToOrigin is false', async () => {
const newTitle = 'wowee, my title just got cooler again';
await PageObjects.header.waitUntilLoadingHasFinished();
await dashboardPanelActions.openContextMenu();
await dashboardPanelActions.clickEdit();
await PageObjects.visualize.saveVisualizationExpectSuccess(newTitle, {
saveAsNew: true,
redirectToOrigin: false,
});
await PageObjects.visualize.notLinkedToOriginatingApp();
});
});
}
10 changes: 10 additions & 0 deletions test/functional/page_objects/visualize_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,16 @@ export function VisualizePageProvider({ getService, getPageObjects }: FtrProvide
await testSubjects.existOrFail('visualizesaveAndReturnButton');
await testSubjects.click('visualizesaveAndReturnButton');
}

public async linkedToOriginatingApp() {
await header.waitUntilLoadingHasFinished();
await testSubjects.existOrFail('visualizesaveAndReturnButton');
}

public async notLinkedToOriginatingApp() {
await header.waitUntilLoadingHasFinished();
await testSubjects.missingOrFail('visualizesaveAndReturnButton');
}
}

return new VisualizePage();
Expand Down
15 changes: 11 additions & 4 deletions x-pack/plugins/lens/public/app_plugin/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface State {
isLoading: boolean;
isSaveModalVisible: boolean;
indexPatternsForTopNav: IndexPatternInstance[];
originatingApp?: string;
persistedDoc?: Document;
lastKnownDoc?: Document;

Expand Down Expand Up @@ -97,6 +98,7 @@ export function App({
fromDate: currentRange.from,
toDate: currentRange.to,
},
originatingApp,
filters: [],
indicateNoData: false,
};
Expand Down Expand Up @@ -321,9 +323,14 @@ export function App({
.then(({ id }) => {
// Prevents unnecessary network request and disables save button
const newDoc = { ...doc, id };
const currentOriginatingApp = state.originatingApp;
setState((s) => ({
...s,
isSaveModalVisible: false,
originatingApp:
saveProps.newCopyOnSave && !saveProps.returnToOrigin
? undefined
: currentOriginatingApp,
persistedDoc: newDoc,
lastKnownDoc: newDoc,
}));
Expand Down Expand Up @@ -368,7 +375,7 @@ export function App({
<div className="lnsApp__header">
<TopNavMenu
config={[
...(!!originatingApp && lastKnownDoc?.id
...(!!state.originatingApp && lastKnownDoc?.id
? [
{
label: i18n.translate('xpack.lens.app.saveAndReturn', {
Expand All @@ -393,14 +400,14 @@ export function App({
: []),
{
label:
lastKnownDoc?.id && !!originatingApp
lastKnownDoc?.id && !!state.originatingApp
? i18n.translate('xpack.lens.app.saveAs', {
defaultMessage: 'Save as',
})
: i18n.translate('xpack.lens.app.save', {
defaultMessage: 'Save',
}),
emphasize: !originatingApp || !lastKnownDoc?.id,
emphasize: !state.originatingApp || !lastKnownDoc?.id,
run: () => {
if (isSaveable && lastKnownDoc) {
setState((s) => ({ ...s, isSaveModalVisible: true }));
Expand Down Expand Up @@ -523,7 +530,7 @@ export function App({
</div>
{lastKnownDoc && state.isSaveModalVisible && (
<SavedObjectSaveModalOrigin
originatingApp={originatingApp}
originatingApp={state.originatingApp}
onSave={(props) => runSave(props)}
onClose={() => setState((s) => ({ ...s, isSaveModalVisible: false }))}
documentInfo={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,15 @@ export default function ({ getPageObjects, getService }) {
const titles = await PageObjects.dashboard.getPanelTitles();
expect(titles.indexOf(newTitle)).to.not.be(-1);
});

it('loses originatingApp connection after save as when redirectToOrigin is false', async () => {
const newTitle = 'wowee, my title just got cooler again';
await PageObjects.dashboard.waitForRenderComplete();
await dashboardPanelActions.openContextMenu();
await dashboardPanelActions.clickEdit();
await PageObjects.lens.save(newTitle, true, false);
await PageObjects.lens.notLinkedToOriginatingApp();
await PageObjects.common.navigateToApp('dashboard');
});
});
}
10 changes: 10 additions & 0 deletions x-pack/test/functional/page_objects/lens_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,5 +195,15 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont
async createLayer() {
await testSubjects.click('lnsLayerAddButton');
},

async linkedToOriginatingApp() {
await PageObjects.header.waitUntilLoadingHasFinished();
await testSubjects.existOrFail('lnsApp_saveAndReturnButton');
},

async notLinkedToOriginatingApp() {
await PageObjects.header.waitUntilLoadingHasFinished();
await testSubjects.missingOrFail('lnsApp_saveAndReturnButton');
},
});
}

0 comments on commit 605046a

Please sign in to comment.