Skip to content

[Cases][AttachmentV2] Migrate persistable state part 2 - ML and AIOps charts#262597

Merged
christineweng merged 4 commits intoelastic:mainfrom
christineweng:cases-migrate-persistable-2
Apr 17, 2026
Merged

[Cases][AttachmentV2] Migrate persistable state part 2 - ML and AIOps charts#262597
christineweng merged 4 commits intoelastic:mainfrom
christineweng:cases-migrate-persistable-2

Conversation

@christineweng
Copy link
Copy Markdown
Contributor

@christineweng christineweng commented Apr 10, 2026

Summary

Dependecy: #261407 to be merged first

This PR migrates the remaining persistable state attachments to the unified attachment registry. Case attachment type constants are moved to cases plugin, and a naming conversion is added to keep type names follow the same convention.

Feature flag: xpack.cases.attachments.enabled: true (restart kibana after turning the flag on/off)

Key changes per plugin:

Cases:

  • Removed persistable state registry in user actions, updated related tests
  • Added schema validator on the client side

Machine learning:

  • Updated persistable state registry to unified registry, updated the shape to match the new schema
  • Removed locally defined constants related to cases

AiOps:

  • Updated persistable state registry to unified registry, updated the shape to match the new schema
  • Removed locally defined constants related to cases

How to test:

Check and confirm the charts can be added and viewed in a case with the feature flag off/on. Note that turning the flag on then off will hide any attachments added when the flag is on, this is expected.

Checklist

  • 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.

@christineweng christineweng self-assigned this Apr 10, 2026
@christineweng christineweng force-pushed the cases-migrate-persistable-2 branch from 38ce0f3 to 71cec09 Compare April 10, 2026 17:37

return persistableBuilder.build();

default:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium comment/comment.tsx:220

Removing the AttachmentType.persistableState case causes persistable state attachments to silently return [] instead of rendering. When isLegacyAttachmentRequest(attachment) is true for a persistable state attachment, the switch falls through to default and produces an empty array, hiding the attachment from users.

🤖 Copy this AI Prompt to have your agent fix this:
In file x-pack/platform/plugins/shared/cases/public/components/user_actions/comment/comment.tsx around line 220:

Removing the `AttachmentType.persistableState` case causes persistable state attachments to silently return `[]` instead of rendering. When `isLegacyAttachmentRequest(attachment)` is true for a persistable state attachment, the switch falls through to `default` and produces an empty array, hiding the attachment from users.

Evidence trail:
- `x-pack/platform/plugins/shared/cases/common/constants/attachments.ts` lines 37-44: `LEGACY_ATTACHMENT_TYPES` includes `LEGACY_PERSISTABLE_STATE_TYPE`
- `x-pack/platform/plugins/shared/cases/common/utils/attachments/v1_type_guards.ts` lines 36-40: `isLegacyAttachmentRequest()` checks if type is in `LEGACY_ATTACHMENT_TYPES`
- `x-pack/platform/plugins/shared/cases/public/components/user_actions/comment/comment.tsx` lines 179-221 (REVIEWED_COMMIT): switch statement inside `if (isLegacyAttachmentRequest(attachment))` block ends with `default: return [];` - missing `persistableState` case
- git_diff base=MERGE_BASE head=REVIEWED_COMMIT path=comment.tsx: shows `case AttachmentType.persistableState:` was removed

@christineweng christineweng force-pushed the cases-migrate-persistable-2 branch from 71cec09 to 743c4f3 Compare April 13, 2026 20:36
@christineweng christineweng added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Cases Security Solution Cases team v9.5.0 labels Apr 13, 2026
@christineweng christineweng marked this pull request as ready for review April 13, 2026 20:41
@christineweng christineweng requested review from a team as code owners April 13, 2026 20:41
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low comment/comment.tsx:67

If comment.type === AttachmentType.persistableState, the deleted persistableStateAttachmentTypeRegistry branch is no longer executed, so getDeleteLabelTitle returns the generic ${i18n.REMOVED_FIELD} ${i18n.COMMENT.toLowerCase()} label instead of the registry-specific label that getDeleteLabelFromRegistry would have provided. Since persistableState is in LEGACY_ATTACHMENT_TYPES, isLegacyAttachmentRequest(comment) returns true, but the removed persistableState case means the code falls through to the final return statement.

  if (isUnifiedReferenceAttachmentRequest(comment)) {
     return getDeleteLabelFromRegistry({
       caseData,
       registry: unifiedAttachmentTypeRegistry,
       getId: () => toUnifiedAttachmentType(comment.type, owner),
       getAttachmentProps: () => ({
         attachmentId: comment.attachmentId,
         metadata: comment.metadata,
       }),
     });
   }
 
+  if (comment.type === AttachmentType.persistableState) {
+    return `${i18n.REMOVED_FIELD} ${i18n.ATTACHMENT.toLowerCase()}`;
+  }
+
   return `${i18n.REMOVED_FIELD} ${i18n.COMMENT.toLowerCase()}`;
🤖 Copy this AI Prompt to have your agent fix this:
In file x-pack/platform/plugins/shared/cases/public/components/user_actions/comment/comment.tsx around lines 67-80:

If `comment.type === AttachmentType.persistableState`, the deleted `persistableStateAttachmentTypeRegistry` branch is no longer executed, so `getDeleteLabelTitle` returns the generic `${i18n.REMOVED_FIELD} ${i18n.COMMENT.toLowerCase()}` label instead of the registry-specific label that `getDeleteLabelFromRegistry` would have provided. Since `persistableState` is in `LEGACY_ATTACHMENT_TYPES`, `isLegacyAttachmentRequest(comment)` returns `true`, but the removed `persistableState` case means the code falls through to the final return statement.

