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

Reporting: Fix _index and _id columns in CSV export #96097

Merged
merged 15 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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 @@ -6,13 +6,12 @@
* Side Public License, v 1.
*/

import { Capabilities } from 'kibana/public';
import { getSharingData, showPublicUrlSwitch } from './get_sharing_data';
import { IUiSettingsClient } from 'kibana/public';
import { Capabilities, IUiSettingsClient } from 'kibana/public';
import { IndexPattern } from 'src/plugins/data/public';
import { createSearchSourceMock } from '../../../../data/common/search/search_source/mocks';
import { indexPatternMock } from '../../__mocks__/index_pattern';
import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../../common';
import { IndexPattern } from 'src/plugins/data/public';
import { indexPatternMock } from '../../__mocks__/index_pattern';
import { getSharingData, showPublicUrlSwitch } from './get_sharing_data';

describe('getSharingData', () => {
let mockConfig: IUiSettingsClient;
Expand All @@ -36,6 +35,32 @@ describe('getSharingData', () => {
const result = await getSharingData(searchSourceMock, { columns: [] }, mockConfig);
expect(result).toMatchInlineSnapshot(`
Object {
"columns": Array [],
"searchSource": Object {
"index": "the-index-pattern-id",
"sort": Array [
Object {
"_score": "desc",
},
],
},
}
`);
});

test('returns valid data for sharing when columns are selected', async () => {
const searchSourceMock = createSearchSourceMock({ index: indexPatternMock });
const result = await getSharingData(
searchSourceMock,
{ columns: ['column_a', 'column_b'] },
mockConfig
);
expect(result).toMatchInlineSnapshot(`
Object {
"columns": Array [
"column_a",
"column_b",
],
"searchSource": Object {
"index": "the-index-pattern-id",
"sort": Array [
Expand Down Expand Up @@ -69,16 +94,16 @@ describe('getSharingData', () => {
);
expect(result).toMatchInlineSnapshot(`
Object {
"columns": Array [
"cool-timefield",
"cool-field-1",
"cool-field-2",
"cool-field-3",
"cool-field-4",
"cool-field-5",
"cool-field-6",
],
"searchSource": Object {
"fields": Array [
"cool-timefield",
"cool-field-1",
"cool-field-2",
"cool-field-3",
"cool-field-4",
"cool-field-5",
"cool-field-6",
],
"index": "the-index-pattern-id",
"sort": Array [
Object {
Expand Down Expand Up @@ -120,15 +145,15 @@ describe('getSharingData', () => {
);
expect(result).toMatchInlineSnapshot(`
Object {
"columns": Array [
"cool-field-1",
"cool-field-2",
"cool-field-3",
"cool-field-4",
"cool-field-5",
"cool-field-6",
],
"searchSource": Object {
"fields": Array [
"cool-field-1",
"cool-field-2",
"cool-field-3",
"cool-field-4",
"cool-field-5",
"cool-field-6",
],
"index": "the-index-pattern-id",
"sort": Array [
Object {
Expand Down
24 changes: 11 additions & 13 deletions src/plugins/discover/public/application/helpers/get_sharing_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,16 @@
*/

import type { Capabilities, IUiSettingsClient } from 'kibana/public';
import { ISearchSource, SearchSourceFields } from '../../../../data/common';
import { DOC_HIDE_TIME_COLUMN_SETTING, SORT_DEFAULT_ORDER_SETTING } from '../../../common';
import { getSortForSearchSource } from '../angular/doc_table';
import { ISearchSource } from '../../../../data/common';
import { AppState } from '../angular/discover_state';
import type { SavedSearch, SortOrder } from '../../saved_searches/types';
import { AppState } from '../angular/discover_state';
import { getSortForSearchSource } from '../angular/doc_table';

export interface ISharingData {
columns: string[];
Copy link
Member

Choose a reason for hiding this comment

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

lets make this optional

Copy link
Member

Choose a reason for hiding this comment

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

never mind i see its optional on the report type

searchSource: SearchSourceFields;
}

/**
* Preparing data to share the current state as link or CSV/Report
Expand All @@ -20,13 +25,9 @@ export async function getSharingData(
currentSearchSource: ISearchSource,
state: AppState | SavedSearch,
config: IUiSettingsClient
) {
): Promise<ISharingData> {
const searchSource = currentSearchSource.createCopy();
const index = searchSource.getField('index')!;
const fields = {
fields: searchSource.getField('fields'),
fieldsFromSource: searchSource.getField('fieldsFromSource'),
};

searchSource.setField(
'sort',
Expand All @@ -37,7 +38,7 @@ export async function getSharingData(
searchSource.removeField('aggs');
searchSource.removeField('size');

// fields get re-set to match the saved search columns
// Columns that the user has selected in the saved search
let columns = state.columns || [];

if (columns && columns.length > 0) {
Expand All @@ -50,14 +51,11 @@ export async function getSharingData(
if (timeFieldName && !columns.includes(timeFieldName)) {
columns = [timeFieldName, ...columns];
}

// if columns were selected in the saved search, use them for the searchSource's fields
const fieldsKey = fields.fieldsFromSource ? 'fieldsFromSource' : 'fields';
searchSource.setField(fieldsKey, columns);
}

return {
searchSource: searchSource.getSerializedFields(true),
columns,
};
}

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/discover/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ export function plugin(initializerContext: PluginInitializerContext) {

export { SavedSearch, SavedSearchLoader, createSavedSearchesLoader } from './saved_searches';
export { ISearchEmbeddable, SEARCH_EMBEDDABLE_TYPE, SearchInput } from './application/embeddable';
export { loadSharingDataHelpers } from './shared';
export { ISharingData, loadSharingDataHelpers } from './shared';
export { DISCOVER_APP_URL_GENERATOR, DiscoverUrlGeneratorState } from './url_generator';
2 changes: 2 additions & 0 deletions src/plugins/discover/public/shared/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,5 @@
export async function loadSharingDataHelpers() {
return await import('../application/helpers/get_sharing_data');
}

export { ISharingData } from '../application/helpers/get_sharing_data';
Copy link
Member

Choose a reason for hiding this comment

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

just a quick look, it's late and my local Kibana refuses to bootstrap. could you check the build stats of this PR once it's finished, to be sure there is no significant change because of this? I can review it tomorrow for sure

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure -- nothing actually imports this interface, but Typescript complained without the export

Copy link
Member Author

Choose a reason for hiding this comment

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

@kertal - I noticed that this PR added an extra 25K to the discover page load bundle, which is unexpected. I added fc16f85 and watching for how the bundle sizes changes in CI metrics.

Copy link
Member Author

Choose a reason for hiding this comment

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

fc16f85 brought the discover page load bundle size back down. Thanks for the tip @kertal!

Copy link
Member

Choose a reason for hiding this comment

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

even better, this PR saves a few bytes now!

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ describe('GetCsvReportPanelAction', () => {
let core: any;
let context: any;
let mockLicense$: any;
let mockSearchSource: any;

beforeAll(() => {
if (typeof window.URL.revokeObjectURL === 'undefined') {
Expand Down Expand Up @@ -49,22 +50,19 @@ describe('GetCsvReportPanelAction', () => {
},
} as any;

mockSearchSource = {
createCopy: () => mockSearchSource,
removeField: jest.fn(),
setField: jest.fn(),
getField: jest.fn(),
getSerializedFields: jest.fn().mockImplementation(() => ({})),
};

context = {
embeddable: {
type: 'search',
getSavedSearch: () => {
const searchSource = {
createCopy: () => searchSource,
removeField: jest.fn(),
setField: jest.fn(),
getField: jest.fn().mockImplementation((key: string) => {
if (key === 'index') {
return 'my-test-index-*';
}
}),
getSerializedFields: jest.fn().mockImplementation(() => ({})),
};
return { searchSource };
return { searchSource: mockSearchSource };
},
getTitle: () => `The Dude`,
getInspectorAdapters: () => null,
Expand All @@ -79,6 +77,49 @@ describe('GetCsvReportPanelAction', () => {
} as any;
});

it('translates empty embeddable context into job params', async () => {
const panel = new GetCsvReportPanelAction(core, mockLicense$());

await panel.execute(context);

expect(core.http.post).toHaveBeenCalledWith(
'/api/reporting/v1/generate/immediate/csv_searchsource',
{
body: '{"searchSource":{},"columns":[],"browserTimezone":"America/New_York"}',
}
);
});

it('translates embeddable context into job params', async () => {
// setup
mockSearchSource = {
createCopy: () => mockSearchSource,
removeField: jest.fn(),
setField: jest.fn(),
getField: jest.fn(),
getSerializedFields: jest.fn().mockImplementation(() => ({ testData: 'testDataValue' })),
};
context.embeddable.getSavedSearch = () => {
return {
searchSource: mockSearchSource,
columns: ['column_a', 'column_b'],
};
};

const panel = new GetCsvReportPanelAction(core, mockLicense$());

// test
await panel.execute(context);

expect(core.http.post).toHaveBeenCalledWith(
'/api/reporting/v1/generate/immediate/csv_searchsource',
{
body:
'{"searchSource":{"testData":"testDataValue"},"columns":["column_a","column_b"],"browserTimezone":"America/New_York"}',
}
);
});

it('allows downloading for valid licenses', async () => {
const panel = new GetCsvReportPanelAction(core, mockLicense$());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,23 @@

import { i18n } from '@kbn/i18n';
import moment from 'moment-timezone';
import { CoreSetup } from 'src/core/public';
import {
loadSharingDataHelpers,
import type { CoreSetup } from 'src/core/public';
import type {
ISearchEmbeddable,
ISharingData,
SavedSearch,
SEARCH_EMBEDDABLE_TYPE,
} from '../../../../../src/plugins/discover/public';
import { IEmbeddable, ViewMode } from '../../../../../src/plugins/embeddable/public';
import {
IncompatibleActionError,
UiActionsActionDefinition as ActionDefinition,
} from '../../../../../src/plugins/ui_actions/public';
import { LicensingPluginSetup } from '../../../licensing/public';
loadSharingDataHelpers,
SEARCH_EMBEDDABLE_TYPE,
} from '../../../../../src/plugins/discover/public';
import type { IEmbeddable } from '../../../../../src/plugins/embeddable/public';
import { ViewMode } from '../../../../../src/plugins/embeddable/public';
import type { UiActionsActionDefinition as ActionDefinition } from '../../../../../src/plugins/ui_actions/public';
import { IncompatibleActionError } from '../../../../../src/plugins/ui_actions/public';
import type { LicensingPluginSetup } from '../../../licensing/public';
import { API_GENERATE_IMMEDIATE, CSV_REPORTING_ACTION } from '../../common/constants';
import { JobParamsDownloadCSV } from '../../server/export_types/csv_searchsource_immediate/types';
import type { JobParamsDownloadCSV } from '../../server/export_types/csv_searchsource_immediate/types';
import { checkLicense } from '../lib/license_check';

function isSavedSearchEmbeddable(
Expand Down Expand Up @@ -62,16 +64,16 @@ export class GetCsvReportPanelAction implements ActionDefinition<ActionContext>
});
}

public async getSearchSource(savedSearch: SavedSearch, embeddable: ISearchEmbeddable) {
public async getSearchSource(
savedSearch: SavedSearch,
embeddable: ISearchEmbeddable
): Promise<ISharingData> {
const { getSharingData } = await loadSharingDataHelpers();
const searchSource = savedSearch.searchSource.createCopy();
const { searchSource: serializedSearchSource } = await getSharingData(
searchSource,
return await getSharingData(
savedSearch.searchSource,
savedSearch, // TODO: get unsaved state (using embeddale.searchScope): https://github.com/elastic/kibana/issues/43977
this.core.uiSettings
);

return serializedSearchSource;
}

public isCompatible = async (context: ActionContext) => {
Expand All @@ -96,12 +98,13 @@ export class GetCsvReportPanelAction implements ActionDefinition<ActionContext>
}

const savedSearch = embeddable.getSavedSearch();
const searchSource = await this.getSearchSource(savedSearch, embeddable);
const { columns, searchSource } = await this.getSearchSource(savedSearch, embeddable);

const kibanaTimezone = this.core.uiSettings.get('dateFormat:tz');
const browserTimezone = kibanaTimezone === 'Browser' ? moment.tz.guess() : kibanaTimezone;
const immediateJobParams: JobParamsDownloadCSV = {
searchSource,
columns,
browserTimezone,
title: savedSearch.title,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
import { i18n } from '@kbn/i18n';
import moment from 'moment-timezone';
import React from 'react';
import { IUiSettingsClient, ToastsSetup } from 'src/core/public';
import { ShareContext } from '../../../../../src/plugins/share/public';
import { LicensingPluginSetup } from '../../../licensing/public';
import type { IUiSettingsClient, ToastsSetup } from 'src/core/public';
import type { SearchSourceFields } from 'src/plugins/data/common';
import type { ShareContext } from '../../../../../src/plugins/share/public';
import type { LicensingPluginSetup } from '../../../licensing/public';
import { CSV_JOB_TYPE } from '../../common/constants';
import { JobParamsCSV } from '../../server/export_types/csv_searchsource/types';
import type { JobParamsCSV } from '../../server/export_types/csv_searchsource/types';
import { ReportingPanelContent } from '../components/reporting_panel_content_lazy';
import { checkLicense } from '../lib/license_check';
import { ReportingAPIClient } from '../lib/reporting_api_client';
import type { ReportingAPIClient } from '../lib/reporting_api_client';

interface ReportingProvider {
apiClient: ReportingAPIClient;
Expand Down Expand Up @@ -65,7 +66,8 @@ export const csvReportingProvider = ({
browserTimezone,
title: sharingData.title as string,
objectType,
searchSource: sharingData.searchSource,
searchSource: sharingData.searchSource as SearchSourceFields,
columns: sharingData.columns as string[] | undefined,
};

const getJobParams = () => jobParams;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
import { i18n } from '@kbn/i18n';
import moment from 'moment-timezone';
import React from 'react';
import { IUiSettingsClient, ToastsSetup } from 'src/core/public';
import { ShareContext } from '../../../../../src/plugins/share/public';
import { LicensingPluginSetup } from '../../../licensing/public';
import { LayoutParams } from '../../common/types';
import { JobParamsPNG } from '../../server/export_types/png/types';
import { JobParamsPDF } from '../../server/export_types/printable_pdf/types';
import type { IUiSettingsClient, ToastsSetup } from 'src/core/public';
import type { ShareContext } from '../../../../../src/plugins/share/public';
import type { LicensingPluginSetup } from '../../../licensing/public';
import type { LayoutParams } from '../../common/types';
import type { JobParamsPNG } from '../../server/export_types/png/types';
import type { JobParamsPDF } from '../../server/export_types/printable_pdf/types';
import { ScreenCapturePanelContent } from '../components/screen_capture_panel_content_lazy';
import { checkLicense } from '../lib/license_check';
import { ReportingAPIClient } from '../lib/reporting_api_client';
import type { ReportingAPIClient } from '../lib/reporting_api_client';

interface ReportingPDFPNGProvider {
apiClient: ReportingAPIClient;
Expand Down
Loading