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

[Fix] Lose OriginatingApp Connection on Save As #72725

Merged
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');
},
});
}