Skip to content

Commit

Permalink
[Screenshotting] Change diagnostic endpoint method to GET (elastic#19…
Browse files Browse the repository at this point in the history
…2232)

## Summary

Closes elastic#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
  • Loading branch information
tsullivan authored Sep 10, 2024
1 parent 90efe1d commit 65e53eb
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 11 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-reporting/public/reporting_api_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/kbn-reporting/public/reporting_api_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export class ReportingAPIClient implements IReportingAPI {
public getServerBasePath = () => this.http.basePath.serverBasePath;

public verifyBrowser() {
return this.http.post<DiagnoseResponse>(INTERNAL_ROUTES.DIAGNOSE.BROWSER);
return this.http.get<DiagnoseResponse>(INTERNAL_ROUTES.DIAGNOSE.BROWSER);
}

public verifyScreenCapture() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
})}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type SetupServerReturn = Awaited<ReturnType<typeof setupServer>>;
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();
Expand Down Expand Up @@ -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);
Expand All @@ -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(`
Expand All @@ -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(`
Expand All @@ -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',
});
});
Expand Down

0 comments on commit 65e53eb

Please sign in to comment.