[Incident Management] Add page attachment modal by dominiqueclarke · Pull Request #231186 · elastic/kibana

dominiqueclarke · 2025-08-08T16:28:47Z

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

https___google.com.-.Monitors.-.Synthetics.-.Observability.-.Elastic.2.webm

author Dominique Belcher <dominique.clarke@elastic.co> 1749583346 -0400 committer Dominique Belcher <dominique.clarke@elastic.co> 1754678113 -0400 parent 6f8bb90 author Dominique Belcher <dominique.clarke@elastic.co> 1749583346 -0400 committer Dominique Belcher <dominique.clarke@elastic.co> 1754678081 -0400 parent 6f8bb90 author Dominique Belcher <dominique.clarke@elastic.co> 1749583346 -0400 committer Dominique Belcher <dominique.clarke@elastic.co> 1754678047 -0400 parent 6f8bb90 author Dominique Belcher <dominique.clarke@elastic.co> 1749583346 -0400 committer Dominique Belcher <dominique.clarke@elastic.co> 1754677385 -0400 parent 6f8bb90 author Dominique Belcher <dominique.clarke@elastic.co> 1749583346 -0400 committer Dominique Belcher <dominique.clarke@elastic.co> 1754677363 -0400 add link attachment type remove dashboard dependency from triggers actions ui [CI] Auto-commit changed files from 'node scripts/yarn_deduplicate' integrate screenshot adjust types quick checks remove unnecessary dashboard dependency add actions split and merge screenshot images remove screenshot functionality adjust rendered component adjust types and linting adjust types adjust test adjust types adjust tests adjust types remove dashboard implementation add attachment type to observability remove attachment type from cases adjust license Update use_dashboard_menu_items.tsx Update use_dashboard_menu_items.tsx remove additional references in cases adjust test adjust tests add "add to case" button to Synthetics monitor details adjust types adjust security start plugin adjust import adjust import Update assignees.test.tsx adjust tests add feature flag [CI] Auto-commit changed files from 'node scripts/styled_components_mapping' adjust import add ai summary add summary to attachment children set modal to closed adjust tests remove page attachment references from observability-schema adjust plugin setup adjust observability schema package add page attachment modal add tests adjust types and remove unnecessary test [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' [CI] Auto-commit changed files from 'node scripts/telemetry_check' adjust type persist the screen context adjust isObsAIAssistantEnabled and add tests adjust tests improve prompt adjust types remove ai elements remove useScreenContext delete unnecessary fixes remove useMonitorScreenContext remove unncessary change

elasticmachine · 2025-08-11T13:30:32Z

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

…urce-definitions/scripts/fix-location-collection.ts'

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

...ed/public/components/add_page_attachment_to_case_modal/add_page_attachment_to_case_modal.tsx

...blic/components/add_page_attachment_to_case_modal/add_page_attachment_to_case_modal_lazy.tsx

...ed/public/components/add_page_attachment_to_case_modal/add_page_attachment_to_case_modal.tsx

...servability/plugins/synthetics/public/apps/synthetics/components/monitor_details/actions.tsx

...s/observability/plugins/observability_shared/public/components/add_to_case_comment/index.tsx

...ed/public/components/add_page_attachment_to_case_modal/add_page_attachment_to_case_modal.tsx

.../observability/plugins/synthetics/public/apps/synthetics/hooks/use_monitor_detail_locator.ts

.../plugins/synthetics/public/apps/synthetics/components/monitor_details/add_to_case_action.tsx

cesco-f · 2025-08-13T10:11:58Z

.../plugins/synthetics/public/apps/synthetics/components/monitor_details/add_to_case_action.tsx

+          pageAttachmentState={pageState}
+          cases={cases}
+          onCloseModal={onCloseModal}
+          notifications={notifications}


I think it'd be better not to pass notifications at all, we can get it where needed from useKibana().services, right?

Notifications is a core component, which is available globally, so you technically you are right. However, let me explain this pattern a bit more to give you context of why I chose to pass the dependency rather than use useKibana.

Shared components often need dependencies. To fetch dependencies, we often use useKibana. However, those dependencies are dictated by the consuming plugin.

Let's pretend for a second I wasn't passing notifications. Let's instead pretend I was passing observabilityAIAssistant. (I mentioned before I split this off from a larger ticket, which did pass observabilityAIAssistant).

If I use useKibana within the shared component to fetch that dependency, but it's not available from the consuming plugin, I'm likely to get a big fat reference issue. This is a typical pattern and bug we see in things like the alerts flyout, where the flyout code can be called from multiple different plugins. It's also the same issue that caused the bug Justin is working on in this PR.

By having shared components explicitly define their dependencies rather than hiding the dependency and fetching it via useKibana, we avoid that class of bug and explicitly notify the calling component of which dependencies are needed to utilize this shared component. The plugin can then include those dependencies if they wish to use that shared component.

Got it!

We had a short conversation in private but I think the summary could be: if it's a new shared component we should not use this pattern, the plugin integrating the component will have to make sure to have the required services in their Kibana context.

The problem comes when adding a required service to a shared component that is already being used in different plugins, in that case we'll get an error if one of the plugins does not have the required service available in their context.

Please correct me if I'm wrong.

yes that is correct, and we've had this class of bugs come up many, many times before.

Are we keeping notifications as a parameter even if it's a new shared component?

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

…no-cache --fix'

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

x-pack/solutions/observability/plugins/observability_shared/kibana.jsonc

x-pack/solutions/observability/plugins/synthetics/kibana.jsonc

TinaHeiligers

Changes to observability_shared and synthetics manifest files aren't valid json.

Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>

…bana.jsonc Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>

TinaHeiligers

LGTM

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

cesco-f · 2025-08-19T06:47:24Z

yarn.lock

    schema-utils "^3.0.0"

-"string-width-cjs@npm:string-width@^4.2.0":
+"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:


Why there are changes to yarn.lock?

I think it's possible someone didn't merge their yarn.lock in a PR that went into main, cause these are the changes that are being generated via yarn kbn bootstrap.

cesco-f · 2025-08-19T06:48:38Z

...ed/public/components/add_page_attachment_to_case_modal/add_page_attachment_to_case_modal.tsx

+    >
+      <AddToCaseButtonContent
+        pageAttachmentState={pageAttachmentState}
+        notifications={notifications}


Suggested change

notifications={notifications}

This notifications is indeed in use, it is used to show a notification if you do not have proper privileges.

I don't see notifications used in AddToCaseButtonContent

Commit pushed.

cesco-f

LGTM

elasticmachine · 2025-08-19T15:10:31Z

💚 Build Succeeded

Buildkite Build
Commit: df00725

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`observabilityShared`	308	316	+8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`observabilityShared`	516	517	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`observability`	1.3MB	1.3MB	+47.0B
`observabilityShared`	37.5KB	99.0KB	+61.4KB
`synthetics`	1.0MB	1.0MB	-62.0B
total			+61.4KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`observabilityShared`	20	21	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`observability`	155.9KB	155.9KB	+31.0B
`observabilityShared`	67.1KB	67.5KB	+452.0B
total			+483.0B

Unknown metric groups

API count

id	before	after	diff
`observabilityShared`	522	524	+2

async chunk count

id	before	after	diff
`observabilityShared`	6	7	+1

ESLint disabled line counts

id	before	after	diff
`observabilityShared`	14	15	+1

Total ESLint disabled count

id	before	after	diff
`observabilityShared`	15	16	+1

History

💔 Build #330786 failed aadf817
💚 Build #330641 succeeded 5ae4e32
💔 Build #330584 failed 3b85c8b
💔 Build #330536 failed 15217db
💔 Build #330512 failed 3b27554
💔 Build #330493 failed 7ad8159

## Summary Summarize your PR. If it involves visual changes include a screenshot or gif. [https___google.com - Monitors - Synthetics - Observability - Elastic (2).webm](https://github.com/user-attachments/assets/1927f666-c9ca-49ad-a3fe-9e0ac8cadeb9) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Francesco Fagnani <fagnani.francesco@gmail.com> Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>

…)" This reverts commit 807b177.