Evidence trail:
- Diff at REVIEWED_COMMIT shows removal of `persistableState` branch from `getDeleteLabelTitle` (lines 63-75 removed in diff)
- `x-pack/platform/plugins/shared/cases/common/constants/attachments.ts` lines 27, 37-44: `LEGACY_PERSISTABLE_STATE_TYPE = 'persistableState'` is in `LEGACY_ATTACHMENT_TYPES`
- `x-pack/platform/plugins/shared/cases/common/utils/attachments/v1_type_guards.ts` lines 36-41: `isLegacyAttachmentRequest` returns true for types in `LEGACY_ATTACHMENT_TYPES`
- `x-pack/platform/plugins/shared/cases/common/utils/attachments/v2_type_guards.ts` lines 29-33: `isUnifiedReferenceAttachmentRequest` checks for `'attachmentId' in context`
- `x-pack/platform/plugins/shared/cases/common/types/domain/attachment/v1.ts` lines 301-306: `PersistableStateAttachmentPayloadRt` has `persistableStateAttachmentTypeId` and `persistableStateAttachmentState`, not `attachmentId`

@peteharverson peteharverson requested a review from rbrtj April 14, 2026 08:26
@christineweng christineweng force-pushed the cases-migrate-persistable-2 branch from 743c4f3 to ff01442 Compare April 14, 2026 17:49
Copy link
Copy Markdown
Contributor

@rbrtj rbrtj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ML & AIOps changes LGTM

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Apr 15, 2026

Catch flakiness early (recommended)

Recommended before merge: run the flaky test runner against this PR to catch flakiness early.

Trigger a run with the Flaky Test Runner UI or post this comment on the PR:

/flaky ftrConfig:x-pack/platform/test/cases_api_integration/security_and_spaces/config_basic.ts:30
/flaky ftrConfig:x-pack/platform/test/cases_api_integration/security_and_spaces/config_trial.ts:30
/flaky ftrConfig:x-pack/platform/test/functional_with_es_ssl/apps/cases/group2/config.ts:30

This check is experimental. Share your feedback in the #appex-qa channel.

Posted via Macroscope — Flaky Test Runner nudge

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #85 / dashboard app - esql controls dashboard - add a value type ES|QL control should add an ES|QL value control

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 618 619 +1
cases 1992 1991 -1
total -0

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
cases 143 149 +6

Async chunks

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

id before after diff
aiops 525.6KB 525.6KB +25.0B
cases 2.3MB 2.3MB -1.2KB
ml 5.8MB 5.8MB +123.0B
total -1.0KB

Page load bundle

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

id before after diff
aiops 13.5KB 13.8KB +272.0B
cases 167.9KB 168.7KB +762.0B
total +1.0KB
Unknown metric groups

API count

id before after diff
cases 169 175 +6

History

cc @christineweng

@kibanamachine
Copy link
Copy Markdown
Contributor

Flaky Test Runner Stats

🟠 Some tests failed. - kibana-flaky-test-suite-runner#11644

[✅] x-pack/platform/test/cases_api_integration/security_and_spaces/config_basic.ts: 25/25 tests passed.
[❌] x-pack/platform/test/cases_api_integration/security_and_spaces/config_trial.ts: 24/25 tests passed.
[✅] x-pack/platform/test/functional_with_es_ssl/apps/cases/group2/config.ts: 25/25 tests passed.

see run history

),
type: attachmentTypeByEmbeddableType[embeddableType],
data: {
state: JSON.parse(JSON.stringify(persistableStateAttachmentState)),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we consider structuredClone here?

@christineweng christineweng enabled auto-merge (squash) April 17, 2026 18:59
@christineweng christineweng merged commit 8af6815 into elastic:main Apr 17, 2026
17 checks passed
kapral18 added a commit to kapral18/kibana that referenced this pull request Apr 19, 2026
* main: (114 commits)
  Fix observability_ai_assistant_tool_call EBT error when connector is an inference endpoint (elastic#263334)
  init on install (elastic#263732)
  [One Workflow] fail-fast TaskRecovery for interrupted runs (elastic#261275)
  [Entity Store] Reset state error after successful task run (elastic#263087)
  [api-docs] 2026-04-19 Daily api_docs build (elastic#264280)
  [UII] Fix integration card row height calculation (elastic#264212)
  [scout] migrate FTR logstash api tests (elastic#262953)
  [StorageIndexAdapter] Set auto_expand_replicas to fix yellow health on single-node ES clusters (elastic#263096)
  [api-docs] 2026-04-18 Daily api_docs build (elastic#264260)
  [Scout] Update test config manifests (elastic#264257)
  [Security Solution][Detection Engine] enables AI rule creation feature flag (elastic#264036)
  [dashboards as code] only validate id on PUT route when creating new dashboard (elastic#264161)
  chore(NA): bump version to 9.5.0 (elastic#262165)
  skip failing test suite (elastic#263649)
  skip failing test suite (elastic#264236)
  [Discover] Convert remaining Enzyme tests to RTL (elastic#259676)
  auto-implement: Labels in model endpoints table of the model details flyout look misaligned (elastic#263770)
  [ci] Promote ES docker image after verification (elastic#263890)
  [Observability:Onboarding] Remove suppress global announcements that was breaking ensemble tests (elastic#264169)
  [Cases][AttachmentV2] Migrate persistable state part 2 - ML and AIOps charts (elastic#262597)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Cases Security Solution Cases team v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants