Skip to content

Commit

Permalink
[8.11] Fixes issue with removing spaces from related objects in share…
Browse files Browse the repository at this point in the history
… to space action (#169177) (#169566)

# Backport

This will backport the following commits from `main` to `8.11`:
- [Fixes issue with removing spaces from related objects in share to
space action (#169177)](#169177)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jeramy
Soucy","email":"jeramy.soucy@elastic.co"},"sourceCommit":{"committedDate":"2023-10-23T18:45:36Z","message":"Fixes
issue with removing spaces from related objects in share to space action
(#169177)\n\ncloses #168657\r\n\r\n## Summary\r\n\r\nUpdates the `share
to spaces` action to refrain from removing spaces\r\nfrom related
objects (objects referenced by the target object).\r\n\r\nI have also
updated the description of issue #130562, which essentially\r\nwill
replace much of the client-side implementation of this action,
to\r\nexplicitly include this behavior.\r\n\r\n### Manual Testing\r\n-
Create 2 spaces: A and B\r\n- Add a sample data set (e.g. flight) to
space A\r\n- In Discover, create a saved query called \"s1\" (add a
filter that uses\r\nthe sample data logs data view, and use the filter
menu button)\r\n- Go to `Stack Management->Saved` Objects and share the
\"s1\" query to\r\nspace B\r\n- Verify that the related data view is
also shared to space B.\r\n- Un-share the \"s1\" query from space B\r\n-
Verify that the related data view is still shared to space B.\r\n\r\n###
Automated
Tests\r\n-\r\nx-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.test.tsx\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4af91754e02e1f055d4912b298fd0f7d68740b74","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Security","release_note:skip","backport:prev-minor","v8.11.0","v8.12.0"],"number":169177,"url":"https://github.com/elastic/kibana/pull/169177","mergeCommit":{"message":"Fixes
issue with removing spaces from related objects in share to space action
(#169177)\n\ncloses #168657\r\n\r\n## Summary\r\n\r\nUpdates the `share
to spaces` action to refrain from removing spaces\r\nfrom related
objects (objects referenced by the target object).\r\n\r\nI have also
updated the description of issue #130562, which essentially\r\nwill
replace much of the client-side implementation of this action,
to\r\nexplicitly include this behavior.\r\n\r\n### Manual Testing\r\n-
Create 2 spaces: A and B\r\n- Add a sample data set (e.g. flight) to
space A\r\n- In Discover, create a saved query called \"s1\" (add a
filter that uses\r\nthe sample data logs data view, and use the filter
menu button)\r\n- Go to `Stack Management->Saved` Objects and share the
\"s1\" query to\r\nspace B\r\n- Verify that the related data view is
also shared to space B.\r\n- Un-share the \"s1\" query from space B\r\n-
Verify that the related data view is still shared to space B.\r\n\r\n###
Automated
Tests\r\n-\r\nx-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.test.tsx\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4af91754e02e1f055d4912b298fd0f7d68740b74"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/169177","number":169177,"mergeCommit":{"message":"Fixes
issue with removing spaces from related objects in share to space action
(#169177)\n\ncloses #168657\r\n\r\n## Summary\r\n\r\nUpdates the `share
to spaces` action to refrain from removing spaces\r\nfrom related
objects (objects referenced by the target object).\r\n\r\nI have also
updated the description of issue #130562, which essentially\r\nwill
replace much of the client-side implementation of this action,
to\r\nexplicitly include this behavior.\r\n\r\n### Manual Testing\r\n-
Create 2 spaces: A and B\r\n- Add a sample data set (e.g. flight) to
space A\r\n- In Discover, create a saved query called \"s1\" (add a
filter that uses\r\nthe sample data logs data view, and use the filter
menu button)\r\n- Go to `Stack Management->Saved` Objects and share the
\"s1\" query to\r\nspace B\r\n- Verify that the related data view is
also shared to space B.\r\n- Un-share the \"s1\" query from space B\r\n-
Verify that the related data view is still shared to space B.\r\n\r\n###
Automated
Tests\r\n-\r\nx-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.test.tsx\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4af91754e02e1f055d4912b298fd0f7d68740b74"}}]}]
BACKPORT-->

Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co>
  • Loading branch information
kibanamachine and jeramysoucy authored Oct 23, 2023
1 parent 412f0da commit 379d9eb
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const SelectableSpacesControl = (props: Props) => {
content={
<FormattedMessage
id="xpack.spaces.management.copyToSpace.selectSpacesControl.disabledTooltip"
defaultMessage="The object already exists in this space."
defaultMessage="The object or a related object already exists in this space."
/>
}
position="left"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const APPEND_PROHIBITED = (
defaultMessage: 'Cannot share to this space',
})}
content={i18n.translate('xpack.spaces.shareToSpace.prohibitedSpaceTooltip', {
defaultMessage: 'A copy of this saved object exists in this space.',
defaultMessage: 'A copy of this saved object or a related object exists in this space.',
})}
position="left"
type="iInCircle"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ const ALL_SPACES_PROHIBITED_TOOLTIP = (
)}
content={i18n.translate(
'xpack.spaces.shareToSpace.shareModeControl.shareToAllSpaces.allSpacesProhibitedTooltipContent',
{ defaultMessage: 'A copy of this saved object exists in at least one other space.' }
{
defaultMessage:
'A copy of this saved object or a related object exists in at least one other space.',
}
)}
position="left"
type="iInCircle"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,110 @@ describe('ShareToSpaceFlyout', () => {
expect(onClose).toHaveBeenCalledTimes(1);
});

describe('handles related objects correctly', () => {
const relatedObject = {
type: 'index-pattern',
id: 'd3d7af60-4c81-11e8-b3d7-01146121b73d',
spaces: ['my-active-space', 'space-1'],
inboundReferences: [
{
type: 'dashboard',
id: 'my-dash',
name: 'foo',
},
],
};

it('adds spaces to related objects when only adding spaces', async () => {
const { wrapper, onClose, mockSpacesManager, mockToastNotifications, savedObjectToShare } =
await setup({
additionalShareableReferences: [relatedObject],
});

expect(wrapper.find(ShareToSpaceForm)).toHaveLength(1);
expect(wrapper.find(EuiLoadingSpinner)).toHaveLength(0);
expect(wrapper.find(NoSpacesAvailable)).toHaveLength(0);

changeSpaceSelection(wrapper, ['space-1', 'space-2']);
await clickButton(wrapper, 'save');

const expectedObjects: Array<{ type: string; id: string }> = [
savedObjectToShare,
relatedObject,
].map(({ type, id }) => ({
type,
id,
}));
expect(mockSpacesManager.updateSavedObjectsSpaces).toBeCalledTimes(1);
expect(mockSpacesManager.updateSavedObjectsSpaces).toHaveBeenCalledWith(
expectedObjects,
['space-2'],
[]
);

expect(mockToastNotifications.addSuccess).toHaveBeenCalledTimes(1);
expect(mockToastNotifications.addError).not.toHaveBeenCalled();
expect(onClose).toHaveBeenCalledTimes(1);
});

it('does not remove spaces from related objects when only removing spaces', async () => {
const { wrapper, onClose, mockSpacesManager, mockToastNotifications, savedObjectToShare } =
await setup({
additionalShareableReferences: [relatedObject],
});

expect(wrapper.find(ShareToSpaceForm)).toHaveLength(1);
expect(wrapper.find(EuiLoadingSpinner)).toHaveLength(0);
expect(wrapper.find(NoSpacesAvailable)).toHaveLength(0);

changeSpaceSelection(wrapper, []);
await clickButton(wrapper, 'save');

expect(mockSpacesManager.updateSavedObjectsSpaces).toBeCalledTimes(1);
expect(mockSpacesManager.updateSavedObjectsSpaces).toHaveBeenCalledWith(
[{ type: savedObjectToShare.type, id: savedObjectToShare.id }],
[],
['space-1']
);

expect(mockToastNotifications.addSuccess).toHaveBeenCalledTimes(1);
expect(mockToastNotifications.addError).not.toHaveBeenCalled();
expect(onClose).toHaveBeenCalledTimes(1);
});

it('adds spaces but does not remove spaces from related objects when adding and removing spaces', async () => {
const { wrapper, onClose, mockSpacesManager, mockToastNotifications, savedObjectToShare } =
await setup({
additionalShareableReferences: [relatedObject],
});

expect(wrapper.find(ShareToSpaceForm)).toHaveLength(1);
expect(wrapper.find(EuiLoadingSpinner)).toHaveLength(0);
expect(wrapper.find(NoSpacesAvailable)).toHaveLength(0);

changeSpaceSelection(wrapper, ['space-2', 'space-3']);
await clickButton(wrapper, 'save');

expect(mockSpacesManager.updateSavedObjectsSpaces).toBeCalledTimes(2);
expect(mockSpacesManager.updateSavedObjectsSpaces).toHaveBeenNthCalledWith(
1,
[{ type: savedObjectToShare.type, id: savedObjectToShare.id }],
['space-2', 'space-3'],
['space-1']
);
expect(mockSpacesManager.updateSavedObjectsSpaces).toHaveBeenNthCalledWith(
2,
[{ type: relatedObject.type, id: relatedObject.id }],
['space-2', 'space-3'],
[]
);

expect(mockToastNotifications.addSuccess).toHaveBeenCalledTimes(1);
expect(mockToastNotifications.addError).not.toHaveBeenCalled();
expect(onClose).toHaveBeenCalledTimes(1);
});
});

describe('correctly renders share mode control', () => {
function getDescriptionAndWarning(wrapper: ReactWrapper) {
const descriptionNode = findTestSubject(wrapper, 'share-mode-control-description');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,27 +85,55 @@ function createDefaultChangeSpacesHandler(
spacesToRemove: string[]
) => {
const { title } = object;
const objectsToUpdate = objects.map(({ type, id }) => ({ type, id })); // only use 'type' and 'id' fields
const objectsToUpdate: Array<{ type: string; id: string }> = objects.map(({ type, id }) => ({
type,
id,
})); // only use 'type' and 'id' fields
const relativesCount = objects.length - 1;
const toastTitle = i18n.translate('xpack.spaces.shareToSpace.shareSuccessTitle', {
values: { objectNoun: object.noun },
defaultMessage: 'Updated {objectNoun}',
description: `Object noun can be plural or singular, examples: "Updated objects", "Updated job"`,
});
await spacesManager.updateSavedObjectsSpaces(objectsToUpdate, spacesToAdd, spacesToRemove);

// If removing spaces and there are referenced objects ("related objects" in UI),
// only remove spaces from the target object.
if (spacesToRemove.length > 0 && objectsToUpdate.length > 1) {
const indexOfTarget = objectsToUpdate.findIndex((element) => element.id === object.id);
if (indexOfTarget >= 0) {
objectsToUpdate.splice(indexOfTarget, 1);
}

const updateTarget = spacesManager.updateSavedObjectsSpaces(
[{ type: object.type, id: object.id }],
spacesToAdd,
spacesToRemove
);

// Only if there are also spaces being added, affect any referenced/related objects
const updateRelated =
spacesToAdd.length > 0
? spacesManager.updateSavedObjectsSpaces(objectsToUpdate, spacesToAdd, [])
: undefined;

await Promise.all([updateTarget, updateRelated]);
} else {
await spacesManager.updateSavedObjectsSpaces(objectsToUpdate, spacesToAdd, spacesToRemove);
}

const isSharedToAllSpaces = spacesToAdd.includes(ALL_SPACES_ID);
let toastText: string;

if (spacesToAdd.length > 0 && spacesToRemove.length > 0 && !isSharedToAllSpaces) {
toastText = i18n.translate('xpack.spaces.shareToSpace.shareSuccessAddRemoveText', {
defaultMessage: `'{object}' {relativesCount, plural, =0 {was} =1 {and {relativesCount} related object were} other {and {relativesCount} related objects were}} added to {spacesTargetAdd} and removed from {spacesTargetRemove}.`,
defaultMessage: `'{object}' {relativesCount, plural, =0 {was} =1 {and {relativesCount} related object were} other {and {relativesCount} related objects were}} added to {spacesTargetAdd}. '{object}' was removed from {spacesTargetRemove}.`,
values: {
object: title,
relativesCount,
spacesTargetAdd: getSpacesTargetString(spacesToAdd),
spacesTargetRemove: getSpacesTargetString(spacesToRemove),
},
description: `Uses output of xpack.spaces.shareToSpace.spacesTarget or xpack.spaces.shareToSpace.allSpacesTarget as 'spacesTarget...' inputs. Example strings: "'Finance dashboard' was added to 1 space and removed from 2 spaces.", "'Finance dashboard' and 2 related objects were added to 3 spaces and removed from all spaces."`,
description: `Uses output of xpack.spaces.shareToSpace.spacesTarget or xpack.spaces.shareToSpace.allSpacesTarget as 'spacesTarget...' inputs. Example strings: "'Finance dashboard' was added to 1 space. 'Finance dashboard' was removed from 2 spaces.", "'Finance dashboard' and 2 related objects were added to 3 spaces. 'Finance dashboard' was removed from all spaces."`,
});
} else if (spacesToAdd.length > 0) {
toastText = i18n.translate('xpack.spaces.shareToSpace.shareSuccessAddText', {
Expand All @@ -119,13 +147,12 @@ function createDefaultChangeSpacesHandler(
});
} else {
toastText = i18n.translate('xpack.spaces.shareToSpace.shareSuccessRemoveText', {
defaultMessage: `'{object}' {relativesCount, plural, =0 {was} =1 {and {relativesCount} related object were} other {and {relativesCount} related objects were}} removed from {spacesTarget}.`,
defaultMessage: `'{object}' was removed from {spacesTarget}.`,
values: {
object: title,
relativesCount,
spacesTarget: getSpacesTargetString(spacesToRemove),
},
description: `Uses output of xpack.spaces.shareToSpace.spacesTarget or xpack.spaces.shareToSpace.allSpacesTarget as 'spacesTarget' input. Example strings: "'Finance dashboard' was removed from 1 space.", "'Finance dashboard' and 2 related objects were removed from all spaces."`,
description: `Uses output of xpack.spaces.shareToSpace.spacesTarget or xpack.spaces.shareToSpace.allSpacesTarget as 'spacesTarget' input. Example string: "'Finance dashboard' was removed from 1 space.", "'Finance dashboard' was removed from all spaces."`,
});
}
toastNotifications.addSuccess({ title: toastTitle, text: toastText });
Expand All @@ -149,6 +176,7 @@ export const ShareToSpaceFlyoutInternal = (props: ShareToSpaceFlyoutProps) => {
}),
[object]
);

const {
flyoutIcon,
flyoutTitle = i18n.translate('xpack.spaces.shareToSpace.flyoutTitle', {
Expand Down Expand Up @@ -322,6 +350,15 @@ export const ShareToSpaceFlyoutInternal = (props: ShareToSpaceFlyoutProps) => {
title: i18n.translate('xpack.spaces.shareToSpace.shareErrorTitle', {
values: { objectNoun: savedObjectTarget.noun },
defaultMessage: 'Error updating {objectNoun}',
description: `Object noun can be plural or singular, examples: "Failed to update objects", "Failed to update job"`,
}),
toastMessage: i18n.translate('xpack.spaces.shareToSpace.shareErrorText', {
defaultMessage: `Unable to update '{object}' {relativesCount, plural, =0 {} =1 {or {relativesCount} related object} other {or one or more of {relativesCount} related objects}}.`,
values: {
object: savedObjectTarget.title,
relativesCount: spacesToAdd.length > 0 ? referenceGraph.length - 1 : 0,
},
description: `Uses output of xpack.spaces.shareToSpace.spacesTarget or xpack.spaces.shareToSpace.allSpacesTarget as 'spacesTarget...' inputs. Example strings: "'Finance dashboard' was added to 1 space. 'Finance dashboard' was removed from 2 spaces.", "'Finance dashboard' and 2 related objects were added to 3 spaces. 'Finance dashboard' was removed from all spaces."`,
}),
});
}
Expand Down

0 comments on commit 379d9eb

Please sign in to comment.