Skip to content

Commit

Permalink
[Lens] Prevent overwriting managed content from editor (#175062)
Browse files Browse the repository at this point in the history
## Summary

Close #166720

I marked this a breaking change since it is preventing users from doing
something they have been able to do before. They can no longer save
changes to managed Lens visualizations. Instead, they have to save
changes to a new visualization.

To test, import this `ndjson` file which includes both a managed and an
unmanaged visualization:

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
  • Loading branch information
3 people authored Jan 24, 2024
1 parent 274c314 commit 1d4b7df
Show file tree
Hide file tree
Showing 41 changed files with 333 additions and 47 deletions.
1 change: 1 addition & 0 deletions .buildkite/ftr_configs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ enabled:
- x-pack/test/functional/apps/lens/open_in_lens/dashboard/config.ts
- x-pack/test/functional/apps/license_management/config.ts
- x-pack/test/functional/apps/logstash/config.ts
- x-pack/test/functional/apps/managed_content/config.ts
- x-pack/test/functional/apps/management/config.ts
- x-pack/test/functional/apps/maps/group1/config.ts
- x-pack/test/functional/apps/maps/group2/config.ts
Expand Down
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ packages/kbn-logging @elastic/kibana-core
packages/kbn-logging-mocks @elastic/kibana-core
x-pack/plugins/logs_shared @elastic/obs-ux-logs-team
x-pack/plugins/logstash @elastic/logstash
packages/kbn-managed-content-badge @elastic/kibana-visualizations
packages/kbn-managed-vscode-config @elastic/kibana-operations
packages/kbn-managed-vscode-config-cli @elastic/kibana-operations
packages/kbn-management/cards_navigation @elastic/platform-deployment-management
Expand Down
3 changes: 2 additions & 1 deletion .i18nrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@
"unifiedFieldList": "packages/kbn-unified-field-list",
"unifiedHistogram": "src/plugins/unified_histogram",
"unifiedDataTable": "packages/kbn-unified-data-table",
"unsavedChangesBadge": "packages/kbn-unsaved-changes-badge"
"unsavedChangesBadge": "packages/kbn-unsaved-changes-badge",
"managedContentBadge": "packages/kbn-managed-content-badge"
},
"translations": []
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@
"@kbn/logging-mocks": "link:packages/kbn-logging-mocks",
"@kbn/logs-shared-plugin": "link:x-pack/plugins/logs_shared",
"@kbn/logstash-plugin": "link:x-pack/plugins/logstash",
"@kbn/managed-content-badge": "link:packages/kbn-managed-content-badge",
"@kbn/management-cards-navigation": "link:packages/kbn-management/cards_navigation",
"@kbn/management-plugin": "link:src/plugins/management",
"@kbn/management-settings-application": "link:packages/kbn-management/settings/application",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,13 @@ export class TestSubjects extends FtrService {
}
}

