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] add version to all export types job params #106137

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
de232ba
add version to csv params
tsullivan Jul 19, 2021
915fa9c
fix ts
tsullivan Jul 19, 2021
1482d03
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 20, 2021
141a174
fix api tests
tsullivan Jul 20, 2021
dfa0e56
use kibana version from packageInfo
tsullivan Jul 20, 2021
f80097e
use kibana version from packageInfo
tsullivan Jul 20, 2021
9b592f6
clean up ide warnings
tsullivan Jul 20, 2021
b37a238
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 21, 2021
02688b7
utility to log and set a default params version
tsullivan Jul 21, 2021
793618c
fix baseparams ts
tsullivan Jul 21, 2021
96d0dff
update snapshot
tsullivan Jul 21, 2021
8ff2152
check version in enqueue job
tsullivan Jul 21, 2021
27f1181
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 21, 2021
732c891
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 22, 2021
8d59bd4
add temporary ts-ignore for canvas
tsullivan Jul 22, 2021
a44cf64
clarify comment
tsullivan Jul 22, 2021
9c522d4
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 22, 2021
bc1a8ad
fix hardcoded version in png_pdf_panel
tsullivan Jul 22, 2021
86ef317
clarify the UNVERSIONED_VERSION variable with a comment
tsullivan Jul 22, 2021
cd65714
fix canvas jest test
tsullivan Jul 22, 2021
ce87467
fix ts in example app
tsullivan Jul 26, 2021
e11a0a0
Merge branch 'master' into reporting/csv-searchsource-version-param
kibanamachine Jul 26, 2021
fe67dc6
fix types
tsullivan Jul 26, 2021
9cde23f
Merge branch 'master' into reporting/csv-searchsource-version-param
kibanamachine Jul 26, 2021
8f37970
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 27, 2021
839b01e
send version param to canvas util for job params
tsullivan Jul 27, 2021
79c4507
update jest snapshot
tsullivan Jul 27, 2021
061ef07
Update utils.test.ts
tsullivan Jul 27, 2021
685eaec
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 28, 2021
1af338f
fix snapshot
tsullivan Jul 28, 2021
cfb7f12
remove browserTimezone and version from integration boilerplate
tsullivan Jul 28, 2021
dd529e8
wip ensure version is always populated in job params inside of the se…
tsullivan Jul 28, 2021
c8e140c
wip2
tsullivan Jul 28, 2021
a90a701
wip3
tsullivan Jul 28, 2021
b767a75
wip4
tsullivan Jul 28, 2021
4024f9c
wip5
tsullivan Jul 28, 2021
ed3f8ae
wip6
tsullivan Jul 28, 2021
e8185d7
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 28, 2021
23409f4
update note
tsullivan Jul 28, 2021
31349c1
update example plugin
tsullivan Jul 28, 2021
d9536b4
wip7
tsullivan Jul 28, 2021
472ae85
improve tests
tsullivan Jul 28, 2021
43bdd21
fix dynamic job params
tsullivan Jul 28, 2021
64f54f5
better testing
tsullivan Jul 28, 2021
7795958
improve enqueue_job test
tsullivan Jul 28, 2021
56f0393
more tests
tsullivan Jul 29, 2021
c24c44e
fix types
tsullivan Jul 29, 2021
3f4a582
fix types
tsullivan Jul 29, 2021
dcfda41
fix example ts
tsullivan Jul 29, 2021
8ca7cfc
simplify props
tsullivan Jul 29, 2021
0dca459
fix test
tsullivan Jul 29, 2021
f753f76
--wip-- [skip ci]
tsullivan Jul 29, 2021
300ba45
consolidate baseparams back into one interface
tsullivan Jul 29, 2021
f4710b7
Merge branch 'master' into reporting/csv-searchsource-version-param
kibanamachine Jul 29, 2021
0c85dab
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 29, 2021
a6e34b0
fix rison encoding of apiClient param
tsullivan Jul 29, 2021
87dc4ba
clean up
tsullivan Jul 29, 2021
d74cc9e
reorganize imports
tsullivan Jul 29, 2021
1c96876
back out functional change
tsullivan Jul 29, 2021
47329a1
Merge branch 'master' into reporting/csv-searchsource-version-param
tsullivan Jul 29, 2021
dd06b40
fix 400 error in download csv
tsullivan Jul 29, 2021
abdd0e6
Merge branch 'master' into reporting/csv-searchsource-version-param
kibanamachine Aug 2, 2021
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
4 changes: 2 additions & 2 deletions x-pack/examples/reporting_example/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
import React from 'react';
import ReactDOM from 'react-dom';
import { AppMountParameters, CoreStart } from '../../../../src/core/public';
import { SetupDeps, StartDeps } from './types';
import { ReportingExampleApp } from './components/app';
import { SetupDeps, StartDeps } from './types';

export const renderApp = (
coreStart: CoreStart,
deps: Omit<StartDeps & SetupDeps, 'developerExamples'>,
{ appBasePath, element }: AppMountParameters
{ appBasePath, element }: AppMountParameters // FIXME: appBasePath is deprecated
Copy link
Member

Choose a reason for hiding this comment

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

worth opening an issue for this, adding a blocker label to it and a version in which the appBasePath will be removed

Copy link
Member Author

Choose a reason for hiding this comment

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

) => {
ReactDOM.render(<ReportingExampleApp basename={appBasePath} {...coreStart} {...deps} />, element);

Expand Down
14 changes: 3 additions & 11 deletions x-pack/examples/reporting_example/public/components/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,10 @@ import { BrowserRouter as Router } from 'react-router-dom';
import * as Rx from 'rxjs';
import { takeWhile } from 'rxjs/operators';
import { ScreenshotModePluginSetup } from 'src/plugins/screenshot_mode/public';
import { CoreStart } from '../../../../../src/core/public';
import { NavigationPublicPluginStart } from '../../../../../src/plugins/navigation/public';
import { constants, ReportingStart } from '../../../../../x-pack/plugins/reporting/public';
import { JobParamsPDF } from '../../../../plugins/reporting/server/export_types/printable_pdf/types';

interface ReportingExampleAppDeps {
interface ReportingExampleAppProps {
basename: string;
notifications: CoreStart['notifications'];
http: CoreStart['http'];
navigation: NavigationPublicPluginStart;
reporting: ReportingStart;
screenshotMode: ScreenshotModePluginSetup;
}
Expand All @@ -46,11 +40,9 @@ const sourceLogos = ['Beats', 'Cloud', 'Logging', 'Kibana'];

export const ReportingExampleApp = ({
basename,
notifications,
http,
reporting,
screenshotMode,
}: ReportingExampleAppDeps) => {
}: ReportingExampleAppProps) => {
const { getDefaultLayoutSelectors } = reporting;

// Context Menu
Expand All @@ -74,7 +66,7 @@ export const ReportingExampleApp = ({
});
});

