[Fix] Additional tests and clean up for share modal redesign initiati…

…ve (#183643) ## Summary Closes #181066 Closes epic https://github.com/elastic/kibana-team/issues/735 Closes #183752 - [x] Based on PR #180406, the test 'generates a report with single timefilter' was skipped in x-pack/test/functional/apps/discover/reporting.ts because short URLs are the default and make the test fail. - [x] In addition test/functional/apps/dashboard/group5/share.ts was skipped for similar reasons - [x] x-pack/test/functional/apps/lens/group4/share.ts 'should preserve filter and query when sharing' - [x] [x-pack/test_serverless/functional/test_suites/common/discover/x_pack/reporting.ts](https://github.com/elastic/kibana/pull/180406/files/03f8d94aedd6d76bcaf7cfa4db714c7665269807#diff-f8df59654e26e509d3e2b9fd3b998da7ea0f7b1d02bddced1acbd588d6b55883) refactoring - [x] [x-pack/test/functional/apps/discover/feature_controls/discover_security.ts](https://github.com/elastic/kibana/pull/180406/files/ec0bec8387e8b75de2e57adb34517741052e18c4#diff-1efa1c849e2a1f9e206702cafa82c5d5d7b1a446add207b693f8fbebc992b59d) - [x] Add lens by-value FTR to confirm share link in lens is passing a share link that will open to a Lens visualization. For reference check out #180406 (review) ### Checklist - [x] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
elastic · Jun 12, 2024 · 4afcc0a · 4afcc0a
1 parent 83c1512
commit 4afcc0a
Show file tree

Hide file tree

Showing 13 changed files with 149 additions and 88 deletions.
diff --git a/packages/kbn-reporting/public/share/share_context_menu/register_csv_modal_reporting.tsx b/packages/kbn-reporting/public/share/share_context_menu/register_csv_modal_reporting.tsx
@@ -142,7 +142,7 @@ export const reportingCsvShareProvider = ({
 
       const relativePath = apiClient.getReportingPublicJobPath(
         reportType,
-        apiClient.getDecoratedJobParams(getJobParams())
+        apiClient.getDecoratedJobParams(getJobParams(true))
       );
 
       const absoluteUrl = new URL(relativePath, window.location.href).toString();

diff --git a/src/plugins/share/README.mdx b/src/plugins/share/README.mdx
@@ -13,10 +13,14 @@ The `share` plugin contains various utilities for displaying sharing context men
 generating deep links to other apps using *locators*, and creating short URLs.
 
 
-## Sharing context menu
+## Sharing context menu and modal
 
-You can register an item into sharing context menu (which is displayed in
-Dashboard, Discover, and Visualize apps).
+You can register an item into sharing context modal or menu (which is displayed in
+Dashboard, Discover, and Visualize apps). In early 2024, the Shared UX team created a tabbed share modal redesign.
+Canvas still is using the older share context menu, but Dashboard, Discover, and Visualize apps are since using 
+the new modal implementation. This change was made for a less cluttered UI and streamlined user experience. 
+Copy links default to short links based on user feedback. Reports/Exports have been consolidated into one tab called 
+Exports, versus the separated panels in the share context menu. 
 
 
 ## Locators
@@ -204,15 +208,3 @@ const url = await shortUrls.create({
 
 To resolve the short URL, navigate to `/r/s/<slug>` in the browser.
 
-### Redesign of Share Context menu
-April 2024 the share context menu changed from using EUI panels to a tabbed modal. One of the goals
-was to streamline the user experience and remove areas of confusion. For instance, the saved object
-and snapshot radio options in the Link portion was confusing to users. The following was implemented 
-in the redesign:
-
-When user clicks the ‘copy link’ button
-For dashboard: copy the “snapshot” URL to user clipboard
-For lens: copy the “saved object” URL to user clipboard. 
-If lens is not saved to library you cannot copy (show unsaved changed error as in figma)
-For discover: discover is saved: copy the “snapshot” URL to user clipboard
-Default to short URL where possible 
diff --git a/src/plugins/share/public/components/tabs/export/export_content.tsx b/src/plugins/share/public/components/tabs/export/export_content.tsx
@@ -133,6 +133,7 @@ const ExportContentUi = ({
                   iconType="copyClipboard"
                   onClick={copy}
                   data-test-subj="shareReportingCopyURL"
+                  data-share-url={absoluteUrl}
                 >
                   <FormattedMessage
                     id="share.modalContent.copyUrlButtonLabel"

diff --git a/src/plugins/share/public/components/tabs/link/link_content.tsx b/src/plugins/share/public/components/tabs/link/link_content.tsx
@@ -19,7 +19,7 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
-import React, { useCallback, useState } from 'react';
+import React, { useCallback, useMemo, useState } from 'react';
 import { IShareContext } from '../../context';
 
 type LinkProps = Pick<
@@ -52,7 +52,7 @@ export const LinkContent = ({
   const [url, setUrl] = useState<string>('');
   const [urlParams] = useState<UrlParams | undefined>(undefined);
   const [isTextCopied, setTextCopied] = useState(false);
-  const [, setShortUrlCache] = useState<string | undefined>(undefined);
+  const [shortUrlCache, setShortUrlCache] = useState<string | undefined>(undefined);
 
   const getUrlWithUpdatedParams = useCallback(
     (tempUrl: string): string => {
@@ -72,7 +72,6 @@ export const LinkContent = ({
 
       // persist updated url to state
       setUrl(urlWithUpdatedParams);
-
       return urlWithUpdatedParams;
     },
     [urlParams]
@@ -93,6 +92,7 @@ export const LinkContent = ({
       const snapshotUrl = getSnapshotUrl();
       const shortUrl = await urlService.shortUrls.get(null).createFromLongUrl(snapshotUrl);
       setShortUrlCache(shortUrl.url);
+
       return shortUrl.url;
     }
   }, [shareableUrlLocatorParams, urlService.shortUrls, getSnapshotUrl, setShortUrlCache]);
@@ -111,8 +111,14 @@ export const LinkContent = ({
     copyToClipboard(urlToCopy);
     setUrl(urlToCopy);
     setTextCopied(true);
+    return urlToCopy;
   }, [url, delegatedShareUrlHandler, allowShortUrl, createShortUrl, getSnapshotUrl]);
 
+  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>
@@ -154,7 +160,7 @@ export const LinkContent = ({
             <EuiButton
               fill
               data-test-subj="copyShareUrlButton"
-              data-share-url={url}
+              data-share-url={handleTestUrl}
               onBlur={() => (objectType === 'lens' && isDirty ? null : setTextCopied(false))}
               onClick={copyUrlHelper}
               color={objectType === 'lens' && isDirty ? 'warning' : 'primary'}

diff --git a/test/functional/apps/dashboard/group3/dashboard_state.ts b/test/functional/apps/dashboard/group3/dashboard_state.ts
@@ -185,8 +185,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       await PageObjects.dashboard.waitForRenderComplete();
     };
 
-    // FLAKY: https://github.com/elastic/kibana/issues/139762
-    describe.skip('Directly modifying url updates dashboard state', () => {
+    describe('Directly modifying url updates dashboard state', () => {
       before(async () => {
         await PageObjects.dashboard.gotoDashboardLandingPage();
         await PageObjects.dashboard.clickNewDashboard();

diff --git a/test/functional/apps/dashboard/group5/share.ts b/test/functional/apps/dashboard/group5/share.ts
@@ -49,13 +49,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       await PageObjects.share.clickShareTopNavButton();
       return await PageObjects.share.isShareMenuOpen();
     });
-    // if (mode === 'savedObject') {
-    // await PageObjects.share.exportAsSavedObject();
-    // }
-    return PageObjects.share.getSharedUrl();
+    return await PageObjects.share.getSharedUrl();
   };
 
-  describe.skip('share dashboard', () => {
+  describe('share dashboard', () => {
     const testFilterState = async (mode: TestingModes) => {
       it('should not have "filters" state in either app or global state when no filters', async () => {
         expect(await getSharedUrl(mode)).to.not.contain('filters');
@@ -120,7 +117,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       await PageObjects.common.unsetTime();
     });
 
-    describe.skip('snapshot share', async () => {
+    describe('snapshot share', async () => {
       describe('test local state', async () => {
         it('should not have "panels" state when not in unsaved changes state', async () => {
           await testSubjects.missingOrFail('dashboardUnsavedChangesBadge');
@@ -147,7 +144,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         });
       });
 
-      describe.skip('test filter state', async () => {
+      describe('test filter state', async () => {
         await testFilterState('snapshot');
       });
 
@@ -158,7 +155,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       });
     });
 
-    describe.skip('saved object share', async () => {
+    describe('saved object share', async () => {
       describe('test filter state', async () => {
         await testFilterState('savedObject');
       });

diff --git a/test/functional/apps/discover/group5/_shared_links.ts b/test/functional/apps/discover/group5/_shared_links.ts
@@ -61,11 +61,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
       });
 
       describe('permalink', function () {
-        it('should allow for copying the snapshot URL as a short URL', async function () {
-          const re = new RegExp(baseUrl + '/app/r/s/.+$');
+        it('should allow for copying the snapshot URL', async function () {
+          const re = new RegExp(baseUrl + '/app/r.+$');
           await retry.try(async () => {
             const actualUrl = await PageObjects.share.getSharedUrl();
-            expect(actualUrl).to.match(re);
+            expect(actualUrl).match(re);
           });
         });
 
@@ -104,12 +104,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         await teardown();
       });
 
-      it('should allow for copying the snapshot URL as a short URL and should open it', async function () {
-        const re = new RegExp(baseUrl + '/app/r/s/.+$');
+      it('should allow for copying the snapshot URL and should open it', async function () {
+        const re = new RegExp(baseUrl + '/app/r.*$');
         let actualUrl: string = '';
         await retry.try(async () => {
           actualUrl = await PageObjects.share.getSharedUrl();
-          expect(actualUrl).to.match(re);
+          expect(actualUrl).match(re);
         });
 
         const actualTime = await PageObjects.timePicker.getTimeConfig();

diff --git a/x-pack/plugins/lens/public/app_plugin/lens_top_nav.tsx b/x-pack/plugins/lens/public/app_plugin/lens_top_nav.tsx
@@ -625,7 +625,9 @@ export const LensTopNavMenu = ({
               allowEmbed: false,
               allowShortUrl: false,
               delegatedShareUrlHandler: () => {
-                return isCurrentStateDirty ? shareableUrl! : savedObjectURL.href;
+                return isCurrentStateDirty || !currentDoc?.savedObjectId
+                  ? shareableUrl!
+                  : savedObjectURL.href;
               },
               objectId: currentDoc?.savedObjectId,
               objectType: 'lens',
@@ -636,7 +638,7 @@ export const LensTopNavMenu = ({
               },
               sharingData,
               // only want to know about changes when savedObjectURL.href
-              isDirty: isCurrentStateDirty,
+              isDirty: isCurrentStateDirty || !currentDoc?.savedObjectId,
               // disable the menu if both shortURL permission and the visualization has not been saved
               // TODO: improve here the disabling state with more specific checks
               disabledShareUrl: Boolean(!shareUrlEnabled && !currentDoc?.savedObjectId),

diff --git a/x-pack/test/functional/apps/discover/feature_controls/discover_security.ts b/x-pack/test/functional/apps/discover/feature_controls/discover_security.ts
@@ -5,7 +5,9 @@
  * 2.0.
  */
 
+import { DISCOVER_APP_LOCATOR } from '@kbn/discover-plugin/common';
 import expect from '@kbn/expect';
+import { decompressFromBase64 } from 'lz-string';
 import { FtrProviderContext } from '../../../ftr_provider_context';
 import { getSavedQuerySecurityUtils } from '../../saved_query_management/utils/saved_query_security';
 
@@ -35,6 +37,7 @@ export default function (ctx: FtrProviderContext) {
   const testSubjects = getService('testSubjects');
   const appsMenu = getService('appsMenu');
   const kibanaServer = getService('kibanaServer');
+  const deployment = getService('deployment');
   const logstashIndexName = 'logstash-2015.09.22';
 
   async function setDiscoverTimeRange() {
@@ -119,6 +122,19 @@ export default function (ctx: FtrProviderContext) {
         await globalNav.badgeMissingOrFail();
       });
 
+      it('Shows short urls for users with the right privileges', async () => {
+        let actualUrl: string = '';
+        await PageObjects.share.clickShareTopNavButton();
+        const re = new RegExp(
+          deployment.getHostPort().replace(':80', '').replace(':443', '') + '/app/r.*$'
+        );
+        await retry.try(async () => {
+          actualUrl = await PageObjects.share.getSharedUrl();
+          expect(actualUrl).to.match(re);
+          await PageObjects.share.closeShareModal();
+        });
+      });
+
       it('shows CSV reports', async () => {
         await PageObjects.share.clickShareTopNavButton();
         await PageObjects.share.clickTab('Export');
@@ -190,6 +206,44 @@ export default function (ctx: FtrProviderContext) {
         await PageObjects.unifiedFieldList.expectMissingFieldListItemVisualize('bytes');
       });
 
+      it('should allow for copying the snapshot URL', async function () {
+        await PageObjects.share.clickShareTopNavButton();
+        const actualUrl = await PageObjects.share.getSharedUrl();
+        expect(actualUrl).to.contain(`?l=${DISCOVER_APP_LOCATOR}`);
+        const urlSearchParams = new URLSearchParams(actualUrl);
+        expect(JSON.parse(decompressFromBase64(urlSearchParams.get('lz')!)!)).to.eql({
+          query: {
+            language: 'kuery',
+            query: '',
+          },
+          sort: [['@timestamp', 'desc']],
+          columns: [],
+          interval: 'auto',
+          filters: [],
+          dataViewId: 'logstash-*',
+          timeRange: {
+            from: '2015-09-19T06:31:44.000Z',
+            to: '2015-09-23T18:31:44.000Z',
+          },
+          refreshInterval: {
+            value: 60000,
+            pause: true,
+          },
+        });
+        await PageObjects.share.closeShareModal();
+      });
+
+      it(`Doesn't show short urls for users without those privileges`, async () => {
+        await PageObjects.share.clickShareTopNavButton();
+        let actualUrl: string = '';
+
+        await retry.try(async () => {
+          actualUrl = await PageObjects.share.getSharedUrl();
+          // only shows in long urls
+          expect(actualUrl).to.contain(DISCOVER_APP_LOCATOR);
+          await PageObjects.share.closeShareModal();
+        });
+      });
       savedQuerySecurityUtils.shouldDisallowSavingButAllowLoadingSavedQueries();
     });
 
@@ -253,6 +307,19 @@ export default function (ctx: FtrProviderContext) {
         await PageObjects.unifiedFieldList.expectMissingFieldListItemVisualize('bytes');
       });
 
+      it('Shows short urls for users with the right privileges', async () => {
+        await PageObjects.share.clickShareTopNavButton();
+        let actualUrl: string = '';
+        const re = new RegExp(
+          deployment.getHostPort().replace(':80', '').replace(':443', '') + '/app/r.*$'
+        );
+        await retry.try(async () => {
+          actualUrl = await PageObjects.share.getSharedUrl();
+          expect(actualUrl).to.match(re);
+          await PageObjects.share.closeShareModal();
+        });
+      });
+
       savedQuerySecurityUtils.shouldDisallowSavingButAllowLoadingSavedQueries();
     });
 

diff --git a/x-pack/test/functional/apps/discover/reporting.ts b/x-pack/test/functional/apps/discover/reporting.ts
@@ -6,8 +6,8 @@
  */
 
 import expect from '@kbn/expect';
-import { Key } from 'selenium-webdriver';
 import moment from 'moment';
+import { Key } from 'selenium-webdriver';
 import { FtrProviderContext } from '../../ftr_provider_context';
 
 export default function ({ getService, getPageObjects }: FtrProviderContext) {
@@ -103,8 +103,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         await PageObjects.discover.selectIndexPattern('ecommerce');
       });
 
-      // Discover defaults to short urls - is this test helpful? Clarify in separate PR
-      xit('generates a report with single timefilter', async () => {
+      it('generates a report with single timefilter', async () => {
         await PageObjects.discover.clickNewSearchButton();
         await PageObjects.timePicker.setCommonlyUsedTime('Last_24 hours');
         await PageObjects.discover.saveSearch('single-timefilter-search');
@@ -116,22 +115,23 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         await PageObjects.share.clickShareTopNavButton();
         await PageObjects.reporting.openExportTab();
         const copyButton = await testSubjects.find('shareReportingCopyURL');
-        const reportURL = (await copyButton.getAttribute('data-share-url')) ?? '';
+        const reportURL = decodeURIComponent(
+          (await copyButton.getAttribute('data-share-url')) ?? ''
+        );
 
         // get number of filters in URLs
         const timeFiltersNumberInReportURL =
-          decodeURIComponent(reportURL).split(
-            'query:(range:(order_date:(format:strict_date_optional_time'
-          ).length - 1;
+          reportURL.split('query:(range:(order_date:(format:strict_date_optional_time').length - 1;
         const timeFiltersNumberInSharedURL = sharedURL.split('time:').length - 1;
 
         expect(timeFiltersNumberInSharedURL).to.be(1);
         expect(sharedURL.includes('time:(from:now-24h%2Fh,to:now))')).to.be(true);
 
         expect(timeFiltersNumberInReportURL).to.be(1);
+
         expect(
-          decodeURIComponent(reportURL).includes(
-            'query:(range:(order_date:(format:strict_date_optional_time'
+          reportURL.includes(
+            `query:(range:(order_date:(format:strict_date_optional_time,gte:now-24h/h,lte:now))))`
           )
         ).to.be(true);
 
@@ -298,6 +298,10 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
         await setupPage();
       });
 
+      afterEach(async () => {
+        await PageObjects.reporting.checkForReportingToasts();
+      });
+
       it('generates a report with data', async () => {
         await PageObjects.discover.loadSavedSearch('Ecommerce Data');
         await retry.try(async () => {