[Reporting] improve deprecation logging, of PDF creation params#44130
Conversation
| }; | ||
|
|
||
| return `/app/kibana#${hash}?${queryString || ''}`; | ||
| }; |
There was a problem hiding this comment.
I was able to hoist these helper functions up since a logger is passed in, and logging is not done from the helper functions
| browserTimezone, | ||
| objectType, | ||
| title, | ||
| relativeUrl, // not deprecating |
There was a problem hiding this comment.
It was an in-the-moment decision to say to keep this, and not add warning logging when it is used
💚 Build Succeeded |
💚 Build Succeeded |
| return `/app/kibana#${hash}?${queryString || ''}`; | ||
| }; | ||
| export function compatibilityShimFactory(server, logger) { | ||
| if (!logger) throw new Error('need that logger'); |
There was a problem hiding this comment.
Might be fixable with TS, but probably not worth investing in here.
There was a problem hiding this comment.
Shoot, this line was supposed to be dev-only 😱
joelgriffith
left a comment
There was a problem hiding this comment.
Code LGTM. Was thinking TS might be prudent here, but not sure we should keep this around anyways.
| throw new Error('savedObjectId is required to determine the title from the saved object'); | ||
| } | ||
| /* | ||
| * TODO: Kibana 8.0: |
| } | ||
| } | ||
| } catch (err) { | ||
| logger.error(err); // 404 for the savedObjectId, etc |
There was a problem hiding this comment.
This adds the logging that was needed for:
A single deprecation warning is logged, and then the job does NOT get created. Nothing else got logged (I did not have verbose logging turned on).
from #43628
I agree with not wanting to keep it around. The existence makes the difficulty of moving to TypeScript VERY difficult, because the compatibility layer changes the shape of the data that comes in from the route handler. So the definition of what makes I've been really tempted to convert that layer to Typescript a few times, because I want to change where We can talk more about this offline. This is getting to one of the hardest parts of improving the Reporting code |
💚 Build Succeeded |
…tic#44130) * [Reporting] improve deprecation logging, of PDF creation params * more test * more test stuff * note * correctness * undo prettier * remove dev statement
…) (#44217) * [Reporting] improve deprecation logging, of PDF creation params * more test * more test stuff * note * correctness * undo prettier * remove dev statement
Summary
Closes #43628
This PR updates messaging provided by the
compatibility_shimlayer of PDF job creation. As this layer needs to be removed for the next major version, this is a re-visit of how it works and how its tested.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibility