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.

Sign up for GitHub

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] Rewrite addForceNowQuerystring to getFullUrls #44851

Merged
merged 14 commits into from
Sep 9, 2019
Merged

[Reporting] Rewrite addForceNowQuerystring to getFullUrls #44851

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
aaaf651
this is the one
tsullivan Sep 5, 2019
1737905
this could break a job nbd
tsullivan Sep 5, 2019
9a86dc3
get_full_url to observe PNG and PDF job payload types
tsullivan Sep 5, 2019
5c46d7d
fix typescripts
tsullivan Sep 5, 2019
f010372
fix ts in tests
tsullivan Sep 6, 2019
f162de7
fix more types
tsullivan Sep 6, 2019
ed93c41
cosmetic
tsullivan Sep 6, 2019
d1186b8
remove PDF execute compatibility shim test -- that stuff is handled i…
tsullivan Sep 6, 2019
14c199f
fix unit test
tsullivan Sep 6, 2019
c23af64
remove old strings
tsullivan Sep 6, 2019
0aae22f
Remove pdf execute compatibility shim entirely
tsullivan Sep 6, 2019
26b9431
combine the 2 maps
tsullivan Sep 6, 2019
96a2e3c
More reject matchers in the test
tsullivan Sep 6, 2019
c4e19dd
Merge branch 'master' into reporting/screenshots-with-absolute-urls-c…
tsullivan Sep 9, 2019
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
93 changes: 0 additions & 93 deletions ...gacy/plugins/reporting/export_types/common/execute_job/add_force_now_query_string.test.ts
View file
Open in desktop

This file was deleted.

60 changes: 0 additions & 60 deletions ...ck/legacy/plugins/reporting/export_types/common/execute_job/add_force_now_query_string.ts
View file
Open in desktop

This file was deleted.

3 changes: 0 additions & 3 deletions x-pack/legacy/plugins/reporting/export_types/common/execute_job/decrypt_job_headers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ describe('headers', () => {
isImmediate: false,
savedObjectType: 'search',
},
relativeUrl: '/app/kibana#/something',
timeRange: {},
},
server: mockServer,
})
Expand All @@ -55,7 +53,6 @@ describe('headers', () => {
isImmediate: false,
savedObjectType: 'search',
},
relativeUrl: '/app/kibana#/something',
headers: encryptedHeaders,
},
server: mockServer,
Expand Down
123 changes: 14 additions & 109 deletions .../legacy/plugins/reporting/export_types/common/execute_job/get_conditional_headers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

import { createMockServer } from '../../../test_helpers/create_mock_server';
import { getConditionalHeaders, getCustomLogo } from './index';
import { JobDocPayload } from '../../../types';
import { JobDocPayloadPDF } from '../../printable_pdf/types';

let mockServer: any;
beforeEach(() => {
Expand All @@ -26,15 +28,7 @@ describe('conditions', () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
Copy link
Member Author

Choose a reason for hiding this comment

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

A lot of the tests had mock data like this: job params that don't line up to existing features. The csv export type doesn't use savedObjectId and savedObjectType as jobParams. See JobParamsDiscoverCsv is the actual definition. Invalid mocks were part of the reason Typescripting is taking awhile.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies on that -- I believe I wrote these a while ago. You're dead-on about TS taking so long: I believe I originally wrote these mocks with little TS input, so they were likely incorrect. In any case, sorry!

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha, it's not problem. Mostly I write these things down because I learn as I go

},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -51,15 +45,7 @@ describe('conditions', () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -80,15 +66,7 @@ describe('conditions', () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -105,15 +83,7 @@ describe('conditions', () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -128,15 +98,7 @@ describe('conditions', () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -159,15 +121,7 @@ describe('conditions', () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -184,15 +138,7 @@ describe('conditions', () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -208,15 +154,7 @@ test('uses basePath from job when creating saved object service', async () => {
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -226,16 +164,7 @@ test('uses basePath from job when creating saved object service', async () => {

const jobBasePath = '/sbp/s/marketing';
await getCustomLogo({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
basePath: jobBasePath,
},
Copy link
Member Author

Choose a reason for hiding this comment

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

putting a custom logo on a CSV export would make no sense

job: { basePath: jobBasePath } as JobDocPayloadPDF,
conditionalHeaders,
server: mockServer,
});
Expand All @@ -252,15 +181,7 @@ test(`uses basePath from server if job doesn't have a basePath when creating sav
};

const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: permittedHeaders,
server: mockServer,
});
Expand All @@ -269,15 +190,7 @@ test(`uses basePath from server if job doesn't have a basePath when creating sav
mockServer.uiSettingsServiceFactory().get.mockReturnValue(logo);

await getCustomLogo({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayloadPDF,
conditionalHeaders,
server: mockServer,
});
Expand All @@ -291,15 +204,7 @@ describe('config formatting', () => {
test(`lowercases server.host`, async () => {
mockServer = createMockServer({ settings: { 'server.host': 'COOL-HOSTNAME' } });
const { conditionalHeaders } = await getConditionalHeaders({
job: {
title: 'cool-job-bro',
type: 'csv',
jobParams: {
savedObjectId: 'abc-123',
isImmediate: false,
savedObjectType: 'search',
},
},
job: {} as JobDocPayload,
filteredHeaders: {},
server: mockServer,
});
Expand Down
Loading