const getPDFJobParamsDefault = (): JobParamsPDF => {
const getPDFJobParamsDefault = () => {
return {
layout: {
id: constants.LAYOUT_TYPES.PRESERVE_LAYOUT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ test('getPdfJobParams returns the correct job params for canvas layout', () => {
const jobParams = getPdfJobParams(workpadSharingData, basePath);
expect(jobParams).toMatchInlineSnapshot(`
Object {
"browserTimezone": "America/New_York",
"layout": Object {
"dimensions": Object {
"height": 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,18 @@
*/

import { IBasePath } from 'kibana/public';
import moment from 'moment-timezone';
import rison from 'rison-node';
import { BaseParams } from '../../../../../reporting/common/types';
import { CanvasWorkpad } from '../../../../types';

export interface CanvasWorkpadSharingData {
workpad: Pick<CanvasWorkpad, 'id' | 'name' | 'height' | 'width'>;
pageCount: number;
}

// TODO: get the correct type from Reporting plugin
type JobParamsPDF = BaseParams & { relativeUrls: string[] };

export function getPdfJobParams(
{ workpad: { id, name: title, width, height }, pageCount }: CanvasWorkpadSharingData,
basePath: IBasePath
): JobParamsPDF {
) {
const urlPrefix = basePath.get().replace(basePath.serverBasePath, ''); // for Spaces prefix, which is included in basePath.get()
const canvasEntry = `${urlPrefix}/app/canvas#`;

Expand All @@ -43,7 +38,6 @@ export function getPdfJobParams(
}

return {
browserTimezone: moment.tz.guess(),
layout: {
dimensions: { width, height },
id: 'canvas',
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/reporting/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ export enum JOB_STATUSES {
export const REPORT_TABLE_ID = 'reportJobListing';
export const REPORT_TABLE_ROW_ID = 'reportJobRow';

// Job params require a `version` field as of 7.15.0. For older jobs set with
// automation that have no version value in the job params, we assume the
// intended version is 7.14.0
export const UNVERSIONED_VERSION = '7.14.0';
ppisljar marked this conversation as resolved.
Show resolved Hide resolved

// hacky endpoint: download CSV without queueing a report
// FIXME: find a way to make these endpoints "generic" instead of hardcoded, as are the queued report export types
export const API_GENERATE_IMMEDIATE = `${API_BASE_URL_V1}/generate/immediate/csv_searchsource`;
3 changes: 2 additions & 1 deletion x-pack/plugins/reporting/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,11 @@ export interface ReportDocument extends ReportDocumentHead {
}

export interface BaseParams {
browserTimezone?: string; // browserTimezone is optional: it is not in old POST URLs that were generated prior to being added to this interface
layout?: LayoutParams;
objectType: string;
title: string;
browserTimezone: string; // to format dates in the user's time zone
version: string; // to handle any state migrations
}

export type JobId = string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,45 @@
*/

import { i18n } from '@kbn/i18n';
import moment from 'moment';
import { stringify } from 'query-string';
import rison from 'rison-node';
import { HttpSetup } from 'src/core/public';
import rison, { RisonObject } from 'rison-node';
import { HttpSetup, IUiSettingsClient } from 'src/core/public';
import {
API_BASE_GENERATE,
API_BASE_URL,
API_GENERATE_IMMEDIATE,
API_LIST_URL,
API_MIGRATE_ILM_POLICY_URL,
REPORTING_MANAGEMENT_HOME,
} from '../../../common/constants';
import { DownloadReportFn, JobId, ManagementLinkFn, ReportApiJSON } from '../../../common/types';
import {
BaseParams,
DownloadReportFn,
JobId,
ManagementLinkFn,
ReportApiJSON,
} from '../../../common/types';
import { add } from '../../notifier/job_completion_notifications';
import { Job } from '../job';

/*
* For convenience, apps do not have to provide the browserTimezone and Kibana version.
* Those fields are added in this client as part of the service.
* TODO: export a type like this to other plugins: https://github.com/elastic/kibana/issues/107085
*/
type AppParams = Omit<BaseParams, 'browserTimezone' | 'version'>;

export interface DiagnoseResponse {
help: string[];
success: boolean;
logs: string;
}

interface JobParams {
[paramName: string]: any;
}

interface IReportingAPI {
// Helpers
getReportURL(jobId: string): string;
getReportingJobPath(exportType: string, jobParams: JobParams): string; // Return a URL to queue a job, with the job params encoded in the query string of the URL. Used for copying POST URL
getReportingJobPath<T>(exportType: string, jobParams: BaseParams & T): string; // Return a URL to queue a job, with the job params encoded in the query string of the URL. Used for copying POST URL
createReportingJob(exportType: string, jobParams: any): Promise<Job>; // Sends a request to queue a job, with the job params in the POST body
getServerBasePath(): string; // Provides the raw server basePath to allow it to be stripped out from relativeUrls in job params

Expand All @@ -57,11 +68,11 @@ interface IReportingAPI {
}

export class ReportingAPIClient implements IReportingAPI {
private http: HttpSetup;

constructor(http: HttpSetup) {
this.http = http;
}
constructor(
private http: HttpSetup,
private uiSettings: IUiSettingsClient,
private kibanaVersion: string
) {}

public getReportURL(jobId: string) {
const apiBaseUrl = this.http.basePath.prepend(API_LIST_URL);
Expand Down Expand Up @@ -132,13 +143,15 @@ export class ReportingAPIClient implements IReportingAPI {
return reports.map((report) => new Job(report));
}

public getReportingJobPath(exportType: string, jobParams: JobParams) {
const params = stringify({ jobParams: rison.encode(jobParams) });
public getReportingJobPath(exportType: string, jobParams: BaseParams) {
const risonObject: RisonObject = jobParams as Record<string, any>;
Copy link
Member

Choose a reason for hiding this comment

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

why do we need this additional casting now?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is pretty much moving the casting down from the local type which was removed: interface JobParams

const params = stringify({ jobParams: rison.encode(risonObject) });
return `${this.http.basePath.prepend(API_BASE_GENERATE)}/${exportType}?${params}`;
}

public async createReportingJob(exportType: string, jobParams: any) {
const jobParamsRison = rison.encode(jobParams);
public async createReportingJob(exportType: string, jobParams: BaseParams) {
const risonObject: RisonObject = jobParams as Record<string, any>;
const jobParamsRison = rison.encode(risonObject);
const resp: { job: ReportApiJSON } = await this.http.post(
`${API_BASE_GENERATE}/${exportType}`,
{
Expand All @@ -154,6 +167,27 @@ export class ReportingAPIClient implements IReportingAPI {
return new Job(resp.job);
}

public async createImmediateReport(baseParams: BaseParams) {
const { objectType: _objectType, ...params } = baseParams; // objectType is not needed for immediate download api
return this.http.post(`${API_GENERATE_IMMEDIATE}`, { body: JSON.stringify(params) });
}

public getDecoratedJobParams<T extends AppParams>(baseParams: T): BaseParams {
// If the TZ is set to the default "Browser", it will not be useful for
// server-side export. We need to derive the timezone and pass it as a param
// to the export API.
const browserTimezone: string =
this.uiSettings.get('dateFormat:tz') === 'Browser'
? moment.tz.guess()
: this.uiSettings.get('dateFormat:tz');

return {
browserTimezone,
version: this.kibanaVersion,
...baseParams,
};
}

public getManagementLink: ManagementLinkFn = () =>
this.http.basePath.prepend(REPORTING_MANAGEMENT_HOME);

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/reporting/public/lib/stream_handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ const mockJobsFound: Job[] = [
{ id: 'job-source-mock3', status: 'pending', output: { csv_contains_formulas: false, max_size_reached: false }, payload: { title: 'specimen' } },
].map((j) => new Job(j as ReportApiJSON)); // prettier-ignore

const jobQueueClientMock = new ReportingAPIClient(coreMock.createSetup().http);
const coreSetup = coreMock.createSetup();
const jobQueueClientMock = new ReportingAPIClient(coreSetup.http, coreSetup.uiSettings, '7.15.0');
jobQueueClientMock.findForJobIds = async () => mockJobsFound;
jobQueueClientMock.getInfo = () =>
Promise.resolve(({ content: 'this is the completed report data' } as unknown) as Job);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@

import React from 'react';
import { mountWithIntl } from '@kbn/test/jest';
import { coreMock } from '../../../../../src/core/public/mocks';
import { Job } from '../lib/job';
import { ReportInfoButton } from './report_info_button';

jest.mock('../lib/reporting_api_client');

import { ReportingAPIClient } from '../lib/reporting_api_client';

const httpSetup = {} as any;
const apiClient = new ReportingAPIClient(httpSetup);
const coreSetup = coreMock.createSetup();
const apiClient = new ReportingAPIClient(coreSetup.http, coreSetup.uiSettings, '7.15.0');

const job = new Job({
id: 'abc-123',
index: '.reporting-2020.04.12',
Expand All @@ -29,6 +31,7 @@ const job = new Job({
meta: { layout: 'preserve_layout', objectType: 'canvas workpad' },
payload: {
browserTimezone: 'America/Phoenix',
version: '7.15.0-test',
layout: { dimensions: { height: 720, width: 1080 }, id: 'preserve_layout' },
objectType: 'canvas workpad',
title: 'My Canvas Workpad',
Expand Down
Loading
Loading