…237064) This reverts commit 807b177.

…)" (elastic#237064) This reverts commit 807b177.

github-actions bot added the author:obs-ux-management PRs authored by the obs ux management team label Aug 8, 2025

dominiqueclarke force-pushed the feat/observability-page-attachment-modal branch 2 times, most recently from d4c19d7 to 5bab651 Compare August 8, 2025 16:49

dominiqueclarke force-pushed the feat/observability-page-attachment-modal branch from bec0d20 to 9b0da85 Compare August 8, 2025 18:41

dominiqueclarke added 6 commits August 8, 2025 14:47

adjust yarn.lock

10fd7c5

adjust test

4853346

adjust bundles

64d92f8

remove unused hook

a27687a

remove unused types

b64d218

fixed bug to prevent error when reopening modal

6ace322

dominiqueclarke changed the title ~~Feat/observability page attachment modal~~ [Incident Management] Add page attachment modal Aug 11, 2025

dominiqueclarke added Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. v9.2.0 release_note:skip Skip the PR/issue when compiling release notes labels Aug 11, 2025

dominiqueclarke marked this pull request as ready for review August 11, 2025 13:30

dominiqueclarke requested review from a team as code owners August 11, 2025 13:30

dominiqueclarke added the backport:skip This PR does not require backporting label Aug 11, 2025