public async isEuiSwitchChecked(selector: string) {
const euiSwitch = await this.find(selector);
public async isEuiSwitchChecked(selector: string | WebElementWrapper) {
let euiSwitch: WebElementWrapper;
if (typeof selector === 'string') {
euiSwitch = await this.find(selector);
} else {
euiSwitch = selector;
}
const isChecked = await euiSwitch.getAttribute('aria-checked');
return isChecked === 'true';
}
Expand Down
3 changes: 3 additions & 0 deletions packages/kbn-managed-content-badge/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# @kbn/managed-content-badge

Empty package generated by @kbn/generate
28 changes: 28 additions & 0 deletions packages/kbn-managed-content-badge/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { i18n } from '@kbn/i18n';
import type { EuiToolTipProps } from '@elastic/eui';
import type { TopNavMenuBadgeProps } from '@kbn/navigation-plugin/public';

export const getManagedContentBadge: (tooltipText: string) => TopNavMenuBadgeProps = (
tooltipText
) => ({
'data-test-subj': 'managedContentBadge',
badgeText: i18n.translate('managedContentBadge.text', {
defaultMessage: 'Managed',
}),
title: i18n.translate('managedContentBadge.text', {
defaultMessage: 'Managed',
}),
color: 'primary',
iconType: 'glasses',
toolTipProps: {
content: tooltipText,
position: 'bottom',
} as EuiToolTipProps,
});
13 changes: 13 additions & 0 deletions packages/kbn-managed-content-badge/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test/jest_node',
rootDir: '../..',
roots: ['<rootDir>/packages/kbn-managed-content-badge'],
};
5 changes: 5 additions & 0 deletions packages/kbn-managed-content-badge/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "shared-browser",
"id": "@kbn/managed-content-badge",
"owner": "@elastic/kibana-visualizations"
}
6 changes: 6 additions & 0 deletions packages/kbn-managed-content-badge/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@kbn/managed-content-badge",
"private": true,
"version": "1.0.0",
"license": "SSPL-1.0 OR Elastic License 2.0"
}
20 changes: 20 additions & 0 deletions packages/kbn-managed-content-badge/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"outDir": "target/types",
"types": [
"jest",
"node"
]
},
"include": [
"**/*.ts",
],
"exclude": [
"target/**/*"
],
"kbn_references": [
"@kbn/i18n",
"@kbn/navigation-plugin",
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ export const dashboardReadonlyBadge = {
};

export const dashboardManagedBadge = {
getText: () =>
i18n.translate('dashboard.badge.managed.text', {
defaultMessage: 'Managed',
}),
getTooltip: () =>
i18n.translate('dashboard.badge.managed.tooltip', {
defaultMessage: 'This dashboard is system managed. Clone this dashboard to make changes.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
LazyLabsFlyout,
getContextProvider as getPresentationUtilContextProvider,
} from '@kbn/presentation-util-plugin/public';
import { getManagedContentBadge } from '@kbn/managed-content-badge';
import { ViewMode } from '@kbn/embeddable-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import { TopNavMenuProps } from '@kbn/navigation-plugin/public';
Expand Down Expand Up @@ -305,17 +306,7 @@ export function InternalDashboardTopNav({
});
}
if (showWriteControls && managed) {
allBadges.push({
'data-test-subj': 'dashboardSaveRecommendedBadge',
badgeText: dashboardManagedBadge.getText(),
title: '',
color: 'primary',
iconType: 'glasses',
toolTipProps: {
content: dashboardManagedBadge.getTooltip(),
position: 'bottom',
} as EuiToolTipProps,
});
allBadges.push(getManagedContentBadge(dashboardManagedBadge.getTooltip()));
}
return allBadges;
}, [hasUnsavedChanges, viewMode, hasRunMigrations, showWriteControls, managed]);
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/dashboard/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@
"@kbn/presentation-containers",
"@kbn/presentation-panel-plugin",
"@kbn/content-management-table-list-view-common",
"@kbn/shared-ux-utility"
"@kbn/shared-ux-utility",
"@kbn/managed-content-badge"
],
"exclude": ["target/**/*"]
}
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ function SavedObjectSaveModalDashboard(props: SaveModalDashboardProps) {
options={isAddToLibrarySelected ? tagOptions : undefined} // Show tags when not adding to dashboard
description={documentInfo.description}
showDescription={true}
mustCopyOnSaveMessage={props.mustCopyOnSaveMessage}
{...{
confirmButtonLabel,
initialCopyOnSave,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export function SaveModalDashboardSelector(props: SaveModalDashboardSelectorProp
content={
<FormattedMessage
id="presentationUtil.saveModalDashboard.dashboardInfoTooltip"
defaultMessage="items added to the Visualize Library are available to all dashboards. Edits to a library item appear everywhere it is used."
defaultMessage="Items added to the Visualize Library are available to all dashboards. Edits to a library item appear everywhere it is used."
/>
}
/>
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/presentation_util/public/components/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ export interface SaveModalDashboardProps {
onClose: () => void;
onSave: (props: OnSaveProps & { dashboardId: string | null; addToLibrary: boolean }) => void;
tagOptions?: React.ReactNode | ((state: SaveModalState) => React.ReactNode);
// include a message if the user has to copy on save
mustCopyOnSaveMessage?: string;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import { shallow } from 'enzyme';
import React from 'react';
import { SavedObjectSaveModal } from './saved_object_save_modal';

import { mountWithIntl } from '@kbn/test-jest-helpers';
import { render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

describe('SavedObjectSaveModal', () => {
it('should render matching snapshot', () => {
Expand Down Expand Up @@ -72,19 +73,46 @@ describe('SavedObjectSaveModal', () => {
});

it('allows specifying custom save button label', () => {
const wrapper = mountWithIntl(
const confirmButtonLabel = 'Save and done';

render(
<SavedObjectSaveModal
onSave={() => void 0}
onClose={() => void 0}
title={'Saved Object title'}
showCopyOnSave={false}
objectType="visualization"
showDescription={true}
confirmButtonLabel="Save and done"
confirmButtonLabel={confirmButtonLabel}
/>
);
expect(wrapper.find('button[data-test-subj="confirmSaveSavedObjectButton"]').text()).toBe(
'Save and done'

expect(screen.queryByText(confirmButtonLabel)).toBeInTheDocument();
});

it('enforces copy on save', async () => {
const onSave = jest.fn();

render(
<SavedObjectSaveModal
onSave={onSave}
onClose={() => void 0}
title={'Saved Object title'}
objectType="visualization"
showDescription={true}
showCopyOnSave={true}
mustCopyOnSaveMessage="You must save a copy of the object."
/>
);

expect(onSave).not.toHaveBeenCalled();

expect(screen.getByTestId('saveAsNewCheckbox')).toBeDisabled();
userEvent.click(screen.getByRole('button', { name: 'Save' }));

await waitFor(() => {
expect(onSave).toHaveBeenCalled();
expect(onSave.mock.calls[0][0].newCopyOnSave).toBe(true);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,13 @@ import {
EuiSwitch,
EuiSwitchEvent,
EuiTextArea,
EuiIconTip,
} from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import React from 'react';
import { EuiText } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { euiThemeVars } from '@kbn/ui-theme';

export interface OnSaveProps {
newTitle: string;
Expand All @@ -44,6 +46,7 @@ interface Props {
onClose: () => void;
title: string;
showCopyOnSave: boolean;
mustCopyOnSaveMessage?: string;
onCopyOnSaveChange?: (copyOnChange: boolean) => void;
initialCopyOnSave?: boolean;
objectType: string;
Expand Down Expand Up @@ -173,7 +176,7 @@ export class SavedObjectSaveModal extends React.Component<Props, SaveModalState>

<EuiModalFooter>
<EuiFlexGroup justifyContent="flexEnd" alignItems="center">
{this.props.showCopyOnSave && <EuiFlexItem grow>{this.renderCopyOnSave()}</EuiFlexItem>}
{this.props.showCopyOnSave && this.renderCopyOnSave()}
<EuiFlexItem grow={false}>
<EuiButtonEmpty data-test-subj="saveCancelButton" onClick={this.props.onClose}>
<FormattedMessage
Expand Down Expand Up @@ -249,7 +252,7 @@ export class SavedObjectSaveModal extends React.Component<Props, SaveModalState>

await this.props.onSave({
newTitle: this.state.title,
newCopyOnSave: this.state.copyOnSave,
newCopyOnSave: Boolean(this.props.mustCopyOnSaveMessage) || this.state.copyOnSave,
isTitleDuplicateConfirmed: this.state.isTitleDuplicateConfirmed,
onTitleDuplicate: this.onTitleDuplicate,
newDescription: this.state.visualizationDescription,
Expand Down Expand Up @@ -355,18 +358,29 @@ export class SavedObjectSaveModal extends React.Component<Props, SaveModalState>

private renderCopyOnSave = () => {
return (
<EuiSwitch
data-test-subj="saveAsNewCheckbox"
checked={this.state.copyOnSave}
onChange={this.onCopyOnSaveChange}
label={
<FormattedMessage
id="savedObjects.saveModal.saveAsNewLabel"
defaultMessage="Save as new {objectType}"
values={{ objectType: this.props.objectType }}
<>
<EuiFlexItem grow={false}>
<EuiSwitch
data-test-subj="saveAsNewCheckbox"
checked={Boolean(this.props.mustCopyOnSaveMessage) || this.state.copyOnSave}
disabled={Boolean(this.props.mustCopyOnSaveMessage)}
onChange={this.onCopyOnSaveChange}
label={
<FormattedMessage
id="savedObjects.saveModal.saveAsNewLabel"
defaultMessage="Save as new {objectType}"
values={{ objectType: this.props.objectType }}
/>
}
/>
}
/>
</EuiFlexItem>
{this.props.mustCopyOnSaveMessage && (
<EuiFlexItem css={{ marginLeft: `-${euiThemeVars.euiSize}` }} grow={false}>
<EuiIconTip type="iInCircle" content={this.props.mustCopyOnSaveMessage} />
</EuiFlexItem>
)}
<EuiFlexItem grow={true} />
</>
);
};
}
2 changes: 1 addition & 1 deletion src/plugins/saved_objects/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
"@kbn/i18n",
"@kbn/data-views-plugin",
"@kbn/i18n-react",
"@kbn/test-jest-helpers",
"@kbn/utility-types",
"@kbn/ui-theme",
],
"exclude": [
"target/**/*",
Expand Down
5 changes: 5 additions & 0 deletions test/functional/fixtures/kbn_archiver/managed_content.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{"attributes":{"allowHidden":false,"fieldAttrs":"{}","fieldFormatMap":"{}","fields":"[]","name":"logstash-*","runtimeFieldMap":"{}","sourceFilters":"[]","timeFieldName":"@timestamp","title":"logstash-*"},"coreMigrationVersion":"8.8.0","created_at":"2024-01-18T17:35:58.606Z","id":"5f863f70-4728-4e8d-b441-db08f8c33b28","managed":false,"references":[],"type":"index-pattern","typeMigrationVersion":"8.0.0","updated_at":"2024-01-18T17:35:58.606Z","version":"WzI4LDFd"}

{"attributes":{"description":"","state":{"adHocDataViews":{},"datasourceStates":{"formBased":{"layers":{"e633b1af-3ab4-4bf5-8faa-fefde06c4a4a":{"columnOrder":["f2555a1a-6f93-43fd-bc63-acdfadd47729","d229daf9-9658-4579-99af-01d8adb2f25f"],"columns":{"d229daf9-9658-4579-99af-01d8adb2f25f":{"dataType":"number","isBucketed":false,"label":"Median of bytes","operationType":"median","params":{"emptyAsNull":true},"scale":"ratio","sourceField":"bytes"},"f2555a1a-6f93-43fd-bc63-acdfadd47729":{"dataType":"date","isBucketed":true,"label":"@timestamp","operationType":"date_histogram","params":{"dropPartials":false,"includeEmptyRows":true,"interval":"auto"},"scale":"interval","sourceField":"@timestamp"}},"incompleteColumns":{},"sampling":1}}},"indexpattern":{"layers":{}},"textBased":{"layers":{}}},"filters":[],"internalReferences":[],"query":{"language":"kuery","query":""},"visualization":{"axisTitlesVisibilitySettings":{"x":true,"yLeft":true,"yRight":true},"fittingFunction":"None","gridlinesVisibilitySettings":{"x":true,"yLeft":true,"yRight":true},"labelsOrientation":{"x":0,"yLeft":0,"yRight":0},"layers":[{"accessors":["d229daf9-9658-4579-99af-01d8adb2f25f"],"layerId":"e633b1af-3ab4-4bf5-8faa-fefde06c4a4a","layerType":"data","position":"top","seriesType":"bar_stacked","showGridlines":false,"xAccessor":"f2555a1a-6f93-43fd-bc63-acdfadd47729"}],"legend":{"isVisible":true,"position":"right"},"preferredSeriesType":"bar_stacked","tickLabelsVisibilitySettings":{"x":true,"yLeft":true,"yRight":true},"valueLabels":"hide"}},"title":"Lens vis (managed)","visualizationType":"lnsXY"},"coreMigrationVersion":"8.8.0","created_at":"2024-01-18T17:42:12.920Z","id":"managed-36db-4a3b-a4ba-7a64ab8f130b","managed":true,"references":[{"id":"5f863f70-4728-4e8d-b441-db08f8c33b28","name":"indexpattern-datasource-layer-e633b1af-3ab4-4bf5-8faa-fefde06c4a4a","type":"index-pattern"}],"type":"lens","typeMigrationVersion":"8.9.0","updated_at":"2024-01-18T17:42:12.920Z","version":"WzQ1LDFd"}

{"attributes":{"description":"","state":{"adHocDataViews":{},"datasourceStates":{"formBased":{"layers":{"e633b1af-3ab4-4bf5-8faa-fefde06c4a4a":{"columnOrder":["f2555a1a-6f93-43fd-bc63-acdfadd47729","d229daf9-9658-4579-99af-01d8adb2f25f"],"columns":{"d229daf9-9658-4579-99af-01d8adb2f25f":{"dataType":"number","isBucketed":false,"label":"Median of bytes","operationType":"median","params":{"emptyAsNull":true},"scale":"ratio","sourceField":"bytes"},"f2555a1a-6f93-43fd-bc63-acdfadd47729":{"dataType":"date","isBucketed":true,"label":"@timestamp","operationType":"date_histogram","params":{"dropPartials":false,"includeEmptyRows":true,"interval":"auto"},"scale":"interval","sourceField":"@timestamp"}},"incompleteColumns":{},"sampling":1}}},"indexpattern":{"layers":{}},"textBased":{"layers":{}}},"filters":[],"internalReferences":[],"query":{"language":"kuery","query":""},"visualization":{"axisTitlesVisibilitySettings":{"x":true,"yLeft":true,"yRight":true},"fittingFunction":"None","gridlinesVisibilitySettings":{"x":true,"yLeft":true,"yRight":true},"labelsOrientation":{"x":0,"yLeft":0,"yRight":0},"layers":[{"accessors":["d229daf9-9658-4579-99af-01d8adb2f25f"],"layerId":"e633b1af-3ab4-4bf5-8faa-fefde06c4a4a","layerType":"data","position":"top","seriesType":"bar_stacked","showGridlines":false,"xAccessor":"f2555a1a-6f93-43fd-bc63-acdfadd47729"}],"legend":{"isVisible":true,"position":"right"},"preferredSeriesType":"bar_stacked","tickLabelsVisibilitySettings":{"x":true,"yLeft":true,"yRight":true},"valueLabels":"hide"}},"title":"Lens vis (unmanaged)","visualizationType":"lnsXY"},"coreMigrationVersion":"8.8.0","created_at":"2024-01-18T17:42:12.920Z","id":"unmanaged-36db-4a3b-a4ba-7a64ab8f130b","managed":false,"references":[{"id":"5f863f70-4728-4e8d-b441-db08f8c33b28","name":"indexpattern-datasource-layer-e633b1af-3ab4-4bf5-8faa-fefde06c4a4a","type":"index-pattern"}],"type":"lens","typeMigrationVersion":"8.9.0","updated_at":"2024-01-18T17:42:12.920Z","version":"WzQ1LDFd"}
2 changes: 2 additions & 0 deletions tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,8 @@
"@kbn/logs-shared-plugin/*": ["x-pack/plugins/logs_shared/*"],
"@kbn/logstash-plugin": ["x-pack/plugins/logstash"],
"@kbn/logstash-plugin/*": ["x-pack/plugins/logstash/*"],
"@kbn/managed-content-badge": ["packages/kbn-managed-content-badge"],
"@kbn/managed-content-badge/*": ["packages/kbn-managed-content-badge/*"],
"@kbn/managed-vscode-config": ["packages/kbn-managed-vscode-config"],
"@kbn/managed-vscode-config/*": ["packages/kbn-managed-vscode-config/*"],
"@kbn/managed-vscode-config-cli": ["packages/kbn-managed-vscode-config-cli"],
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/lens/public/app_plugin/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
updateIndexPatterns,
selectActiveDatasourceId,
selectFramePublicAPI,
selectIsManaged,
} from '../state_management';
import { SaveModalContainer, runSaveLensVisualization } from './save_modal_container';
import { LensInspector } from '../lens_inspector_service';
Expand Down Expand Up @@ -509,6 +510,8 @@ export function App({
[locator, shortUrls]
);

const isManaged = useLensSelector(selectIsManaged);

const returnToOriginSwitchLabelForContext =
initialContext &&
'isEmbeddable' in initialContext &&
Expand Down Expand Up @@ -614,6 +617,7 @@ export function App({
initialInput={initialInput}
redirectTo={redirectTo}
redirectToOrigin={redirectToOrigin}
managed={isManaged}
initialContext={initialContext}
returnToOriginSwitchLabel={
returnToOriginSwitchLabelForContext ??
Expand Down
Loading

0 comments on commit 1d4b7df

Please sign in to comment.