-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[Observability] Page attachment type #225295
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
Changes from all commits
77d172f
3bcb4d4
bc5c03d
192116f
8f3782f
0aeebda
6037685
5266320
14e8f82
ac2ef57
22cc56a
9d0d253
b45cdba
6d1c98b
5bd0383
d28a975
e9818ad
b988b67
6565e1b
711442b
ad3b528
beb55d4
f612f90
072a087
da1e7a7
d81aec3
c7166c5
479cf7c
87dd13f
5bde52d
07f312a
ddf1838
83bfd87
58bb176
60067a9
d42f848
ca0ddcb
f9c39a1
905c668
3a6f626
15fa00f
a866930
8e4bc8e
6f88fc2
3fa8b3e
7b944e8
2b165df
0478977
041df8d
660c524
6febbcf
3ed6892
6949dfd
552bc73
aad339b
aab1ad9
386d1bd
c31efa7
af34ae3
f5ef416
7f10f86
e5788c3
b3c182e
567b490
58b8d28
6563239
11730cb
f5c9d46
9c437d1
9d31b5d
3724b5b
0cc84e4
35bb2db
ecb7fd2
dc1779f
a18c17f
973b650
5745c78
6c1029e
867bd02
9c69465
d064307
941403f
08be407
37a3f9d
59379f2
5560306
41167a9
c9f0f43
c3378bf
e8037be
2f40548
d63072c
b5d9da5
932bf1a
cf31a52
8c045a8
8ef4b60
dbb5783
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 |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # @kbn/page-attachment-schema | ||
|
|
||
| This shared package contains the zod schema definition for persisted state of the page attachment type used in cases. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| export * from './src/latest'; | ||
| export * from './src/constants'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| module.exports = { | ||
| preset: '@kbn/test', | ||
| rootDir: '../../../../..', | ||
| roots: ['<rootDir>/x-pack/platform/packages/shared/kbn-page-attachment-schema'], | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "type": "shared-common", | ||
| "id": "@kbn/page-attachment-schema", | ||
| "owner": [ | ||
| "@elastic/obs-ux-management-team" | ||
| ], | ||
| "group": "platform", | ||
| "visibility": "shared" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "name": "@kbn/page-attachment-schema", | ||
| "description": "Page attachment zod schema definition and constants.", | ||
| "private": true, | ||
| "version": "1.0.0", | ||
| "license": "Elastic License 2.0" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| export const PAGE_ATTACHMENT_TYPE = '.page'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| export { | ||
| type PageAttachmentPersistedState, | ||
| pageAttachmentPersistedStateSchema, | ||
| } from './schema/latest'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| export { type PageAttachmentPersistedState, pageAttachmentPersistedStateSchema } from './v1'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import { z } from '@kbn/zod'; | ||
|
|
||
| export const pageAttachmentPersistedStateSchema = z.object({ | ||
| /** | ||
| * The type of page or page asset, e.g., 'dashboard', 'synthetics-test-run', 'slo-history', etc | ||
|
Contributor
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. Why do we need type here?
Member
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. It's basically a sub-type to help us make future decisions about how to render this page, how to evaluate it in LLM analysis, etc.
Contributor
Author
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. @jasonrhodes Michael and I had actually discussed this over zoom last week. I should have put the reply here for reference. |
||
| */ | ||
| type: z.string(), | ||
| url: z.object({ | ||
| /** | ||
| * The URL to the page or page asset, excluding the base path. | ||
| */ | ||
| pathAndQuery: z.string(), | ||
| /** | ||
| * The label to render in the "Go to" action, example "View in Dashboard" for the asset | ||
| */ | ||
| actionLabel: z.string(), | ||
| /** | ||
| * The label to render in the attachment comment for the page or page asset | ||
| */ | ||
| label: z.string(), | ||
| /** | ||
| * The icon, rendered via {@link EuiIcon}, representing the page type, displayed in the comment and action. | ||
| */ | ||
| iconType: z.string().optional(), | ||
| }), | ||
| /** | ||
| * Optional screen context for the page. A plain text description that | ||
| * can be provided to an LLM to generate a summary or perform analysis | ||
| */ | ||
| screenContext: z.array(z.object({ screenDescription: z.string() })).optional(), | ||
| }); | ||
|
|
||
| export type PageAttachmentPersistedState = z.infer<typeof pageAttachmentPersistedStateSchema>; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| "extends": "../../../../../tsconfig.base.json", | ||
| "compilerOptions": { | ||
| "outDir": "target/types", | ||
| "types": [ | ||
| "jest", | ||
| "node" | ||
| ] | ||
| }, | ||
| "include": [ | ||
| "**/*.ts" | ||
| ], | ||
| "kbn_references": [ | ||
| "@kbn/zod", | ||
| ], | ||
| "exclude": [ | ||
| "target/**/*", | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,7 +48,8 @@ | |
| "savedObjectsFinder" | ||
| ], | ||
| "extraPublicDirs": [ | ||
| "common" | ||
| "common", | ||
| "public/client/attachment_framework/types" | ||
|
Contributor
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 wonder if we should move these types to |
||
| ] | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,12 @@ import type { Boom } from '@hapi/boom'; | |
| import { boomify, isBoom } from '@hapi/boom'; | ||
| import { schema } from '@kbn/config-schema'; | ||
| import type { CustomHttpResponseOptions, ResponseError, Headers, Logger } from '@kbn/core/server'; | ||
| import { isInternalURL } from '@kbn/std'; | ||
| import { | ||
| type PageAttachmentPersistedState, | ||
| PAGE_ATTACHMENT_TYPE, | ||
| } from '@kbn/page-attachment-schema'; | ||
| import type { AttachmentRequestAttributes } from '../../common/types/attachments'; | ||
| import type { CaseError, HTTPError } from '../../common/error'; | ||
| import { isCaseError, isHTTPError } from '../../common/error'; | ||
|
|
||
|
|
@@ -35,7 +41,45 @@ export function wrapError( | |
| }; | ||
| } | ||
|
|
||
| export const escapeHatch = schema.object({}, { unknowns: 'allow' }); | ||
| export const escapeHatch = schema.object( | ||
| {}, | ||
| { | ||
| unknowns: 'allow', | ||
| } | ||
| ); | ||
|
|
||
| export const validAttachment = schema.object( | ||
| {}, | ||
| { | ||
| unknowns: 'allow', | ||
| validate: (value) => { | ||
| if ( | ||
| isPersistableStatePageAttachment(value) && | ||
| value.persistableStateAttachmentTypeId === PAGE_ATTACHMENT_TYPE | ||
| ) { | ||
| const persistedState = | ||
| value.persistableStateAttachmentState as PageAttachmentPersistedState; | ||
| const url = persistedState?.url?.pathAndQuery; | ||
| if (url && !isInternalURL(url)) { | ||
| return `External urls are not supported for page attachments. The provided url is: ${url}`; | ||
| } | ||
| } | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| /** | ||
| * Checks if the given value is a persistable state page attachment type. | ||
| */ | ||
| export const isPersistableStatePageAttachment = ( | ||
|
Contributor
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: This isn't actually checking if it's a
Contributor
Author
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. This particularly util wasn't intended to check if it's a I suppose I could move all the checks here. That just wasn't my original intent. |
||
| value: unknown | ||
| ): value is AttachmentRequestAttributes => { | ||
| return ( | ||
| typeof value === 'object' && | ||
| value !== null && | ||
| Boolean((value as AttachmentRequestAttributes).persistableStateAttachmentTypeId) | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * Creates a warning header with a message formatted according to RFC7234. | ||
|
|
||
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.
Did we look into reducing this?
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.
I'm not sure I can with the addition of add to case flyout from the cases plugin, at least without it being pulled out into a package. cc: @cnasikas @michaelolo24
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.
👍 thanks for looking. Don't want to block, but it would be helpful to follow up at some point.
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.
In syncing with Christos last week, the option of moving it out to a package has been proposed, so definitely something we can investigate