Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Enable report sharing #153429

Merged
merged 35 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
76b5ebc
:sparkles: Enable reporting sharing for Lens
dej611 Mar 22, 2023
7fbac81
:sparkles: Extends reporting to support lens_visualization
dej611 Mar 22, 2023
7b49555
:recycle: Migrate objectType to "lens"
dej611 Mar 22, 2023
dc40345
:recycle: Minor refactors
dej611 Mar 22, 2023
f408271
:bug: Fix bug when shareURL priviliges are not granted
dej611 Mar 23, 2023
ef9246e
:recycle: Add priviledges check
dej611 Mar 23, 2023
9c21584
:white_check_mark: Add functional tests
dej611 Mar 23, 2023
4d885eb
Merge branch 'main' into feature/130438
dej611 Mar 23, 2023
b9f06e7
:camera_flash: Update snapshots
dej611 Mar 23, 2023
b1e0f7c
Merge branch 'feature/130438' of github.com:dej611/kibana into featur…
dej611 Mar 23, 2023
f638c8f
Merge branch 'main' into feature/130438
stratoula Mar 24, 2023
7422a61
:recycle: Move from unsaved to timestamped title
dej611 Mar 24, 2023
429d5f5
Merge branch 'feature/130438' of github.com:dej611/kibana into featur…
dej611 Mar 24, 2023
a359e4b
:bug: Fix i18n id
dej611 Mar 24, 2023
e556fe1
:fire: Remove unused translations
dej611 Mar 24, 2023
85a42f2
Merge branch 'main' into feature/130438
stratoula Mar 27, 2023
70c9ed8
Merge branch 'main' into feature/130438
tsullivan Mar 27, 2023
2228506
Merge branch 'main' into feature/130438
dej611 Mar 28, 2023
22dfdb5
Merge remote-tracking branch 'upstream/main' into feature/130438
dej611 Mar 29, 2023
daad4c7
:bug: Fix issue with partial state and SO id provided
dej611 Mar 29, 2023
fa61289
:ok_hand: Improved reporting URL for saved visualizations
dej611 Mar 29, 2023
2b3d9e9
:sparkles: Add error handling for reporting
dej611 Mar 29, 2023
d73df1e
:white_check_mark: Add functional test for reporting URL
dej611 Mar 29, 2023
a1b3006
Update x-pack/plugins/lens/public/editor_frame_service/editor_frame/w…
dej611 Mar 29, 2023
5e711ca
Merge remote-tracking branch 'upstream/main' into feature/130438
dej611 Mar 30, 2023
5040e20
Merge branch 'feature/130438' of github.com:dej611/kibana into featur…
dej611 Mar 30, 2023
82c18f1
:white_check_mark: Fix unit test
dej611 Mar 30, 2023
719afe2
Merge branch 'main' into feature/130438
dej611 Mar 30, 2023
39ca589
:white_check_mark: Fix test titles
dej611 Mar 31, 2023
f7b8943
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Mar 31, 2023
e8c4e5f
:bug: Fix first rendering ghost issue
dej611 Mar 31, 2023
39dcb2e
Merge branch 'feature/130438' of github.com:dej611/kibana into featur…
dej611 Mar 31, 2023
23286a3
Merge remote-tracking branch 'upstream/main' into feature/130438
dej611 Apr 4, 2023
fe0d606
:bug: Fix reporting for dirty state
dej611 Apr 4, 2023
a414421
Merge branch 'main' into feature/130438
stratoula Apr 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const downloadCsvShareProvider = ({
formatFactoryFn,
}: DownloadPanelShareOpts): ShareMenuProvider => {
const getShareMenuItems = ({ objectType, sharingData, onClose }: ShareContext) => {
if ('lens_visualization' !== objectType) {
if ('lens' !== objectType) {
return [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ describe('application-level user messages', () => {
activeDatasource: {
checkIntegrity: jest.fn(() => ['missing_pattern']),
} as unknown as Datasource,
activeDatasourceState: { state: {} },
activeDatasourceState: { isLoading: false, state: {} },
core: createCoreStartWithPermissions(),
...irrelevantProps,
})
Expand All @@ -166,7 +166,7 @@ describe('application-level user messages', () => {
activeDatasource: {
checkIntegrity: jest.fn(() => ['missing_pattern']),
} as unknown as Datasource,
activeDatasourceState: { state: {} },
activeDatasourceState: { isLoading: false, state: {} },
// user can go to management, but indexPatterns management is not accessible
core: createCoreStartWithPermissions({
navLinks: { management: true },
Expand All @@ -188,7 +188,7 @@ describe('application-level user messages', () => {
activeDatasource: {
checkIntegrity: jest.fn(() => ['missing_pattern']),
} as unknown as Datasource,
activeDatasourceState: { state: {} },
activeDatasourceState: { isLoading: false, state: {} },
// user can't go to management at all
core: createCoreStartWithPermissions({
navLinks: { management: false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const getApplicationUserMessages = ({
visualization: VisualizationState | undefined;
visualizationMap: VisualizationMap;
activeDatasource: Datasource | null | undefined;
activeDatasourceState: { state: unknown } | null;
activeDatasourceState: { isLoading: boolean; state: unknown } | null;
dataViews: DataViewsState;
core: CoreStart;
}): UserMessage[] => {
Expand Down
41 changes: 28 additions & 13 deletions x-pack/plugins/lens/public/app_plugin/lens_top_nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import { getEsQueryConfig } from '@kbn/data-plugin/public';
import type { DataView } from '@kbn/data-views-plugin/public';
import { useKibana } from '@kbn/kibana-react-plugin/public';
import { DataViewPickerProps } from '@kbn/unified-search-plugin/public';
import moment from 'moment';
import { LENS_APP_LOCATOR } from '../../common/locator/locator';
import { ENABLE_SQL } from '../../common/constants';
import { LensAppServices, LensTopNavActions, LensTopNavMenuProps } from './types';
import { toggleSettingsMenuOpen } from './settings_menu';
Expand Down Expand Up @@ -457,8 +459,9 @@ export const LensTopNavMenu = ({
isSaveable && application.capabilities.dashboard?.showWriteControls
);

const unsavedTitle = i18n.translate('xpack.lens.app.unsavedFilename', {
defaultMessage: 'unsaved',
const defaultLensTitle = i18n.translate('xpack.lens.app.share.defaultDashboardTitle', {
defaultMessage: 'Lens Visualization [{date}]',
values: { date: moment().toISOString(true) },
});
const additionalMenuEntries = useMemo(() => {
if (!visualization.activeId) return undefined;
Expand Down Expand Up @@ -574,13 +577,12 @@ export const LensTopNavMenu = ({
if (!share) {
return;
}
const sharingData = {
activeData,
csvEnabled,
title: title || unsavedTitle,
};

const { shareableUrl, savedObjectURL } = await getShareURL(
const {
shareableUrl,
savedObjectURL,
reportingLocatorParams: locatorParams,
} = await getShareURL(
shortUrlService,
{ application, data },
{
Expand All @@ -593,8 +595,20 @@ export const LensTopNavMenu = ({
visualization,
currentDoc,
adHocDataViews: adHocDataViews.map((dataView) => dataView.toSpec()),
}
},
shareUrlEnabled,
isCurrentStateDirty
);
const sharingData = {
activeData,
csvEnabled,
reportingDisabled: !csvEnabled,
title: title || defaultLensTitle,
locatorParams: {
id: LENS_APP_LOCATOR,
params: locatorParams,
},
};

share.toggleShareContextMenu({
anchorElement,
Expand All @@ -603,7 +617,7 @@ export const LensTopNavMenu = ({
shareableUrl: shareableUrl || '',
shareableUrlForSavedObject: savedObjectURL.href,
objectId: currentDoc?.savedObjectId,
objectType: 'lens_visualization',
objectType: 'lens',
objectTypeTitle: i18n.translate('xpack.lens.app.share.panelTitle', {
defaultMessage: 'visualization',
}),
Expand Down Expand Up @@ -735,7 +749,6 @@ export const LensTopNavMenu = ({
initialContextIsEmbedded,
activeData,
isSaveable,
shortUrlService,
application,
getIsByValueMode,
savingToLibraryPermitted,
Expand All @@ -746,7 +759,7 @@ export const LensTopNavMenu = ({
lensInspector,
title,
share,
unsavedTitle,
shortUrlService,
data,
filters,
query,
Expand All @@ -756,6 +769,8 @@ export const LensTopNavMenu = ({
visualizationMap,
visualization,
currentDoc,
adHocDataViews,
defaultLensTitle,
isCurrentStateDirty,
onAppLeave,
runSave,
Expand All @@ -770,7 +785,6 @@ export const LensTopNavMenu = ({
isOnTextBasedMode,
lensStore,
theme$,
adHocDataViews,
]);

const onQuerySubmitWrapped = useCallback(
Expand Down Expand Up @@ -1072,6 +1086,7 @@ export const LensTopNavMenu = ({
screenTitle={'lens'}
appName={'lens'}
displayStyle="detached"
className="hide-for-sharing"
/>
);
};
37 changes: 30 additions & 7 deletions x-pack/plugins/lens/public/app_plugin/share_action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ function getShareURLForSavedObject(
);
}

function getShortShareableURL(
shortUrlService: (params: LensAppLocatorParams) => Promise<string>,
export function getLocatorParams(
data: LensAppServices['data'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works well for unsaved content in Lens, but when there is a saved Lens object loaded without any unsaved changes, the locator params should use the savedObjectId field of LensAppLocatorParams. This will enable a common use case, where users copy a POST URL from the UI, and use it as a template to programmatically generate reports. Having the ability to replace the saved object ID as-needed is indispensable for this use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the way URLs are generate for saved visualizations and unsaved, the URL format are pretty different (explicit vs short URLs).
Due to this difference the shareableUrlForSavedObject property has been added and used also for the reporting when avaialble: https://github.com/elastic/kibana/pull/153429/files#diff-be49f342798d67aec36153234bac3806d1b22566c1cd71a84b95be7c77157cc4R121

Copy link
Member

@tsullivan tsullivan Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I meant for my comment to be specific only to the locatorParams, and not the shareableUrl. The locator params are what actually gets used as the app state for the Reporting API.

To clarify: Let's say I opened from the Visualize Library, and made no changes, and then click the "Copy POST URL" button. The result looks like:
http://localhost:5601/api/reporting/generate/pngV2?jobParams=%28browserTimezone%3AAmerica%2FPhoenix%2Clayout%3A%28dimensions%3A%28height%3A593%2Cwidth%3A1171.7421875%29%2Cid%3Apreserve_layout%29%2ClocatorParams%3A%28id%3ALENS_APP_LOCATOR%2Cparams%3A%28activeDatasourceId%3AformBased%2CdataViewSpecs%3A%21%28%29%2CdatasourceStates%3A%28formBased%3A%28isLoading%3A%21f%2Cstate%3A%28currentIndexPatternId%3Aff959d40-b880-11e8-a6d9-e546fe2bba5f%2Clayers%3A%28%2797c63ea6-9305-4755-97d1-0f26817c6f9a%27%3A%28columnOrder%3A%21%28%279f61a7df-198e-4754-b34c-81ed544136ba%27%2Cebcb19af-0900-4439-949f-d8cd9bccde19%2C%275575214b-7f21-4b6c-8bc1-34433c6a0c58%27%29%2Ccolumns%3A%28%275575214b-7f21-4b6c-8bc1-34433c6a0c58%27%3A%28dataType%3Anumber%2CisBucketed%3A%21f%2Clabel%3A%27Count%20of%20records%27%2CoperationType%3Acount%2Cscale%3Aratio%2CsourceField%3A___records___%29%2C%279f61a7df-198e-4754-b34c-81ed544136ba%27%3A%28dataType%3Astring%2CisBucketed%3A%21t%2Clabel%3A%27Top%20values%20of%20category.keyword%27%2CoperationType%3Aterms%2Cparams%3A%28missingBucket%3A%21f%2CorderBy%3A%28columnId%3A%275575214b-7f21-4b6c-8bc1-34433c6a0c58%27%2Ctype%3Acolumn%29%2CorderDirection%3Adesc%2CotherBucket%3A%21t%2CparentFormat%3A%28id%3Aterms%29%2Csize%3A10%29%2Cscale%3Aordinal%2CsourceField%3Acategory.keyword%29%2Cebcb19af-0900-4439-949f-d8cd9bccde19%3A%28dataType%3Adate%2CisBucketed%3A%21t%2Clabel%3Aorder_date%2CoperationType%3Adate_histogram%2Cparams%3A%28includeEmptyRows%3A%21t%2Cinterval%3A%271d%27%29%2Cscale%3Ainterval%2CsourceField%3Aorder_date%29%29%2CincompleteColumns%3A%28%29%2CindexPatternId%3Aff959d40-b880-11e8-a6d9-e546fe2bba5f%29%29%29%29%29%2Cfilters%3A%21%28%29%2Cquery%3A%28language%3Akuery%2Cquery%3A%27%27%29%2Creferences%3A%21%28%28id%3Aff959d40-b880-11e8-a6d9-e546fe2bba5f%2Cname%3Aindexpattern-datasource-layer-97c63ea6-9305-4755-97d1-0f26817c6f9a%2Ctype%3Aindex-pattern%29%29%2CresolvedDateRange%3A%28fromDate%3A%272023-03-21T19%3A16%3A12.417Z%27%2CtoDate%3A%272023-03-28T19%3A16%3A12.417Z%27%29%2CsearchSessionId%3A%2743960930-60e1-42e3-a611-c6ae4a3af63c%27%2Cvisualization%3A%28activeId%3AlnsXY%2Cstate%3A%28axisTitlesVisibilitySettings%3A%28x%3A%21f%2CyLeft%3A%21f%2CyRight%3A%21t%29%2CfittingFunction%3ANone%2CgridlinesVisibilitySettings%3A%28x%3A%21t%2CyLeft%3A%21t%2CyRight%3A%21t%29%2Clayers%3A%21%28%28accessors%3A%21%28%275575214b-7f21-4b6c-8bc1-34433c6a0c58%27%29%2ClayerId%3A%2797c63ea6-9305-4755-97d1-0f26817c6f9a%27%2ClayerType%3Adata%2Cposition%3Atop%2CseriesType%3Abar_percentage_stacked%2CshowGridlines%3A%21f%2CsplitAccessor%3A%279f61a7df-198e-4754-b34c-81ed544136ba%27%2CxAccessor%3Aebcb19af-0900-4439-949f-d8cd9bccde19%29%29%2Clegend%3A%28isVisible%3A%21t%2ClegendSize%3Aauto%2Cposition%3Aright%29%2CpreferredSeriesType%3Abar_percentage_stacked%2CtickLabelsVisibilitySettings%3A%28x%3A%21t%2CyLeft%3A%21t%2CyRight%3A%21t%29%2CvalueLabels%3Ashow%2CyLeftExtent%3A%28mode%3Afull%29%2CyRightExtent%3A%28mode%3Afull%29%29%29%29%29%2CobjectType%3Alens%2Ctitle%3A%27Report%20Test%27%2Cversion%3A%278.8.0%27%29

After decoding the query string contents and parsing the rison into JSON, the locator params come out to:

  "locatorParams": {
    "id": "LENS_APP_LOCATOR",
    "params": {
      "activeDatasourceId": "formBased",
      "dataViewSpecs": [],
      "datasourceStates": {
        "formBased": {
          "isLoading": false,
          "state": {
            "currentIndexPatternId": "ff959d40-b880-11e8-a6d9-e546fe2bba5f",
            "layers": {
              "97c63ea6-9305-4755-97d1-0f26817c6f9a": {
                "columnOrder": [
                  "9f61a7df-198e-4754-b34c-81ed544136ba",
                  "ebcb19af-0900-4439-949f-d8cd9bccde19",
                  "5575214b-7f21-4b6c-8bc1-34433c6a0c58"
                ],
                "columns": {
                  "5575214b-7f21-4b6c-8bc1-34433c6a0c58": {
                    "dataType": "number",
                    "isBucketed": false,
                    "label": "Count of records",
                    "operationType": "count",
                    "scale": "ratio",
                    "sourceField": "___records___"
                  },
                  "9f61a7df-198e-4754-b34c-81ed544136ba": {
                    "dataType": "string",
                    "isBucketed": true,
                    "label": "Top values of category.keyword",
                    "operationType": "terms",
                    "params": {
                      "missingBucket": false,
                      "orderBy": {
                        "columnId": "5575214b-7f21-4b6c-8bc1-34433c6a0c58",
                        "type": "column"
                      },
                      "orderDirection": "desc",
                      "otherBucket": true,
                      "parentFormat": {
                        "id": "terms"
                      },
                      "size": 10
                    },
                    "scale": "ordinal",
                    "sourceField": "category.keyword"
                  },
                  "ebcb19af-0900-4439-949f-d8cd9bccde19": {
                    "dataType": "date",
                    "isBucketed": true,
                    "label": "order_date",
                    "operationType": "date_histogram",
                    "params": {
                      "includeEmptyRows": true,
                      "interval": "1d"
                    },
                    "scale": "interval",
                    "sourceField": "order_date"
                  }
                },
                "incompleteColumns": {},
                "indexPatternId": "ff959d40-b880-11e8-a6d9-e546fe2bba5f"
              }
            }
          }
        }
      },
      "filters": [],
      "query": {
        "language": "kuery",
        "query": ""
      },
      "references": [
        {
          "id": "ff959d40-b880-11e8-a6d9-e546fe2bba5f",
          "name": "indexpattern-datasource-layer-97c63ea6-9305-4755-97d1-0f26817c6f9a",
          "type": "index-pattern"
        }
      ],
      "resolvedDateRange": {
        "fromDate": "2023-03-21T19:16:12.417Z",
        "toDate": "2023-03-28T19:16:12.417Z"
      },
      "searchSessionId": "43960930-60e1-42e3-a611-c6ae4a3af63c",
      "visualization": {
        "activeId": "lnsXY",
        "state": {
          "axisTitlesVisibilitySettings": {
            "x": false,
            "yLeft": false,
            "yRight": true
          },
          "fittingFunction": "None",
          "gridlinesVisibilitySettings": {
            "x": true,
            "yLeft": true,
            "yRight": true
          },
          "layers": [
            {
              "accessors": [
                "5575214b-7f21-4b6c-8bc1-34433c6a0c58"
              ],
              "layerId": "97c63ea6-9305-4755-97d1-0f26817c6f9a",
              "layerType": "data",
              "position": "top",
              "seriesType": "bar_percentage_stacked",
              "showGridlines": false,
              "splitAccessor": "9f61a7df-198e-4754-b34c-81ed544136ba",
              "xAccessor": "ebcb19af-0900-4439-949f-d8cd9bccde19"
            }
          ],
          "legend": {
            "isVisible": true,
            "legendSize": "auto",
            "position": "right"
          },
          "preferredSeriesType": "bar_percentage_stacked",
          "tickLabelsVisibilitySettings": {
            "x": true,
            "yLeft": true,
            "yRight": true
          },
          "valueLabels": "show",
          "yLeftExtent": {
            "mode": "full"
          },
          "yRightExtent": {
            "mode": "full"
          }
        }
      }
    }

For a saved Lens with no changes, we need a more optimized form of locatorParams, since they go into a URL that is often handled outside of Kibana. The data in the locatorParams should be simplified and reflect the saved object ID backing the Lens, plus the global time range since that isn't part of the saved object:

  "locatorParams": {
    "id": "LENS_APP_LOCATOR",
    "params": {
      "savedObjectId": "f9634c20-cd9c-11ed-a8c8-4bb7d8009a23",
      "resolvedDateRange": {
        "fromDate": "2023-03-21T19:16:12.417Z",
        "toDate": "2023-03-28T19:16:12.417Z"
      },
    }

This shortens the URL which is great for several reasons:

  • Long URLs cause problems in Watcher and with many proxies
  • In many use cases, users deploy a custom script can get a report from any saved object in a generic way, just by plugging in a different saved object ID into the POST URL. They can use other Kibana APIs to get a list of all saved object IDs they're interested in.
  • Users have told us, when they have a Watch generate reports on a saved object, when they edit the saved object they expect the new reports to reflect the changes.

For all these reasons, we should avoid having the locatorParams contain just the full internal state of what is stored in the saved object. We should make it as referential as it can be, by using the savedObjectId field of the LensAppLocatorParams

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fa61289

{
filters,
Expand All @@ -57,7 +56,9 @@ function getShortShareableURL(
visualizationMap,
visualization,
adHocDataViews,
}: ShareableConfiguration
currentDoc,
}: ShareableConfiguration,
isDirty: boolean
) {
const references = extractReferencesFromState({
activeDatasources: Object.keys(datasourceStates).reduce(
Expand All @@ -80,7 +81,7 @@ function getShortShareableURL(
const serializableDatasourceStates = datasourceStates as LensAppState['datasourceStates'] &
SerializableRecord;

return shortUrlService({
const snapshotParams = {
filters,
query,
resolvedDateRange: getResolvedDateRange(data.query.timefilter.timefilter),
Expand All @@ -90,16 +91,38 @@ function getShortShareableURL(
searchSessionId: data.search.session.getSessionId(),
references,
dataViewSpecs: adHocDataViews,
});
};

return {
shareURL: snapshotParams,
// for reporting use the shorten version when available
reporting:
currentDoc?.savedObjectId && !isDirty
? {
filters,
query,
resolvedDateRange: getResolvedDateRange(data.query.timefilter.timefilter),
savedObjectId: currentDoc?.savedObjectId,
}
: snapshotParams,
};
}

export async function getShareURL(
shortUrlService: (params: LensAppLocatorParams) => Promise<string>,
services: Pick<LensAppServices, 'application' | 'data'>,
configuration: ShareableConfiguration
configuration: ShareableConfiguration,
shareUrlEnabled: boolean,
isDirty: boolean
) {
const { shareURL: locatorParams, reporting: reportingLocatorParams } = getLocatorParams(
services.data,
configuration,
isDirty
);
return {
shareableUrl: await getShortShareableURL(shortUrlService, services.data, configuration),
shareableUrl: await (shareUrlEnabled ? shortUrlService(locatorParams) : undefined),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing this is the code behind the note in the description that a short URL is created every time that a report is requested, but I think we should avoid this.

For Reporting purposes, the locatorParams is what is actually used for the V2 export types: https://github.com/elastic/kibana/blob/1a3ea3a/x-pack/plugins/reporting/public/share_context_menu/register_pdf_png_reporting.tsx#L35-L41, so the short URL is not even used when the report is executing. Unfortunately it is part of the ShowShareMenuOptions in the sharing plugin :\

The dashboard app uses setStateToKbnUrl: https://github.com/elastic/kibana/blob/503b466/src/plugins/dashboard/public/dashboard_app/top_nav/share/show_share_modal.tsx#L164. Is that an alternative option here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not.
In the Lens case the locator is also registered on the server side to make use of the new shortURL service, which is not possible with the setStateToKbnUrl (as it also uses the client storage).
Currently when the user clicks on the Share menu a shortURL is already generated, even if the user won't click on the button. This is not great and perhaps can be improved, but it is completely independent from reporting and depends on the fact that the UrlPanel wants a static URL string for the creation. I can log a separate issue on this to lazy generate the short URL only when the user clicks on the Generate button, but that has an important impact also on the URL share plugin.
Said that, as the short URL is already available, why not use it for reporting?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Said that, as the short URL is already available, why not use it for reporting?

URLs are not used for reporting in the V2 export types. Eventually, the code for V1 types will be removed as usage of those APIs goes down, and all available export types will use locators.

I agree with you the fact that a shortURL being created is independent of the reporting integration. I also think the Sharing panel should not require a shareableUrl, which is another separate issue. cc @vadimkibana

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in fa61289

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added also a functional test to verify the correctness of the URL generated by reporting in this case ( d73df1e )

savedObjectURL: getShareURLForSavedObject(services, configuration.currentDoc),
reportingLocatorParams,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export function FrameLayout(props: FrameLayoutProps) {
aria-labelledby="lns_ChartTitle"
>
<section
className={classNames('lnsFrameLayout__sidebar lnsFrameLayout__sidebar--left', {})}
className={'lnsFrameLayout__sidebar lnsFrameLayout__sidebar--left hide-for-sharing'}
aria-labelledby="dataPanelId"
>
<EuiScreenReaderOnly>
Expand All @@ -79,12 +79,18 @@ export function FrameLayout(props: FrameLayoutProps) {
</h2>
</EuiScreenReaderOnly>
{props.workspacePanel}
<div className="lnsFrameLayout__suggestionPanel">{props.suggestionsPanel}</div>
<div className="lnsFrameLayout__suggestionPanel hide-for-sharing">
{props.suggestionsPanel}
</div>
</section>
<section
className={classNames('lnsFrameLayout__sidebar lnsFrameLayout__sidebar--right', {
'lnsFrameLayout__sidebar-isFullscreen': isFullscreen,
})}
className={classNames(
'lnsFrameLayout__sidebar lnsFrameLayout__sidebar--right',
'hide-for-sharing',
{
'lnsFrameLayout__sidebar-isFullscreen': isFullscreen,
}
)}
aria-labelledby="configPanel"
>
<EuiScreenReaderOnly>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,10 +404,14 @@ export async function persistedStateToExpression(

export function getMissingIndexPattern(
currentDatasource: Datasource | null | undefined,
currentDatasourceState: { state: unknown } | null,
currentDatasourceState: { isLoading: boolean; state: unknown } | null,
indexPatterns: IndexPatternMap
) {
if (currentDatasourceState?.state == null || currentDatasource == null) {
if (
currentDatasourceState?.isLoading ||
currentDatasourceState?.state == null ||
currentDatasource == null
) {
return [];
}
const missingIds = currentDatasource.checkIntegrity(currentDatasourceState.state, indexPatterns);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ const defaultProps = {
getSuggestionForField: () => undefined,
lensInspector: getLensInspectorService(inspectorPluginMock.createStartContract()),
toggleFullscreen: jest.fn(),
getUserMessages: () => [],
addUserMessages: () => () => {},
getUserMessages: jest.fn(() => []),
addUserMessages: jest.fn(() => () => {}),
};

const toExpr = (
Expand Down Expand Up @@ -728,10 +728,10 @@ describe('workspace_panel', () => {
const mounted = await mountWithProvider(
<WorkspacePanel
{...defaultProps}
getUserMessages={getUserMessages}
datasourceMap={{
testDatasource: mockDatasource,
}}
getUserMessages={getUserMessages}
/>
);
instance = mounted.instance;
Expand All @@ -752,11 +752,12 @@ describe('workspace_panel', () => {
// but not yet applied their changes

let userMessages = [] as UserMessage[];
const getUserMessageFn = jest.fn(() => userMessages);

const mounted = await mountWithProvider(
<WorkspacePanel
{...defaultProps}
getUserMessages={() => userMessages}
getUserMessages={getUserMessageFn}
datasourceMap={{
testDatasource: mockDatasource,
}}
Expand Down
Loading