-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] improve deprecation logging, of PDF creation params #44130
Changes from all commits
7830c45
e64a52a
b41b259
012b849
a4ec394
93433e6
fdc7dda
b32c276
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,78 +5,110 @@ | |
*/ | ||
|
||
import { uriEncode } from '../lib/uri_encode'; | ||
import { PLUGIN_ID } from '../../../../common/constants'; | ||
|
||
export function compatibilityShimFactory(server) { | ||
|
||
const logDeprecation = (msg) => { | ||
server.log(['warning', PLUGIN_ID, 'deprecation'], msg + ' This functionality will be removed with the next major version.'); | ||
}; | ||
|
||
const getSavedObjectTitle = async (objectType, savedObjectId, savedObjectsClient) => { | ||
if (!objectType) { | ||
throw new Error('objectType is required to determine the title from the saved object'); | ||
} | ||
|
||
if (!savedObjectId) { | ||
throw new Error('savedObjectId is required to determine the title from the saved object'); | ||
} | ||
/* | ||
* TODO: Kibana 8.0: | ||
* Remove support for parsing Saved Object details from objectType / savedObjectId | ||
* Including support for determining the Report title from objectType / savedObjectId | ||
* | ||
* - `objectType` is optional, but helps differentiate the type of report in the job listing | ||
* - `title` must be explicitly passed | ||
* - `relativeUrls` array OR `relativeUrl` string must be passed | ||
*/ | ||
|
||
logDeprecation('The title should be provided with the job generation request. Please use Kibana to regenerate your URLs.'); | ||
const savedObject = await savedObjectsClient.get(objectType, savedObjectId); | ||
const getSavedObjectTitle = async (objectType, savedObjectId, savedObjectsClient) => { | ||
const savedObject = await savedObjectsClient.get(objectType, savedObjectId); | ||
return savedObject.attributes.title; | ||
}; | ||
|
||
return savedObject.attributes.title; | ||
const getSavedObjectRelativeUrl = (objectType, savedObjectId, queryString) => { | ||
const appPrefixes = { | ||
dashboard: '/dashboard/', | ||
visualization: '/visualize/edit/', | ||
search: '/discover/', | ||
}; | ||
|
||
const appPrefix = appPrefixes[objectType]; | ||
if (!appPrefix) throw new Error('Unexpected app type: ' + objectType); | ||
|
||
const getSavedObjectRelativeUrl = (objectType, savedObjectId, queryString) => { | ||
if (!objectType) { | ||
throw new Error('objectType is required to determine the savedObject urlHash'); | ||
} | ||
|
||
if (!savedObjectId) { | ||
throw new Error('id is required to determine the savedObject relativeUrl'); | ||
} | ||
|
||
logDeprecation('The relativeUrl should be provided with the job generation request. Please use Kibana to regenerate your URLs.'); | ||
const appPrefixes = { | ||
dashboard: '/dashboard/', | ||
visualization: '/visualize/edit/', | ||
search: '/discover/' | ||
}; | ||
|
||
const appPrefix = appPrefixes[objectType]; | ||
if (!appPrefix) throw new Error('Unexpected app type: ' + objectType); | ||
const hash = appPrefix + uriEncode.string(savedObjectId, true); | ||
|
||
const hash = appPrefix + uriEncode.string(savedObjectId, true); | ||
return `/app/kibana#${hash}?${queryString || ''}`; | ||
}; | ||
|
||
return `/app/kibana#${hash}?${queryString || ''}`; | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was able to hoist these helper functions up since a logger is passed in, and logging is not done from the helper functions |
||
export function compatibilityShimFactory(server, logger) { | ||
return function compatibilityShimFactory(createJobFn) { | ||
return async function ( | ||
{ | ||
savedObjectId, // deprecating | ||
queryString, // deprecating | ||
browserTimezone, | ||
objectType, | ||
title, | ||
relativeUrl, // not deprecating | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was an in-the-moment decision to say to keep this, and not add warning logging when it is used |
||
relativeUrls, | ||
layout | ||
}, | ||
headers, | ||
request | ||
) { | ||
if (savedObjectId && (relativeUrl || relativeUrls)) { | ||
throw new Error(`savedObjectId should not be provided if relativeUrls are provided`); | ||
} | ||
if (!savedObjectId && !relativeUrl && !relativeUrls) { | ||
throw new Error(`either relativeUrls or savedObjectId must be provided`); | ||
} | ||
|
||
return function compatibilityShimFactory(createJob) { | ||
return async function ({ | ||
objectType, | ||
savedObjectId, | ||
title, | ||
relativeUrls, | ||
queryString, | ||
browserTimezone, | ||
layout | ||
}, headers, request) { | ||
let kibanaRelativeUrls; | ||
if (relativeUrls) { | ||
kibanaRelativeUrls = relativeUrls; | ||
} else if (relativeUrl) { | ||
kibanaRelativeUrls = [ relativeUrl ]; | ||
} else { | ||
kibanaRelativeUrls = [getSavedObjectRelativeUrl(objectType, savedObjectId, queryString)]; | ||
logger.warning( | ||
`The relativeUrls have been derived from saved object parameters. ` + | ||
`This functionality will be removed with the next major version.` | ||
); | ||
} | ||
|
||
if (objectType && savedObjectId && relativeUrls) { | ||
throw new Error('objectType and savedObjectId should not be provided in addition to the relativeUrls'); | ||
let reportTitle; | ||
try { | ||
if (title) { | ||
reportTitle = title; | ||
} else { | ||
if (objectType && savedObjectId) { | ||
reportTitle = await getSavedObjectTitle( | ||
objectType, | ||
savedObjectId, | ||
request.getSavedObjectsClient() | ||
); | ||
logger.warning( | ||
`The title has been derived from saved object parameters. This ` + | ||
`functionality will be removed with the next major version.` | ||
); | ||
} else { | ||
logger.warning( | ||
`A title parameter should be provided with the job generation ` + | ||
`request. Please use Kibana to regenerate your POST URL to have a ` + | ||
`title included in the PDF.` | ||
); | ||
} | ||
} | ||
} catch (err) { | ||
logger.error(err); // 404 for the savedObjectId, etc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds the logging that was needed for:
from #43628 |
||
throw err; | ||
} | ||
|
||
const transformedJobParams = { | ||
objectType, | ||
title: title || await getSavedObjectTitle(objectType, savedObjectId, request.getSavedObjectsClient()), | ||
relativeUrls: objectType && savedObjectId ? [ getSavedObjectRelativeUrl(objectType, savedObjectId, queryString) ] : relativeUrls, | ||
title: reportTitle, | ||
relativeUrls: kibanaRelativeUrls, | ||
browserTimezone, | ||
layout | ||
layout, | ||
}; | ||
|
||
return await createJob(transformedJobParams, headers, request); | ||
return await createJobFn(transformedJobParams, headers, request); | ||
}; | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related: #44178