Fixes issue with removing spaces from related objects in share to spa…

…ce action (elastic#169177) closes elastic#168657 ## Summary Updates the `share to spaces` action to refrain from removing spaces from related objects (objects referenced by the target object). I have also updated the description of issue elastic#130562, which essentially will replace much of the client-side implementation of this action, to explicitly include this behavior. ### Manual Testing - Create 2 spaces: A and B - Add a sample data set (e.g. flight) to space A - In Discover, create a saved query called "s1" (add a filter that uses the sample data logs data view, and use the filter menu button) - Go to `Stack Management->Saved` Objects and share the "s1" query to space B - Verify that the related data view is also shared to space B. - Un-share the "s1" query from space B - Verify that the related data view is still shared to space B. ### Automated Tests - x-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.test.tsx --------- Co-authored-by: kibanamachine <[email protected]>
kibanamachine · Oct 23, 2023 · 4af9175 · 4af9175
1 parent 03a6a5c
commit 4af9175
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 10 deletions.
diff --git a/...lugins/spaces/public/copy_saved_objects_to_space/components/selectable_spaces_control.tsx b/...lugins/spaces/public/copy_saved_objects_to_space/components/selectable_spaces_control.tsx
@@ -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"

diff --git a/...ugins/spaces/public/share_saved_objects_to_space/components/selectable_spaces_control.tsx b/...ugins/spaces/public/share_saved_objects_to_space/components/selectable_spaces_control.tsx
@@ -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"

diff --git a/x-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_mode_control.tsx b/x-pack/plugins/spaces/public/share_saved_objects_to_space/components/share_mode_control.tsx
@@ -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"

diff --git a/...es/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.test.tsx b/...es/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.test.tsx
@@ -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');

diff --git a/.../spaces/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.tsx b/.../spaces/public/share_saved_objects_to_space/components/share_to_space_flyout_internal.tsx
@@ -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', {
@@ -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 });
@@ -149,6 +176,7 @@ export const ShareToSpaceFlyoutInternal = (props: ShareToSpaceFlyoutProps) => {
     }),
     [object]
   );
+
   const {
     flyoutIcon,
     flyoutTitle = i18n.translate('xpack.spaces.shareToSpace.flyoutTitle', {
@@ -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."`,
         }),
       });
     }