[Reporting] Fix - show diagnostic only when image reporting is enabled#164336
[Reporting] Fix - show diagnostic only when image reporting is enabled#164336rshen91 merged 11 commits intoelastic:mainfrom
Conversation
tsullivan
left a comment
There was a problem hiding this comment.
This is great and thank you so much for catching this!
I left comments about keeping the top-level entry point of Reporting front-end more generic, and getting the specifics out of the config object closer to the point of code where we need it. Let me know if you have other ideas :)
| const configAllowsImages = | ||
| this.config.export_types.pdf.enabled || this.config.export_types.png.enabled; |
There was a problem hiding this comment.
Since this is the top-level entry point for the front-end of Reporting, we should probably consider creating this boolean elsewhere in the code where it's specifically needed. I recommend passing this.config.export_types in the call to mountManagementSection. See below
Edit: changed the wording of my suggestion
There was a problem hiding this comment.
Cool thanks I think I'm following the guidance please see c7adb54
| share.url, | ||
| params | ||
| params, | ||
| configAllowsImages |
There was a problem hiding this comment.
This is up to you, but we have some options to use more generic/intuitive parameters when calling mountManagementSection:
const umountAppCallback = await mountManagementSection(
core,
start,
license$,
this.config.poll,
this.config.export_types,
apiClient,
share.url,
params
);
or more simply (but impacts other code):
const umountAppCallback = await mountManagementSection(
core,
start,
license$,
this.config,
apiClient,
share.url,
params
);
| }; | ||
|
|
||
| export const ReportDiagnostic = ({ apiClient }: Props) => { | ||
| export const ReportDiagnostic = ({ apiClient, configAllowsImages }: Props) => { |
There was a problem hiding this comment.
Following the previous comments, this seems like the best place unwrap a generic config object and create a boolean called configAllowsImages .
| export const ReportDiagnostic = ({ apiClient, configAllowsImages }: Props) => { | |
| export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => { | |
| configAllowsImages = clientConfig.export_types.pdf.enabled || clientConfig.export_types.png.enabled; |
…into serverless-no-diag-report-link
…-ref HEAD~1..HEAD --fix'
|
Pinging @elastic/appex-sharedux (Team:SharedUX) |
tsullivan
left a comment
There was a problem hiding this comment.
LGTM! This is awesome! So happy to see the test updates!
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @rshen91 |
* main: (150 commits) Fixes unnecessary autocompletes on HTTP methods (elastic#163233) [Defend Workflows] Convert filterQuery to kql (elastic#161806) [Fleet] copy `inactivity_timeout` when duplicating agent policy (elastic#164544) Fix 7.17 forward compatibility with 8.2+ (elastic#164274) [ML] Fixes dark mode in flyouts and modals (elastic#164399) [Defend Workflows]Changes to policy settings are not persistent until a refresh (elastic#164403) [Security Solution][Endpoint] Fixes kibana crash when going back to policy details page (elastic#164329) Prepare the Security domain HTTP APIs for Serverless (elastic#162087) skip failing test suite (elastic#160986) [Security Solution] Fix flaky Event Filters test (elastic#164473) [EDR workflows] Osquery serverless tests (elastic#163795) [Fleet] Only show agent dashboard links if there is more than one non-server agent and if the dashboards exist (elastic#164469) [Chrome UI] Fix background color in serverless (elastic#164419) [DOCS] Saved objects - resolve import errors API (elastic#162825) Remove 'Create Rule' button from Rule Group page (elastic#164167) [Security Solution] expandable flyout - fix infinite loop in correlations (elastic#163450) [Remote Clusters] Update copy about port help text (elastic#164442) [api-docs] 2023-08-23 Daily api_docs build (elastic#164524) [data views] Disable scripted fields in serverless environment (elastic#163228) [Reporting] Fix - show diagnostic only when image reporting is enabled (elastic#164336) ...
Summary
Closes #164363
In serverless, pdf and png reports are not possible. I don't think it makes a lot of sense for the screenshotting diagnostic to appear to users since it's misleading.
If pdf and pngs are disabled by the config then the screenshotting diagnostic link will not be shown on the reporting management page like in serverless:
Checklist