-
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
[Response Ops][Cases] Use deprecation object #201004
Changes from 9 commits
5268611
00983c9
3664f50
24b35fa
72cd4cb
a69d1e2
79972e2
3af43a8
12dc126
b1d3b17
0ce90e7
8fd091e
b8f9640
a42a25f
b6c6280
6ec9cce
c849eea
b653ea6
c7bb5e0
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 |
---|---|---|
|
@@ -1004,5 +1004,8 @@ export const getDocLinks = ({ kibanaBranch, buildFlavor }: GetDocLinkOptions): D | |
inferenceManagement: { | ||
inferenceAPIDocumentation: `${ELASTIC_WEBSITE_URL}docs/api/doc/elasticsearch/operation/operation-inference-put`, | ||
}, | ||
cases: { | ||
legacyDeprecations: `${KIBANA_DOCS}breaking-changes-summary.html#breaking-201004`, | ||
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. nit: |
||
}, | ||
}); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,30 @@ | |
*/ | ||
|
||
import { getAllCommentsRoute } from './get_all_comment'; | ||
import { docLinksServiceMock } from '@kbn/core/server/mocks'; | ||
|
||
describe('getAllCommentsRoute', () => { | ||
const docLinks = docLinksServiceMock.createSetupContract(); | ||
|
||
it('marks the endpoint internal in serverless', async () => { | ||
const router = getAllCommentsRoute({ isServerless: true }); | ||
const router = getAllCommentsRoute({ isServerless: true, docLinks }); | ||
|
||
expect(router.routerOptions?.access).toBe('internal'); | ||
expect(router.routerOptions?.deprecated).toMatchInlineSnapshot(` | ||
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. nit: I think it would be better if we had a dedicated test for the deprecation object check. The object is available in all flavors, serverless or non-serverless. Same for all test files. |
||
Object { | ||
"documentationUrl": "https://www.elastic.co/guide/en/kibana/test-branch/breaking-changes-summary.html#breaking-201004", | ||
"reason": Object { | ||
"newApiMethod": "GET", | ||
"newApiPath": "/api/cases/{case_id}/comments/_find", | ||
"type": "migrate", | ||
}, | ||
"severity": "warning", | ||
} | ||
`); | ||
}); | ||
|
||
it('marks the endpoint public in non-serverless', async () => { | ||
const router = getAllCommentsRoute({ isServerless: false }); | ||
const router = getAllCommentsRoute({ isServerless: false, docLinks }); | ||
|
||
expect(router.routerOptions?.access).toBe('public'); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
* 2.0. | ||
*/ | ||
|
||
import type { DocLinksServiceSetup } from '@kbn/core/server'; | ||
import type { CaseRoute } from '../types'; | ||
|
||
import { CASE_STATUS_URL } from '../../../../common/constants'; | ||
|
@@ -15,7 +16,13 @@ import type { statsApiV1 } from '../../../../common/types/api'; | |
/** | ||
* @deprecated since version 8.1.0 | ||
*/ | ||
export const getStatusRoute = ({ isServerless }: { isServerless?: boolean }): CaseRoute => | ||
export const getStatusRoute = ({ | ||
isServerless, | ||
docLinks, | ||
}: { | ||
isServerless?: boolean; | ||
docLinks: DocLinksServiceSetup; | ||
}): CaseRoute => | ||
createCasesRoute({ | ||
method: 'get', | ||
path: CASE_STATUS_URL, | ||
|
@@ -27,8 +34,13 @@ export const getStatusRoute = ({ isServerless }: { isServerless?: boolean }): Ca | |
description: | ||
'Returns the number of cases that are open, closed, and in progress in the default space.', | ||
// You must have `read` privileges for the **Cases** feature in the **Management**, **Observability**, or **Security** section of the Kibana feature privileges, depending on the owner of the cases you're seeking. | ||
// @ts-expect-error TODO(https://github.com/elastic/kibana/issues/196095): Replace {RouteDeprecationInfo} | ||
deprecated: true, | ||
deprecated: { | ||
documentationUrl: docLinks.links.cases.legacyDeprecations, | ||
severity: 'warning', | ||
reason: { | ||
type: 'deprecate', | ||
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 think this should be |
||
}, | ||
}, | ||
}, | ||
handler: async ({ context, request, response }) => { | ||
try { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,30 @@ | |
*/ | ||
|
||
import { getUserActionsRoute } from './get_all_user_actions'; | ||
import { docLinksServiceMock } from '@kbn/core/server/mocks'; | ||
|
||
describe('getUserActionsRoute', () => { | ||
const docLinks = docLinksServiceMock.createSetupContract(); | ||
|
||
it('marks the endpoint internal in serverless', async () => { | ||
const router = getUserActionsRoute({ isServerless: true }); | ||
const router = getUserActionsRoute({ isServerless: true, docLinks }); | ||
|
||
expect(router.routerOptions?.access).toBe('internal'); | ||
expect(router.routerOptions?.deprecated).toMatchInlineSnapshot(` | ||
Object { | ||
"documentationUrl": "https://www.elastic.co/guide/en/kibana/test-branch/breaking-changes-summary.html#breaking-201004", | ||
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.
If it's at all possible to make this a fuzzier match that would be a good idea IMO since the initial part of that URL is almost certain to change as we migrate to the new docs system. 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. We will test that the url ends with |
||
"reason": Object { | ||
"newApiMethod": "GET", | ||
"newApiPath": "/api/cases/<case_id>/user_actions/_find", | ||
"type": "migrate", | ||
}, | ||
"severity": "warning", | ||
} | ||
`); | ||
}); | ||
|
||
it('marks the endpoint public in non-serverless', async () => { | ||
const router = getUserActionsRoute({ isServerless: false }); | ||
const router = getUserActionsRoute({ isServerless: false, docLinks }); | ||
|
||
expect(router.routerOptions?.access).toBe('public'); | ||
}); | ||
|
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.
How did you find out these needed to be updated too?
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.
Because the link to the docs' reference is this value. I just kept "breaking" because of consistency with the other references
That said, I'm still not sure it's working :) I'm waiting for CI to generate the docs