dominiqueclarke and others added 7 commits August 11, 2025 09:39

Merge branch 'main' into feat/observability-page-attachment-modal

ebeb653

[CI] Auto-commit changed files from 'ts-node .buildkite/pipeline-reso…

9ff8f03

…urce-definitions/scripts/fix-location-collection.ts'

adjust test

bc84ba6

Merge branch 'feat/observability-page-attachment-modal' of github.com…

6a4c663

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

[CI] Auto-commit changed files from 'security: 3rd-party dependencies'

89b4e54

update tests

26fca0b

Merge branch 'feat/observability-page-attachment-modal' of github.com…

2dcd67c

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

cesco-f reviewed Aug 13, 2025

View reviewed changes

dominiqueclarke and others added 6 commits August 18, 2025 11:50

Merge branch 'feat/observability-page-attachment-modal' of github.com…

7ad8159

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

[CI] Auto-commit changed files from 'node scripts/eslint_all_files --…

3b27554

…no-cache --fix'

adjust types

e1c3a97

adjust onClose

810a135

Merge branch 'feat/observability-page-attachment-modal' of github.com…

15217db

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

adjust test

9018ff7

TinaHeiligers reviewed Aug 18, 2025

View reviewed changes

x-pack/solutions/observability/plugins/observability_shared/kibana.jsonc Outdated Show resolved Hide resolved

TinaHeiligers reviewed Aug 18, 2025

View reviewed changes

x-pack/solutions/observability/plugins/synthetics/kibana.jsonc Outdated Show resolved Hide resolved

TinaHeiligers requested changes Aug 18, 2025

View reviewed changes

dominiqueclarke and others added 2 commits August 18, 2025 15:40

Update x-pack/solutions/observability/plugins/synthetics/kibana.jsonc

4b9a7ee

Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>

Update x-pack/solutions/observability/plugins/observability_shared/ki…

3b85c8b

…bana.jsonc Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>

TinaHeiligers approved these changes Aug 18, 2025

View reviewed changes

dominiqueclarke added 2 commits August 18, 2025 22:11

adjust test

19f7914

Merge branch 'feat/observability-page-attachment-modal' of github.com…

5ae4e32

…:dominiqueclarke/kibana into feat/observability-page-attachment-modal

cesco-f reviewed Aug 19, 2025

View reviewed changes

dominiqueclarke added 4 commits August 19, 2025 08:48

adjust yarn.lock

8694eec

merge upstream

aadf817

adjust yarn.lock

bf14dbc

remove notifications

df00725

cesco-f approved these changes Aug 19, 2025

View reviewed changes

cesco-f removed their assignment Aug 19, 2025

dominiqueclarke merged commit 807b177 into elastic:main Aug 19, 2025
12 checks passed

dominiqueclarke deleted the feat/observability-page-attachment-modal branch August 19, 2025 15:23

fkanout added a commit to fkanout/kibana that referenced this pull request Sep 30, 2025

Revert "[Incident Management] Add page attachment modal (elastic#231186…

466eb3a

…)" This reverts commit 807b177.

mgiota added a commit to mgiota/kibana that referenced this pull request Sep 30, 2025

Revert "[Incident Management] Add page attachment modal (elastic#231186…

cd9bfda

…)" This reverts commit 807b177.

fkanout added a commit to fkanout/kibana that referenced this pull request Oct 1, 2025

Revert "[Incident Management] Add page attachment modal (elastic#231186…

0abaaf0

…)" This reverts commit 807b177.

fkanout added a commit that referenced this pull request Oct 1, 2025

Revert "[Incident Management] Add page attachment modal (#231186)" (#…

67ebfee

…237064) This reverts commit 807b177.

rylnd pushed a commit to rylnd/kibana that referenced this pull request Oct 17, 2025

Revert "[Incident Management] Add page attachment modal (elastic#231186…

69b74e0

…)" (elastic#237064) This reverts commit 807b177.

Conversation

dominiqueclarke commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

elasticmachine commented Aug 11, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cesco-f Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

TinaHeiligers left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TinaHeiligers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cesco-f left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 19, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dominiqueclarke commented Aug 8, 2025 •

edited

Loading

cesco-f Aug 13, 2025 •

edited

Loading

TinaHeiligers left a comment •

edited

Loading