From 65e53eb02bd4835c920b8084ba40259026e413ea Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Tue, 10 Sep 2024 08:15:37 -0700 Subject: [PATCH] [Screenshotting] Change diagnostic endpoint method to GET (#192232) ## Summary Closes https://github.com/elastic/kibana/issues/171698 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../public/reporting_api_client.test.ts | 2 +- .../kbn-reporting/public/reporting_api_client.ts | 2 +- .../management/components/report_diagnostic.tsx | 2 +- .../server/routes/internal/diagnostic/browser.ts | 4 ++-- .../diagnostic/integration_tests/browser.test.ts | 12 ++++++------ 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/kbn-reporting/public/reporting_api_client.test.ts b/packages/kbn-reporting/public/reporting_api_client.test.ts index d43eea01411bb..c1da4b46ae27a 100644 --- a/packages/kbn-reporting/public/reporting_api_client.test.ts +++ b/packages/kbn-reporting/public/reporting_api_client.test.ts @@ -288,7 +288,7 @@ describe('ReportingAPIClient', () => { it('should send a post request', async () => { await apiClient.verifyBrowser(); - expect(httpClient.post).toHaveBeenCalledWith(expect.stringContaining('/diagnose/browser')); + expect(httpClient.get).toHaveBeenCalledWith(expect.stringContaining('/diagnose/browser')); }); }); diff --git a/packages/kbn-reporting/public/reporting_api_client.ts b/packages/kbn-reporting/public/reporting_api_client.ts index b1579c463f901..247fb4b67d28d 100644 --- a/packages/kbn-reporting/public/reporting_api_client.ts +++ b/packages/kbn-reporting/public/reporting_api_client.ts @@ -251,7 +251,7 @@ export class ReportingAPIClient implements IReportingAPI { public getServerBasePath = () => this.http.basePath.serverBasePath; public verifyBrowser() { - return this.http.post(INTERNAL_ROUTES.DIAGNOSE.BROWSER); + return this.http.get(INTERNAL_ROUTES.DIAGNOSE.BROWSER); } public verifyScreenCapture() { diff --git a/x-pack/plugins/reporting/public/management/components/report_diagnostic.tsx b/x-pack/plugins/reporting/public/management/components/report_diagnostic.tsx index 470fa86b2e9a6..bbaab324a3fd4 100644 --- a/x-pack/plugins/reporting/public/management/components/report_diagnostic.tsx +++ b/x-pack/plugins/reporting/public/management/components/report_diagnostic.tsx @@ -103,7 +103,7 @@ export const ReportDiagnostic = ({ apiClient, clientConfig }: Props) => { id="xpack.reporting.listing.diagnosticSuccessMessage" color="success" title={i18n.translate('xpack.reporting.listing.diagnosticSuccessMessage', { - defaultMessage: 'Everything looks good for reporting to function.', + defaultMessage: 'Everything looks good for screenshot reports to function.', })} /> ); diff --git a/x-pack/plugins/reporting/server/routes/internal/diagnostic/browser.ts b/x-pack/plugins/reporting/server/routes/internal/diagnostic/browser.ts index a4ca554b1e7db..f23a8855849af 100644 --- a/x-pack/plugins/reporting/server/routes/internal/diagnostic/browser.ts +++ b/x-pack/plugins/reporting/server/routes/internal/diagnostic/browser.ts @@ -40,10 +40,10 @@ const path = INTERNAL_ROUTES.DIAGNOSE.BROWSER; export const registerDiagnoseBrowser = (reporting: ReportingCore, logger: Logger) => { const { router } = reporting.getPluginSetupDeps(); - router.post( + router.get( { path, - validate: {}, + validate: false, options: { access: 'internal' }, }, authorizedUserPreRouting(reporting, async (_user, _context, req, res) => { diff --git a/x-pack/plugins/reporting/server/routes/internal/diagnostic/integration_tests/browser.test.ts b/x-pack/plugins/reporting/server/routes/internal/diagnostic/integration_tests/browser.test.ts index d546d3d29ed47..3f1966d2e78d8 100644 --- a/x-pack/plugins/reporting/server/routes/internal/diagnostic/integration_tests/browser.test.ts +++ b/x-pack/plugins/reporting/server/routes/internal/diagnostic/integration_tests/browser.test.ts @@ -25,7 +25,7 @@ type SetupServerReturn = Awaited>; const devtoolMessage = 'DevTools listening on (ws://localhost:4000)'; const fontNotFoundMessage = 'Could not find the default font'; -describe(`POST ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => { +describe(`GET ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => { jest.setTimeout(6000); const reportingSymbol = Symbol('reporting'); const mockLogger = loggingSystemMock.createLogger(); @@ -89,7 +89,7 @@ describe(`POST ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => { screenshotting.diagnose.mockReturnValue(Rx.of(devtoolMessage)); return supertest(httpSetup.server.listener) - .post(INTERNAL_ROUTES.DIAGNOSE.BROWSER) + .get(INTERNAL_ROUTES.DIAGNOSE.BROWSER) .expect(200) .then(({ body }) => { expect(body.success).toEqual(true); @@ -105,7 +105,7 @@ describe(`POST ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => { screenshotting.diagnose.mockReturnValue(Rx.of(logs)); return supertest(httpSetup.server.listener) - .post(INTERNAL_ROUTES.DIAGNOSE.BROWSER) + .get(INTERNAL_ROUTES.DIAGNOSE.BROWSER) .expect(200) .then(({ body }) => { expect(body).toMatchInlineSnapshot(` @@ -127,7 +127,7 @@ describe(`POST ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => { screenshotting.diagnose.mockReturnValue(Rx.of(`${devtoolMessage}\n${fontNotFoundMessage}`)); return supertest(httpSetup.server.listener) - .post(INTERNAL_ROUTES.DIAGNOSE.BROWSER) + .get(INTERNAL_ROUTES.DIAGNOSE.BROWSER) .expect(200) .then(({ body }) => { expect(body).toMatchInlineSnapshot(` @@ -151,11 +151,11 @@ describe(`POST ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}`, () => { screenshotting.diagnose.mockReturnValue(Rx.of(devtoolMessage)); - await supertest(httpSetup.server.listener).post(INTERNAL_ROUTES.DIAGNOSE.BROWSER).expect(200); + await supertest(httpSetup.server.listener).get(INTERNAL_ROUTES.DIAGNOSE.BROWSER).expect(200); expect(usageCounter.incrementCounter).toHaveBeenCalledTimes(1); expect(usageCounter.incrementCounter).toHaveBeenCalledWith({ - counterName: `post ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}:success`, + counterName: `get ${INTERNAL_ROUTES.DIAGNOSE.BROWSER}:success`, counterType: 'reportingApi', }); });