[Cases][Attachments] Case attachments V2 dual read by christineweng · Pull Request #256133 · elastic/kibana

christineweng · 2026-03-05T00:06:34Z

Summary

This PR introduced the dual read logic in case attachments V2. It follows the same feature flag logic as write does: when feature flag is on, fetch will either fetching from both saved objects (find method), or try the new SO first, if failed it will fetch from the cases-comments SO (get method).

A new field called mode is introduced at the client and service level. This is used to decide the form of the attachment response. For public apis, we always pass mode='legacy' so the response matches our api docs. For internal routes like findUserActions, we pass mode='unified' and transform the response to the V2 format.

This PR also fully migrates the comment (user) attachment type to the unified registry. This means the rendering on the activity timeline is now driven by the registry, instead of hard coded renderer in the cases code.

You can read more about the architecture design in the OneCases doc

Content of this PR

Read based on feature flag at the client and service level
- x-pack/platform/plugins/shared/cases/server/services/attachments/operations/get.ts
Added mode to routes that return an attachment response
- x-pack/platform/plugins/shared/cases/server/client/attachments
- x-pack/platform/plugins/shared/cases/server/services/attachments
- x-pack/platform/plugins/shared/cases/server/common/models/case_with_comments.ts
Moved comment to unified registry
- x-pack/platform/plugins/shared/cases/public/components/attachments/comment/index.tsx
- x-pack/platform/plugins/shared/cases/public/components/user_actions/comment/comment.tsx
Type widening to accept V1 and V2 types
Cleaned up and organized helpers to reduce repetition and long chains of checks

How to test:

Feature flag: xpack.cases.attachments.enabled

FF is off

All existing cases functionalities should not be impacted

FF is on

In Discover: New comments are written to cases-attachment saved object
In Cases: comments from both SO should render correctly, stats reflects correct counts

This PR is assisted by Cursor

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines
Review the backport guidelines and apply applicable backport:* labels.

janmonschke · 2026-03-05T09:39:37Z

Could you explain why we need those now?

janmonschke · 2026-03-05T09:45:18Z

Consider adding an interface for this and extend from that. Same for the other occurrences of mode in this file

added a type explicitly to 1) verify it is needed for each method 2) easier to find and remove later

elasticmachine · 2026-03-13T21:51:05Z

Pinging @elastic/kibana-cases (Team:Cases)

christineweng · 2026-03-13T21:53:57Z

  rt.exact(
    rt.partial({
-      comments: rt.array(AttachmentRt),
+      comments: rt.array(AttachmentRtV2),


I had to expand this to V2 so that typescript doesn't complain in some utils.

Based on the api doc review this week, no actual comments are returned in current public apis, so I think this should be fine
#256805
#257257

chrisbmar

x-pack/platform/plugins/shared/agent_builder_platform/server/tools/cases/helpers.ts code-review only LGTM

michaelolo24 · 2026-03-18T18:35:48Z

+          aggregations: {
+            caseIds: {
+              terms: {
+                field: `${CASE_ATTACHMENT_SAVED_OBJECT}.references.id`,


We're still eventually going to store the caseId on the saved object as well right?

ah yes, i wanted to discuss this separately: if we may want to add caseId in cases, case-comments(?) and case-attachments so they all link together

Yea, that would be my hope. Should simplify things I suspect. Would we also maintain it in the references array? There are out of the box benefits to them being in references as well...from a migration perspective. References will serve as the source of truth in case the ids change again for whatever reason

michaelolo24 · 2026-03-18T18:37:38Z

+            attachmentId
+          );
+        } catch (error) {
+          res = await this.context.unsecuredSavedObjectsClient.get<AttachmentPersistedAttributes>(


We should log here and check if there was a legitimate failure that happened with the attachment SO. Also defaulting to just checking comments could also lead to an error with the comment so that then doesn't get caught either. We should ideally update this to handle failures in both and provide helpful log messages in case we need to triage

janmonschke · 2026-03-19T09:58:19Z

Not sure if I'm testing something that is out-of-scope but I found this:

When I go to http://localhost:5601/app/security/alerts and perform Attach to existing case / Attach to new case, I'm getting the following errors:

christineweng · 2026-03-20T01:52:42Z

Not sure if I'm testing something that is out-of-scope but I found this:

When I go to http://localhost:5601/app/security/alerts and perform Attach to existing case / Attach to new case, I'm getting the following errors:

We discussed this in standup. Alert is throwing an error because it is not migrated. Only user comment is migrated in this PR

elasticmachine · 2026-03-20T17:39:04Z

💔 Build Failed

Buildkite Build
Commit: 0653378

Failed CI Steps

Test Failures

[job] [logs] Scout: [ platform / workflows_management ] plugin / local-serverless-observability_complete - Scheduled workflow execution - enabling a scheduled workflow triggers executions automatically
[job] [logs] Scout: [ platform / workflows_management ] plugin / local-serverless-observability_complete - Scheduled workflow execution - enabling a scheduled workflow triggers executions automatically

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`cases`	2088	2093	+5

Async chunks

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

id	before	after	diff
`cases`	2.0MB	2.0MB	+26.9KB

Page load bundle

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

id	before	after	diff
`cases`	204.1KB	207.3KB	+3.3KB

Unknown metric groups

async chunk count

id	before	after	diff
`cases`	50	53	+3

History

cc @christineweng

## Summary This PR introduced the dual read logic in case attachments V2. It follows the same feature flag logic as write does: when feature flag is on, fetch will either fetching from both saved objects (`find` method), or try the new SO first, if failed it will fetch from the `cases-comments` SO (`get` method). A new field called `mode` is introduced at the client and service level. This is used to decide the form of the attachment response. For public apis, we always pass `mode='legacy'` so the response matches our api docs. For internal routes like `findUserActions`, we pass `mode='unified'` and transform the response to the V2 format. This PR also fully migrates the comment (`user`) attachment type to the unified registry. This means the rendering on the activity timeline is now driven by the registry, instead of hard coded renderer in the cases code. You can read more about the architecture design in the [OneCases doc](https://docs.google.com/document/d/1JaourUjQaqF6e9gwqsrQl-ddZ3aeXuB3hrrq67km6SU/edit?tab=t.djrd1kyyx11k) ### Content of this PR - Read based on feature flag at the client and service level - x-pack/platform/plugins/shared/cases/server/services/attachments/operations/get.ts - Added `mode` to routes that return an attachment response - x-pack/platform/plugins/shared/cases/server/client/attachments - x-pack/platform/plugins/shared/cases/server/services/attachments - x-pack/platform/plugins/shared/cases/server/common/models/case_with_comments.ts - Moved comment to unified registry - x-pack/platform/plugins/shared/cases/public/components/attachments/comment/index.tsx - x-pack/platform/plugins/shared/cases/public/components/user_actions/comment/comment.tsx - Type widening to accept V1 and V2 types - Cleaned up and organized helpers to reduce repetition and long chains of checks ### How to test: Feature flag: `xpack.cases.attachments.enabled` **FF is off** - All existing cases functionalities should not be impacted **FF is on** - In Discover: New comments are written to `cases-attachment` saved object - In Cases: comments from both SO should render correctly, stats reflects correct counts This PR is assisted by Cursor ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels.

christineweng self-assigned this Mar 5, 2026

christineweng added release_note:fix backport:skip This PR does not require backporting Team:Cases Security Solution Cases team labels Mar 5, 2026