[8.16] fix issue with generating short url when copying share link (#201475) (#201621)

# Backport This will backport the following commits from `main` to `8.16`: - [fix issue with generating short url when copying share link (#201475)](https://github.com/elastic/kibana/pull/201475)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) \r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"197e606bdf812438dbcfe5bb783198a0ea874b52","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:SharedUX","backport:prev-major"],"number":201475,"url":"https://github.com/elastic/kibana/pull/201475","mergeCommit":{"message":"fix issue with generating short url when copying share link (#201475)\n\n## Summary\r\n\r\nExtracted from https://github.com/elastic/kibana/pull/197484, to make it\r\nmuch easier to backport this fix to other releases that require this\r\nfix.\r\n\r\nThis PR fixes the issue with generating short URL for the share link.\r\n\r\n### Checklist\r\n\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"197e606bdf812438dbcfe5bb783198a0ea874b52"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/201475","number":201475,"mergeCommit":{"message":"fix issue with generating short url when copying share link (#201475)\n\n## Summary\r\n\r\nExtracted from https://github.com/elastic/kibana/pull/197484, to make it\r\nmuch easier to backport this fix to other releases that require this\r\nfix.\r\n\r\nThis PR fixes the issue with generating short URL for the share link.\r\n\r\n### Checklist\r\n\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>","sha":"197e606bdf812438dbcfe5bb783198a0ea874b52"}},{"url":"https://github.com/elastic/kibana/pull/201593","number":201593,"branch":"8.17","state":"OPEN"},{"url":"https://github.com/elastic/kibana/pull/201594","number":201594,"branch":"8.x","state":"OPEN"}]}] BACKPORT-->
2025-04-24 09:48:58 -04:00 · 2024-11-25 22:02:54 +01:00 · 2024-11-25 22:02:54 +01:00 · e495095bea
commit e495095bea
parent 57228493d1
6 changed files with 305 additions and 79 deletions
--- a/src/plugins/share/public/components/context/index.test.tsx
+++ b/src/plugins/share/public/components/context/index.test.tsx
@ -0,0 +1,25 @@
+/*
+ * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+import { renderHook } from '@testing-library/react-hooks';
+import { useShareTabsContext } from '.';
+
+describe('share menu context', () => {
+  describe('useShareTabsContext', () => {
+    it('throws an error if used outside of ShareMenuProvider tree', () => {
+      const { result } = renderHook(() => useShareTabsContext());
+
+      expect(result.error?.message).toEqual(
+        expect.stringContaining(
+          'Failed to call `useShareTabsContext` because the context from ShareMenuProvider is missing.'
+        )
+      );
+    });
+  });
+});
--- a/src/plugins/share/public/components/context/index.tsx
+++ b/src/plugins/share/public/components/context/index.tsx
@ -9,7 +9,7 @@

 import { ThemeServiceSetup } from '@kbn/core-theme-browser';
 import { I18nStart } from '@kbn/core/public';
-import { createContext, useContext } from 'react';
+import React, { type PropsWithChildren, createContext, useContext } from 'react';

 import { AnonymousAccessServiceContract } from '../../../common';
 import type {
@ -35,6 +35,23 @@ export interface IShareContext extends ShareContext {
  anchorElement?: HTMLElement;
 }

-export const ShareTabsContext = createContext<IShareContext | null>(null);
+const ShareTabsContext = createContext<IShareContext | null>(null);

-export const useShareTabsContext = () => useContext(ShareTabsContext);
+export const ShareMenuProvider = ({
+  shareContext,
+  children,
+}: PropsWithChildren<{ shareContext: IShareContext }>) => {
+  return <ShareTabsContext.Provider value={shareContext}>{children}</ShareTabsContext.Provider>;
+};
+
+export const useShareTabsContext = () => {
+  const context = useContext(ShareTabsContext);
+
+  if (!context) {
+    throw new Error(
+      'Failed to call `useShareTabsContext` because the context from ShareMenuProvider is missing. Ensure the component or React root is wrapped with ShareMenuProvider'
+    );
+  }
+
+  return context;
+};
--- a/src/plugins/share/public/components/share_tabs.test.tsx
+++ b/src/plugins/share/public/components/share_tabs.test.tsx
@ -9,7 +9,7 @@

 import React from 'react';
 import { ShareMenuTabs } from './share_tabs';
-import { ShareTabsContext } from './context';
+import { ShareMenuProvider } from './context';
 import { mountWithIntl } from '@kbn/test-jest-helpers';
 import { KibanaLocation, LocatorGetUrlParams, UrlService } from '../../common/url_service';
 import {
@ -77,9 +77,9 @@ describe('Share modal tabs', () => {
      },
    ];
    const wrapper = mountWithIntl(
-      <ShareTabsContext.Provider value={{ ...mockShareContext, shareMenuItems: testItem }}>
+      <ShareMenuProvider shareContext={{ ...mockShareContext, shareMenuItems: testItem }}>
        <ShareMenuTabs />
-      </ShareTabsContext.Provider>
+      </ShareMenuProvider>
    );
    expect(wrapper.find('[data-test-subj="export"]').exists()).toBeTruthy();
  });
@ -92,11 +92,13 @@ describe('Share modal tabs', () => {
        generateExportUrl: mockGenerateExportUrl,
      },
    ];
+
    const wrapper = mountWithIntl(
-      <ShareTabsContext.Provider value={{ ...mockShareContext, shareMenuItems: testItems }}>
+      <ShareMenuProvider shareContext={{ ...mockShareContext, shareMenuItems: testItems }}>
        <ShareMenuTabs />
-      </ShareTabsContext.Provider>
+      </ShareMenuProvider>
    );
+
    expect(wrapper.find('[data-test-subj="export"]').exists()).toBeFalsy();
  });

@ -116,9 +118,9 @@ describe('Share modal tabs', () => {
      },
    ];
    const wrapper = mountWithIntl(
-      <ShareTabsContext.Provider value={{ ...mockShareContext, shareMenuItems: testItem }}>
+      <ShareMenuProvider shareContext={{ ...mockShareContext, shareMenuItems: testItem }}>
        <ShareMenuTabs />
-      </ShareTabsContext.Provider>
+      </ShareMenuProvider>
    );
    expect(wrapper.find('[data-test-subj="export"]').exists()).toBeTruthy();
  });
--- a/src/plugins/share/public/components/share_tabs.tsx
+++ b/src/plugins/share/public/components/share_tabs.tsx
@ -8,16 +8,16 @@
 */

 import React, { type FC } from 'react';
-import { TabbedModal } from '@kbn/shared-ux-tabbed-modal';
+import { TabbedModal, type IModalTabDeclaration } from '@kbn/shared-ux-tabbed-modal';

-import { ShareTabsContext, useShareTabsContext, type IShareContext } from './context';
+import { ShareMenuProvider, useShareTabsContext, type IShareContext } from './context';
 import { linkTab, embedTab, exportTab } from './tabs';

 export const ShareMenu: FC<{ shareContext: IShareContext }> = ({ shareContext }) => {
  return (
-    <ShareTabsContext.Provider value={shareContext}>
+    <ShareMenuProvider {...{ shareContext }}>
      <ShareMenuTabs />
-    </ShareTabsContext.Provider>
+    </ShareMenuProvider>
  );
 };

@ -25,15 +25,9 @@ export const ShareMenu: FC<{ shareContext: IShareContext }> = ({ shareContext })
 export const ShareMenuTabs = () => {
  const shareContext = useShareTabsContext();

-  if (!shareContext) {
-    return null;
-  }
-
  const { allowEmbed, objectTypeMeta, onClose, shareMenuItems, anchorElement } = shareContext;

-  const tabs = [];
-
-  tabs.push(linkTab);
+  const tabs: Array<IModalTabDeclaration<any>> = [linkTab];

  const enabledItems = shareMenuItems.filter(({ shareMenuItem }) => !shareMenuItem?.disabled);

--- a/src/plugins/share/public/components/tabs/link/link_content.test.tsx
+++ b/src/plugins/share/public/components/tabs/link/link_content.test.tsx
@ -0,0 +1,191 @@
+/*
+ * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+import React, { type ComponentProps } from 'react';
+import { __IntlProvider as IntlProvider } from '@kbn/i18n-react';
+import userEvent from '@testing-library/user-event';
+import { render, screen, waitFor } from '@testing-library/react';
+
+import { urlServiceTestSetup } from '../../../../common/url_service/__tests__/setup';
+import { MockLocatorDefinition } from '../../../../common/url_service/mocks';
+import { BrowserShortUrlClientFactory } from '../../../url_service/short_urls/short_url_client_factory';
+import {
+  BrowserShortUrlClientHttp,
+  BrowserShortUrlClient,
+} from '../../../url_service/short_urls/short_url_client';
+import { BrowserUrlService } from '../../../types';
+import { LinkContent } from './link_content';
+
+const renderComponent = (props: ComponentProps<typeof LinkContent>) => {
+  render(
+    <IntlProvider locale="en">
+      <LinkContent {...props} />
+    </IntlProvider>
+  );
+};
+
+describe('LinkContent', () => {
+  const shareableUrl = 'http://localhost:5601/app/dashboards#/view/123';
+
+  const http: BrowserShortUrlClientHttp = {
+    basePath: {
+      get: () => '/xyz',
+    },
+    fetch: jest.fn(async () => {
+      return {} as any;
+    }),
+  };
+
+  let urlService: BrowserUrlService;
+
+  // @ts-expect-error there is a type because we override the shortUrls implementation
+  // eslint-disable-next-line prefer-const
+  ({ service: urlService } = urlServiceTestSetup({
+    shortUrls: ({ locators }) =>
+      new BrowserShortUrlClientFactory({
+        http,
+        locators,
+      }),
+  }));
+
+  beforeAll(() => {
+    Object.defineProperty(document, 'execCommand', {
+      value: jest.fn(() => true),
+    });
+  });
+
+  it('uses the delegatedShareUrlHandler to generate the shareable URL when it is provided', async () => {
+    const user = userEvent.setup();
+    const objectType = 'dashboard';
+    const objectId = '123';
+    const isDirty = false;
+
+    const delegatedShareUrlHandler = jest.fn();
+
+    renderComponent({
+      objectType,
+      objectId,
+      isDirty,
+      shareableUrl,
+      urlService,
+      allowShortUrl: true,
+      delegatedShareUrlHandler,
+    });
+
+    await user.click(screen.getByTestId('copyShareUrlButton'));
+
+    expect(delegatedShareUrlHandler).toHaveBeenCalled();
+  });
+
+  it('returns the shareable URL when the delegatedShareUrlHandler is not provided and shortURLs are not allowed', async () => {
+    const user = userEvent.setup();
+    const objectType = 'dashboard';
+    const objectId = '123';
+    const isDirty = false;
+
+    renderComponent({
+      objectType,
+      objectId,
+      isDirty,
+      shareableUrl,
+      urlService,
+      allowShortUrl: false,
+    });
+
+    const copyButton = screen.getByTestId('copyShareUrlButton');
+
+    await user.click(copyButton);
+
+    waitFor(() => {
+      expect(copyButton.getAttribute('data-share-url')).toBe(shareableUrl);
+    });
+  });
+
+  it('invokes the createWithLocator method on the shortURL service if a locator is present when the copy button is clicked', async () => {
+    const user = userEvent.setup();
+    const objectType = 'dashboard';
+    const objectId = '123';
+    const isDirty = false;
+    const shareableUrlLocatorParams = {
+      locator: new MockLocatorDefinition('TEST_LOCATOR'),
+      params: {},
+    };
+
+    const shortURL = 'http://localhost:5601/xyz/r/s/yellow-orange-tomato';
+
+    const createWithLocatorSpy = jest.spyOn(BrowserShortUrlClient.prototype, 'createWithLocator');
+
+    createWithLocatorSpy.mockResolvedValue({
+      // @ts-expect-error we only return locator property, as that's all we need for this test
+      locator: {
+        getUrl: jest.fn(() => Promise.resolve(shortURL)),
+      },
+    });
+
+    renderComponent({
+      objectType,
+      objectId,
+      isDirty,
+      shareableUrl,
+      urlService,
+      allowShortUrl: true,
+      // @ts-ignore this locator is passed mainly to test the code path that invokes createWithLocator
+      shareableUrlLocatorParams,
+    });
+
+    const copyButton = screen.getByTestId('copyShareUrlButton');
+
+    const numberOfClicks = 4;
+
+    for (const _click of Array.from({ length: numberOfClicks })) {
+      await user.click(copyButton);
+    }
+
+    // should only invoke once no matter how many times the button is clicked
+    expect(createWithLocatorSpy).toHaveBeenCalledTimes(1);
+    expect(copyButton.getAttribute('data-share-url')).toBe(shortURL);
+  });
+
+  it('invokes the createFromLongUrl method on the shortURL service if a locator is not present when the copy button is clicked', async () => {
+    const user = userEvent.setup();
+    const objectType = 'dashboard';
+    const objectId = '123';
+    const isDirty = false;
+
+    const shortURL = 'http://localhost:5601/xyz/r/s/yellow-orange-tomato';
+
+    const createFromLongUrlSpy = jest.spyOn(BrowserShortUrlClient.prototype, 'createFromLongUrl');
+
+    // @ts-expect-error we only return url property, as that's all we need for this test
+    createFromLongUrlSpy.mockResolvedValue({
+      url: shortURL,
+    });
+
+    renderComponent({
+      objectType,
+      objectId,
+      isDirty,
+      shareableUrl,
+      urlService,
+      allowShortUrl: true,
+    });
+
+    const copyButton = screen.getByTestId('copyShareUrlButton');
+
+    const numberOfClicks = 4;
+
+    for (const _click of Array.from({ length: numberOfClicks })) {
+      await user.click(copyButton);
+    }
+
+    // should only invoke once no matter how many times the button is clicked
+    expect(createFromLongUrlSpy).toHaveBeenCalledTimes(1);
+    expect(copyButton.getAttribute('data-share-url')).toBe(shortURL);
+  });
+});
--- a/src/plugins/share/public/components/tabs/link/link_content.tsx
+++ b/src/plugins/share/public/components/tabs/link/link_content.tsx
@ -20,8 +20,8 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
-import React, { useCallback, useMemo, useState } from 'react';
-import { IShareContext } from '../../context';
+import React, { useCallback, useState, useRef, useEffect } from 'react';
+import type { IShareContext } from '../../context';

 type LinkProps = Pick<
  IShareContext,
@ -42,84 +42,80 @@ interface UrlParams {
 }

 export const LinkContent = ({
-  objectType,
  isDirty,
+  objectType,
  shareableUrl,
  urlService,
  shareableUrlLocatorParams,
  allowShortUrl,
  delegatedShareUrlHandler,
 }: LinkProps) => {
-  const [url, setUrl] = useState<string>('');
-  const [urlParams] = useState<UrlParams | undefined>(undefined);
+  const [snapshotUrl, setSnapshotUrl] = useState<string>('');
  const [isTextCopied, setTextCopied] = useState(false);
-  const [shortUrlCache, setShortUrlCache] = useState<string | undefined>(undefined);
+  const [isLoading, setIsLoading] = useState(false);
+  const urlParamsRef = useRef<UrlParams | undefined>(undefined);
+  const urlToCopy = useRef<string | undefined>(undefined);
+  const copiedTextToolTipCleanupIdRef = useRef<ReturnType<typeof setTimeout>>();

-  const getUrlWithUpdatedParams = useCallback(
-    (tempUrl: string): string => {
-      const urlWithUpdatedParams = urlParams
-        ? Object.keys(urlParams).reduce((urlAccumulator, key) => {
-            const urlParam = urlParams[key];
-            return urlParam
-              ? Object.keys(urlParam).reduce((queryAccumulator, queryParam) => {
-                  const isQueryParamEnabled = urlParam[queryParam];
-                  return isQueryParamEnabled
-                    ? queryAccumulator + `&${queryParam}=true`
-                    : queryAccumulator;
-                }, urlAccumulator)
-              : urlAccumulator;
-          }, tempUrl)
-        : tempUrl;
+  const getUrlWithUpdatedParams = useCallback((tempUrl: string): string => {
+    const urlWithUpdatedParams = urlParamsRef.current
+      ? Object.keys(urlParamsRef.current).reduce((urlAccumulator, key) => {
+          const urlParam = urlParamsRef.current?.[key];
+          return urlParam
+            ? Object.keys(urlParam).reduce((queryAccumulator, queryParam) => {
+                const isQueryParamEnabled = urlParam[queryParam];
+                return isQueryParamEnabled
+                  ? queryAccumulator + `&${queryParam}=true`
+                  : queryAccumulator;
+              }, urlAccumulator)
+            : urlAccumulator;
+        }, tempUrl)
+      : tempUrl;

-      // persist updated url to state
-      setUrl(urlWithUpdatedParams);
-      return urlWithUpdatedParams;
-    },
-    [urlParams]
-  );
+    return urlWithUpdatedParams;
+  }, []);

-  const getSnapshotUrl = useCallback(() => {
-    return getUrlWithUpdatedParams(shareableUrl || window.location.href);
+  useEffect(() => {
+    setSnapshotUrl(getUrlWithUpdatedParams(shareableUrl || window.location.href));
  }, [getUrlWithUpdatedParams, shareableUrl]);

  const createShortUrl = useCallback(async () => {
-    if (shareableUrlLocatorParams) {
-      const shortUrls = urlService.shortUrls.get(null);
-      const shortUrl = await shortUrls.createWithLocator(shareableUrlLocatorParams);
-      const urlWithLoc = await shortUrl.locator.getUrl(shortUrl.params, { absolute: true });
-      setShortUrlCache(urlWithLoc);
-      return urlWithLoc;
-    } else {
-      const snapshotUrl = getSnapshotUrl();
-      const shortUrl = await urlService.shortUrls.get(null).createFromLongUrl(snapshotUrl);
-      setShortUrlCache(shortUrl.url);
+    const shortUrlService = urlService.shortUrls.get(null);

-      return shortUrl.url;
+    if (shareableUrlLocatorParams) {
+      const shortUrl = await shortUrlService.createWithLocator(shareableUrlLocatorParams);
+      return shortUrl.locator.getUrl(shortUrl.params, { absolute: true });
+    } else {
+      return (await shortUrlService.createFromLongUrl(snapshotUrl)).url;
    }
-  }, [shareableUrlLocatorParams, urlService.shortUrls, getSnapshotUrl, setShortUrlCache]);
+  }, [shareableUrlLocatorParams, urlService.shortUrls, snapshotUrl]);

  const copyUrlHelper = useCallback(async () => {
-    let urlToCopy = url;
+    setIsLoading(true);

-    if (!urlToCopy || delegatedShareUrlHandler) {
-      urlToCopy = delegatedShareUrlHandler
-        ? delegatedShareUrlHandler?.()
+    if (!urlToCopy.current) {
+      urlToCopy.current = delegatedShareUrlHandler
+        ? delegatedShareUrlHandler()
        : allowShortUrl
        ? await createShortUrl()
-        : getSnapshotUrl();
+        : snapshotUrl;
    }

-    copyToClipboard(urlToCopy);
-    setUrl(urlToCopy);
-    setTextCopied(true);
-    return urlToCopy;
-  }, [url, delegatedShareUrlHandler, allowShortUrl, createShortUrl, getSnapshotUrl]);
+    copyToClipboard(urlToCopy.current);
+    setTextCopied(() => {
+      if (copiedTextToolTipCleanupIdRef.current) {
+        clearInterval(copiedTextToolTipCleanupIdRef.current);
+      }
+
+      // set up timer to revert copied state to false after specified duration
+      copiedTextToolTipCleanupIdRef.current = setTimeout(() => setTextCopied(false), 1000);
+
+      // set copied state to true for now
+      return true;
+    });
+    setIsLoading(false);
+  }, [snapshotUrl, delegatedShareUrlHandler, allowShortUrl, createShortUrl]);

-  const handleTestUrl = useMemo(() => {
-    if (objectType !== 'search' || !allowShortUrl) return getSnapshotUrl();
-    else if (objectType === 'search' && allowShortUrl) return shortUrlCache;
-    return copyUrlHelper();
-  }, [objectType, getSnapshotUrl, allowShortUrl, shortUrlCache, copyUrlHelper]);
  return (
    <>
      <EuiForm>
@ -153,17 +149,18 @@ export const LinkContent = ({
          <EuiToolTip
            content={
              isTextCopied
-                ? i18n.translate('share.link.copied', { defaultMessage: 'Text copied' })
+                ? i18n.translate('share.link.copied', { defaultMessage: 'Link copied' })
                : null
            }
          >
            <EuiButton
              fill
              data-test-subj="copyShareUrlButton"
-              data-share-url={handleTestUrl}
+              data-share-url={urlToCopy.current}
              onBlur={() => (objectType === 'lens' && isDirty ? null : setTextCopied(false))}
              onClick={copyUrlHelper}
              color={objectType === 'lens' && isDirty ? 'warning' : 'primary'}
+              isLoading={isLoading}
            >
              <FormattedMessage id="share.link.copyLinkButton" defaultMessage="Copy link" />
            </EuiButton>